[SPARK-3688][SQL]LogicalPlan can't resolve column correctlly #2542

tianyi · 2014-09-26T06:58:29Z

we should add some judgements base on the datatype to handle some particular sql described in https://issues.apache.org/jira/browse/SPARK-3688

AmplabJenkins · 2014-09-26T07:02:09Z

Can one of the admins verify this patch?

liancheng · 2014-09-26T09:37:05Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala

Please rename this test...

cloud-fan · 2014-09-26T09:40:33Z

Nice catch! I think the problem is: for a table(a string, b string), when we run "select a.b from test a join test b", we have 2 options to resolve a.b. One is "table a, column b", another is "table b, column a.b". Here we know that column a don't have a sub column b, so "table b, column a.b" is invalid, the only option is "table a, column b". However, the system didn't know this and report "ambiguousReferences".
But what if we really have a sub column b for column a? Should we report "ambiguousReferences"? @marmbrus @liancheng What do you think?

tianyi · 2014-09-26T10:00:05Z

@cloud-fan
I think we should follow some orders to resolve a.b
First, if 'a' match a table name or alias, then we recognize a as a table.
if 'a' did not match any table name or alias, then we expect 'a' should be a structType column.

cloud-fan · 2014-09-26T12:04:43Z

@tianyi Let me raise an example. For table: { a: { c: String }, b: String }, if we run select a.b from test a join test b, your PR will still give 2 options to resolve a.b: "table a, column b", "table b, column a.b" as a is StructType. Actually we can tell that a have a sub column c, not b, so "table b, column a.b" is invalid. But it's hard to tell whether the column can be resolved in LogicalPlan#resolve as column name can be very complicated like a.b[1].c.d[0]
I think a perfect solution is keep all the possible resolve path, and go one by one. If more than one path success, then throw an "ambiguousReferences" exception.

liancheng · 2014-09-27T02:30:47Z

@cloud-fan Tried the following snippet with ambiguous references in Hive:

CREATE TABLE t1(a STRUCT<x: INT>, k INT);
CREATE TABLE t2(x INT);
SELECT a.x FROM t1 a JOIN t2 b ON a.k = b.x;

and Hive reports "Invalid column reference 'x'", which I think is reasonable. So it seems that Hive treats struct fields and table columns equally.

liancheng · 2014-09-27T12:21:59Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/LogicalPlan.scala

I don't think we need the withName call anymore, since now we preserve case of attribute names by leveraging resolver to compare names.

liancheng · 2014-09-27T15:29:55Z

@cloud-fan Sorry, made a mistake in the snippet I used, it should be:

CREATE TABLE t1(x INT);
CREATE TABLE t2(a STRUCT<x: INT>, k INT);
SELECT a.x FROM t1 a JOIN t2 b ON a.x = b.k;

And Hive can resolve a.x successfully.

marmbrus · 2014-09-27T19:02:59Z

ok to test

SparkQA · 2014-09-27T19:09:27Z

QA tests have started for PR 2542 at commit 252dbbb.

This patch merges cleanly.

SparkQA · 2014-09-27T21:09:28Z

Tests timed out after a configured wait of 120m.

AmplabJenkins · 2014-09-27T21:09:30Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20921/

tianyi · 2014-09-28T01:51:12Z

Hi, @marmbrus . The current codes still have some bugs to fix, I talked @liancheng yesterday, I will push a update later.

SparkQA · 2014-09-28T02:49:29Z

QA tests have started for PR 2542 at commit a018641.

This patch merges cleanly.

AmplabJenkins · 2014-09-28T02:52:17Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20925/

cloud-fan · 2014-09-28T02:57:20Z

@liancheng Hmm..I don't have a hive environment for test...

CREATE TABLE t1(x INT);
CREATE TABLE t2(a STRUCT<x: INT>, k INT);
SELECT a.x FROM t1 a JOIN t2 b;

Without this PR, spark sql will report ambiguousReferences, but how hive resolve a.x? "table a, column x" or "table b, column a.x"?
Or do we have to add the ON statement?

SparkQA · 2014-09-28T02:59:11Z

QA tests have started for PR 2542 at commit a018641.

This patch merges cleanly.

tianyi · 2014-09-28T04:02:16Z

@cloud-fan, I think it is reasonable for return "ambiguous references" in the case you mentioned, because we can't make sure whether 'a' is a table alias or column name. In my last commits, spark will return "ambiguous references" for your case.

cloud-fan · 2014-09-28T04:16:12Z

@tianyi

CREATE TABLE t1(x INT);
CREATE TABLE t2(a STRUCT<x: INT>, k INT);
SELECT a.x FROM t1 a JOIN t2 b ON a.x = b.k;

But hive can resolve this as @liancheng said. What's the magic here for the ON statement?

SparkQA · 2014-09-28T04:49:29Z

Tests timed out after a configured wait of 120m.

AmplabJenkins · 2014-09-28T04:49:32Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20926/

SparkQA · 2014-09-28T04:59:11Z

Tests timed out after a configured wait of 120m.

SparkQA · 2014-09-28T06:04:31Z

QA tests have started for PR 2542 at commit e9cd8be.

This patch merges cleanly.

SparkQA · 2014-09-28T08:04:32Z

Tests timed out after a configured wait of 120m.

AmplabJenkins · 2014-09-28T08:04:40Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20929/

SparkQA · 2014-09-28T08:49:57Z

QA tests have started for PR 2542 at commit e9cd8be.

This patch merges cleanly.

SparkQA · 2014-09-28T10:49:58Z

Tests timed out after a configured wait of 120m.

…cases

SparkQA · 2014-10-29T04:05:18Z

Test build #22417 has finished for PR 2542 at commit efc2df3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-10-29T04:05:21Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22417/
Test FAILed.

liancheng · 2014-10-30T04:07:48Z

retest this please

SparkQA · 2014-10-30T04:09:47Z

Test build #22517 has started for PR 2542 at commit b708fc7.

This patch merges cleanly.

SparkQA · 2014-10-30T04:14:55Z

Test build #22519 has started for PR 2542 at commit b708fc7.

This patch merges cleanly.

SparkQA · 2014-10-30T05:00:16Z

Test build #22517 has finished for PR 2542 at commit b708fc7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-10-30T05:00:18Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22517/
Test PASSed.

SparkQA · 2014-10-30T05:06:22Z

Test build #22519 has finished for PR 2542 at commit b708fc7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-10-30T05:06:24Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22519/
Test PASSed.

tianyi · 2014-10-30T05:11:38Z

Hi, @marmbrus @liancheng
I had rebased this PR after #2762.
Any more comments on this?

AmplabJenkins · 2014-11-11T21:57:10Z

Can one of the admins verify this patch?

liancheng · 2014-11-14T23:45:49Z

ok to test

SparkQA · 2014-11-14T23:47:46Z

Test build #23398 has started for PR 2542 at commit b708fc7.

This patch merges cleanly.

SparkQA · 2014-11-15T00:55:38Z

Test build #23398 has finished for PR 2542 at commit b708fc7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-11-15T00:55:41Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23398/
Test PASSed.

SparkQA · 2014-12-17T21:37:11Z

Test build #548 has started for PR 2542 at commit b708fc7.

This patch merges cleanly.

SparkQA · 2014-12-17T21:49:55Z

Test build #548 has finished for PR 2542 at commit b708fc7.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2014-12-17T22:28:49Z

Test build #549 has started for PR 2542 at commit b708fc7.

This patch does not merge cleanly.

SparkQA · 2014-12-18T00:07:59Z

Test build #549 has finished for PR 2542 at commit b708fc7.

This patch fails PySpark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

liancheng · 2014-12-22T05:06:40Z

retest this please

SparkQA · 2014-12-22T05:07:30Z

Test build #24696 has started for PR 2542 at commit b708fc7.

This patch merges cleanly.

SparkQA · 2014-12-22T06:17:57Z

Test build #24696 has finished for PR 2542 at commit b708fc7.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2014-12-22T06:18:00Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24696/
Test PASSed.

tianyi · 2015-02-11T06:01:40Z

It's hard to rebase this branch due to so much changes in the latest master branch.
So I had opened a new PR #4524 for this Issue.

liancheng reviewed Sep 26, 2014
View reviewed changes

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala Outdated

Copy link

Contributor

liancheng Sep 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename this test...

liancheng reviewed Sep 27, 2014
View reviewed changes

tianyi added 3 commits October 29, 2014 10:44

add codes to implement the compatibility of with hive, and some test …

84b35ad

…cases

move tests to HiveResolutionSuite

c927056

fix error in test sql

8bfac8b

add golden files

b708fc7

tianyi force-pushed the resolve-issue branch from efc2df3 to b708fc7 Compare October 30, 2014 04:03

tianyi closed this Feb 11, 2015

[SPARK-3688][SQL]LogicalPlan can't resolve column correctlly #2542

[SPARK-3688][SQL]LogicalPlan can't resolve column correctlly #2542

Uh oh!

Conversation

tianyi commented Sep 26, 2014

Uh oh!

AmplabJenkins commented Sep 26, 2014

Uh oh!

liancheng Sep 26, 2014

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Sep 26, 2014

Uh oh!

tianyi commented Sep 26, 2014

Uh oh!

cloud-fan commented Sep 26, 2014

Uh oh!

liancheng commented Sep 27, 2014

Uh oh!

liancheng Sep 27, 2014

Choose a reason for hiding this comment

Uh oh!

liancheng commented Sep 27, 2014

Uh oh!

marmbrus commented Sep 27, 2014

Uh oh!

SparkQA commented Sep 27, 2014

Uh oh!

SparkQA commented Sep 27, 2014

Uh oh!

AmplabJenkins commented Sep 27, 2014

Uh oh!

tianyi commented Sep 28, 2014

Uh oh!

SparkQA commented Sep 28, 2014

Uh oh!

AmplabJenkins commented Sep 28, 2014

Uh oh!

cloud-fan commented Sep 28, 2014

Uh oh!

SparkQA commented Sep 28, 2014

Uh oh!

tianyi commented Sep 28, 2014

Uh oh!

cloud-fan commented Sep 28, 2014

Uh oh!

SparkQA commented Sep 28, 2014

Uh oh!

AmplabJenkins commented Sep 28, 2014

Uh oh!

SparkQA commented Sep 28, 2014

Uh oh!

SparkQA commented Sep 28, 2014

Uh oh!

SparkQA commented Sep 28, 2014

Uh oh!

AmplabJenkins commented Sep 28, 2014

Uh oh!

SparkQA commented Sep 28, 2014

Uh oh!

SparkQA commented Sep 28, 2014

Uh oh!

SparkQA commented Oct 29, 2014

Uh oh!

AmplabJenkins commented Oct 29, 2014

Uh oh!

liancheng commented Oct 30, 2014

Uh oh!

SparkQA commented Oct 30, 2014

Uh oh!

SparkQA commented Oct 30, 2014

Uh oh!

SparkQA commented Oct 30, 2014

Uh oh!

AmplabJenkins commented Oct 30, 2014

Uh oh!

SparkQA commented Oct 30, 2014

Uh oh!

AmplabJenkins commented Oct 30, 2014

Uh oh!