You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/26 00:07:03 UTC
[GitHub] [spark] karenfeng opened a new pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
karenfeng opened a new pull request #31654:
URL: https://github.com/apache/spark/pull/31654
### What changes were proposed in this pull request?
Today, child expressions may be resolved based on "real" or metadata output attributes. We should prefer the real attribute during resolution if one exists.
### Why are the changes needed?
Today, attempting to resolve an expression when there is a "real" output attribute and a metadata attribute with the same name results in resolution failure. This is likely unexpected, as the user may not know about the metadata attribute.
### Does this PR introduce _any_ user-facing change?
Yes. Previously, the user would see an error message when resolving a column with the same name as a "real" output attribute and a metadata attribute as below:
```
org.apache.spark.sql.AnalysisException: Reference 'index' is ambiguous, could be: testcat.ns1.ns2.tableTwo.index, testcat.ns1.ns2.tableOne.index.; line 1 pos 71
at org.apache.spark.sql.catalyst.expressions.package$AttributeSeq.resolve(package.scala:363)
at org.apache.spark.sql.catalyst.plans.logical.LogicalPlan.resolveChildren(LogicalPlan.scala:107)
```
Now, resolution succeeds and provides the "real" output attribute.
### How was this patch tested?
Added a unit test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-786367089
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40062/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-786419848
**[Test build #135482 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135482/testReport)** for PR 31654 at commit [`1c5ab03`](https://github.com/apache/spark/commit/1c5ab03470ba1e99826c3238c0c6ae6fafef22ce).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788271206
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40189/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788461709
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135609/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788271206
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40189/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #31654:
URL: https://github.com/apache/spark/pull/31654
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-786421438
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135482/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788461709
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135609/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788245660
**[Test build #135609 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135609/testReport)** for PR 31654 at commit [`5a7209e`](https://github.com/apache/spark/commit/5a7209edd25fd0f59a0eadc1b0dabd2d146d2a70).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-786332664
**[Test build #135482 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135482/testReport)** for PR 31654 at commit [`1c5ab03`](https://github.com/apache/spark/commit/1c5ab03470ba1e99826c3238c0c6ae6fafef22ce).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] karenfeng commented on a change in pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
karenfeng commented on a change in pull request #31654:
URL: https://github.com/apache/spark/pull/31654#discussion_r584455995
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -2492,6 +2492,35 @@ class DataSourceV2SQLSuite
}
}
+ test("SPARK-34547: metadata columns are resolved last") {
Review comment:
This is covered at the [level](https://github.com/apache/spark/blob/62737e140c7b04805726a33c392c297335db7b45/sql/catalyst/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Relation.scala#L59) of the DatasourceV2Relation, but does not apply above it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31654:
URL: https://github.com/apache/spark/pull/31654#discussion_r584455006
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -2492,6 +2492,35 @@ class DataSourceV2SQLSuite
}
}
+ test("SPARK-34547: metadata columns are resolved last") {
Review comment:
I'm a bit confused. We have a test `SPARK-31255: Projects data column when metadata column has the same name` in this file, which tests when column name and metadata column name have conflicts. Why does that test work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-786421438
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135482/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31654:
URL: https://github.com/apache/spark/pull/31654#discussion_r584461515
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -2492,6 +2492,35 @@ class DataSourceV2SQLSuite
}
}
+ test("SPARK-34547: metadata columns are resolved last") {
Review comment:
Ah I see! Can we remove the hack in `DatasourceV2Relation` now?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-786332664
**[Test build #135482 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135482/testReport)** for PR 31654 at commit [`1c5ab03`](https://github.com/apache/spark/commit/1c5ab03470ba1e99826c3238c0c6ae6fafef22ce).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] karenfeng commented on a change in pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
karenfeng commented on a change in pull request #31654:
URL: https://github.com/apache/spark/pull/31654#discussion_r584992981
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -2492,6 +2492,35 @@ class DataSourceV2SQLSuite
}
}
+ test("SPARK-34547: metadata columns are resolved last") {
Review comment:
We can remove it from `metadataOutput`, but I don't believe we can remove it in `withMetadataColumns` due to the output containing a name collision.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #31654:
URL: https://github.com/apache/spark/pull/31654#discussion_r584462469
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/DataSourceV2SQLSuite.scala
##########
@@ -2492,6 +2492,35 @@ class DataSourceV2SQLSuite
}
}
+ test("SPARK-34547: metadata columns are resolved last") {
+ val t1 = s"${catalogAndNamespace}tableOne"
+ val t2 = s"${catalogAndNamespace}tableTwo"
+ withTable(t1, t2) {
+ sql(s"CREATE TABLE $t1 (id bigint, data string) USING $v2Format " +
+ "PARTITIONED BY (bucket(4, id), id)")
+ sql(s"INSERT INTO $t1 VALUES (1, 'a'), (2, 'b'), (3, 'c')")
+ sql(s"CREATE TABLE $t2 (id bigint, index int) USING $v2Format")
Review comment:
I think the test is to join a v2 table with a relation, while the relation output has name conflicts with the v2 table's metadata output. It doesn't matter what the relation is and we can use a temp view to simplify the test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788271164
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40189/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788245660
**[Test build #135609 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135609/testReport)** for PR 31654 at commit [`5a7209e`](https://github.com/apache/spark/commit/5a7209edd25fd0f59a0eadc1b0dabd2d146d2a70).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788459914
**[Test build #135609 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135609/testReport)** for PR 31654 at commit [`5a7209e`](https://github.com/apache/spark/commit/5a7209edd25fd0f59a0eadc1b0dabd2d146d2a70).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788761781
thanks, merging to master/3.1!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-786367089
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40062/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #31654: [SPARK-34547][SQL] Only use metadata columns for resolution as last resort
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31654:
URL: https://github.com/apache/spark/pull/31654#issuecomment-788263828
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/40189/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org