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 2020/02/04 15:27:54 UTC
[GitHub] [spark] xuanyuanking opened a new pull request #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
xuanyuanking opened a new pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454
### What changes were proposed in this pull request?
This is a follow-up for #25029, in this PR we throw an AnalysisException for nested WITH clause. In this way, the config `spark.sql.legacy.ctePrecedence.enabled` should be set explicitly for the expected behavior.
### Why are the changes needed?
The original change is risky, it may change behavior silently.
### Does this PR introduce any user-facing change?
Yes, change the config `spark.sql.legacy.ctePrecedence.enabled` as optional.
### How was this patch tested?
New UT.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375797139
##########
File path: sql/core/src/test/resources/sql-tests/inputs/cte-nonlegacy.sql
##########
@@ -0,0 +1,2 @@
+--SET spark.sql.legacy.ctePrecedence.enabled = false
+--IMPORT cte.sql
Review comment:
we should do the same thing for `cte-legacy.sql`. This can be done in a followup.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581963973
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375666448
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
+ s"please select the CTE precedence via ${LEGACY_CTE_PRECEDENCE_ENABLED.key}, " +
Review comment:
`Please set ${LEGACY_CTE_PRECEDENCE_ENABLED.key} to false so that name defined in inner CTE takes precedence. See more details in SPARK-28228.`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583551270
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/118039/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375668782
##########
File path: sql/core/src/test/resources/sql-tests/inputs/cte-nonlegacy.sql
##########
@@ -0,0 +1,115 @@
+create temporary view t as select * from values 0, 1, 2 as t(id);
+create temporary view t2 as select * from values 0, 1 as t(id);
+
+-- CTE non-legacy substitution
+SET spark.sql.legacy.ctePrecedence.enabled=false;
Review comment:
use `--SET spark.sql.legacy.ctePrecedence.enabled=false`, so that this won't be treated as a query and appear in the result query.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583706063
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583550463
**[Test build #118039 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/118039/testReport)** for PR 27454 at commit [`7249404`](https://github.com/apache/spark/commit/72494043d5a431dbe024e7472b4df0e5d3b96352).
* 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581963987
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117844/
Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375448881
##########
File path: docs/sql-migration-guide.md
##########
@@ -103,7 +103,7 @@ license: |
- Since Spark 3.0, if files or subdirectories disappear during recursive directory listing (i.e. they appear in an intermediate listing but then cannot be read or listed during later phases of the recursive directory listing, due to either concurrent file deletions or object store consistency issues) then the listing will fail with an exception unless `spark.sql.files.ignoreMissingFiles` is `true` (default `false`). In previous versions, these missing files or subdirectories would be ignored. Note that this change of behavior only applies during initial table file listing (or during `REFRESH TABLE`), not during query execution: the net change is that `spark.sql.files.ignoreMissingFiles` is now obeyed during table file listing / query planning, not only at query execution time.
- - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.
+ - Since Spark 3.0, an AnalysisException will happen by default in case of nested WITH clauses, it forces the users to choose the specific substitution order they wanted, which is controlled by `spark.sql.legacy.ctePrecedence.enabled`. If set to false, inner CTE definitions take precedence over outer definitions. For example, set the config to `false`, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` returns `2`, while setting it to `true`, the result is `1` which is the behavior in version 2.4 and earlier.
Review comment:
Thanks for the advice, done in a55e82d.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583706063
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581964399
cc @cloud-fan
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583686905
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] peter-toth commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
peter-toth commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376279947
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,10 +29,49 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ assertNoNameConflictsInCTE(plan, inTraverse = false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ }
+
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def assertNoNameConflictsInCTE(
+ plan: LogicalPlan,
+ inTraverse: Boolean,
+ cteNames: Set[String] = Set.empty): Unit = {
+ plan.foreach {
+ case w @ With(child, relations) =>
+ val newNames = relations.map {
+ case (cteName, _) =>
+ if (cteNames.contains(cteName)) {
+ throw new AnalysisException(s"Name $cteName is conflict in nested CTE. " +
+ s"Please set ${LEGACY_CTE_PRECEDENCE_ENABLED.key} to false so that name defined " +
+ "in inner CTE takes precedence. See more details in SPARK-28228.")
+ } else {
+ cteName
+ }
+ }.toSet
+ (w.innerChildren :+ child).foreach { p =>
Review comment:
This could be:
```
child.transformExpressions {
case e: SubqueryExpression =>
assertNoNameConflictsInCTE(e.plan, inTraverse = true, cteNames ++ newNames)
e
}
w.innerChildren.foreach { p =>
assertNoNameConflictsInCTE(p, inTraverse = true, cteNames ++ newNames)
}
```
If you check `CTE in subquery shadows outer` test cases you will see that legacy and new results are the same. We shouldn't give `AnalysisException` in those cases.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582942543
**[Test build #117988 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117988/testReport)** for PR 27454 at commit [`c03920a`](https://github.com/apache/spark/commit/c03920a5bf4efc3e6867b5ddde120003bc63f0c5).
* 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582288433
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581967939
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117847/
Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375665518
##########
File path: docs/sql-migration-guide.md
##########
@@ -103,7 +103,7 @@ license: |
- Since Spark 3.0, if files or subdirectories disappear during recursive directory listing (i.e. they appear in an intermediate listing but then cannot be read or listed during later phases of the recursive directory listing, due to either concurrent file deletions or object store consistency issues) then the listing will fail with an exception unless `spark.sql.files.ignoreMissingFiles` is `true` (default `false`). In previous versions, these missing files or subdirectories would be ignored. Note that this change of behavior only applies during initial table file listing (or during `REFRESH TABLE`), not during query execution: the net change is that `spark.sql.files.ignoreMissingFiles` is now obeyed during table file listing / query planning, not only at query execution time.
- - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.
+ - Since Spark 3.0, Spark throws an AnalysisException if name conflict is detected in the nested WITH clause. It forces the users to choose the specific substitution order they wanted, which is controlled by `spark.sql.legacy.ctePrecedence.enabled`. If set to false, inner CTE definitions take precedence over outer definitions. For example, set the config to `false`, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `2`, while setting it to `true`, the result is `1` which is the behavior in version 2.4 and earlier.
Review comment:
`If set to false (which is recommended)`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582219859
**[Test build #117875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117875/testReport)** for PR 27454 at commit [`f75b26a`](https://github.com/apache/spark/commit/f75b26a5aeab71bdc0a4a2f4f6da7075f6b2f996).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582660083
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375666518
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
Review comment:
it's good if we can show the conflicting names.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583921178
Thanks all for the review.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583551262
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583551270
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/118039/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583422194
**[Test build #118039 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/118039/testReport)** for PR 27454 at commit [`7249404`](https://github.com/apache/spark/commit/72494043d5a431dbe024e7472b4df0e5d3b96352).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582562634
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22708/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582660088
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117944/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375667582
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
Review comment:
nit:
```
if (isLegacy.isEmpty) {
assertNoNameConflictsInCTE(...)
traverseAndSubstituteCTE
} else if ...
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582220153
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375666518
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
Review comment:
it's better if we can show the conflicting names.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581963973
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375667017
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
+ s"please select the CTE precedence via ${LEGACY_CTE_PRECEDENCE_ENABLED.key}, " +
+ "see more detail in SPARK-28228.")
+ } else {
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
}
}
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def detectConflictInNestedCTE(
+ plan: LogicalPlan,
+ inTraverse: Boolean,
+ cteNames: Set[String] = Set.empty): Boolean = {
+ plan.foreach {
+ case w @ With(child, relations) =>
+ val newNames = relations.map {
+ case (cteName, _) =>
+ if (cteNames.contains(cteName)) {
+ return true
Review comment:
we can throw exception here so that we know which name is conflicting.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582846891
**[Test build #117988 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117988/testReport)** for PR 27454 at commit [`c03920a`](https://github.com/apache/spark/commit/c03920a5bf4efc3e6867b5ddde120003bc63f0c5).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376172483
##########
File path: docs/sql-migration-guide.md
##########
@@ -103,7 +103,7 @@ license: |
- Since Spark 3.0, if files or subdirectories disappear during recursive directory listing (i.e. they appear in an intermediate listing but then cannot be read or listed during later phases of the recursive directory listing, due to either concurrent file deletions or object store consistency issues) then the listing will fail with an exception unless `spark.sql.files.ignoreMissingFiles` is `true` (default `false`). In previous versions, these missing files or subdirectories would be ignored. Note that this change of behavior only applies during initial table file listing (or during `REFRESH TABLE`), not during query execution: the net change is that `spark.sql.files.ignoreMissingFiles` is now obeyed during table file listing / query planning, not only at query execution time.
- - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.
+ - Since Spark 3.0, Spark throws an AnalysisException if name conflict is detected in the nested WITH clause. It forces the users to choose the specific substitution order they wanted, which is controlled by `spark.sql.legacy.ctePrecedence.enabled`. If set to false (which is recommended), inner CTE definitions take precedence over outer definitions. For example, set the config to `false`, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `2`, while setting it to `true`, the result is `1` which is the behavior in version 2.4 and earlier.
Review comment:
`Spark throws an AnalysisException ... by default.`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581968556
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375767693
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
+ s"please select the CTE precedence via ${LEGACY_CTE_PRECEDENCE_ENABLED.key}, " +
Review comment:
Thanks, done in c03920a.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375045228
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -17,24 +17,57 @@
package org.apache.spark.sql.catalyst.analysis
+import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, With}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.internal.SQLConf
-import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
/**
* Analyze WITH nodes and substitute child plan with CTE definitions.
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ if (SQLConf.get.legacyCTEPrecedenceEnabled.isEmpty) {
Review comment:
nit:
```
val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
if (isLegacy.isEmpty) ...
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582220154
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22637/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376165371
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,10 +29,49 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ assertNoNameConflictsInCTE(plan, inTraverse = false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ }
+
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def assertNoNameConflictsInCTE(
+ plan: LogicalPlan,
+ inTraverse: Boolean,
+ cteNames: Set[String] = Set.empty): Unit = {
+ plan.foreach {
+ case w @ With(child, relations) =>
+ val newNames = relations.map {
+ case (cteName, _) =>
+ if (cteNames.contains(cteName)) {
+ throw new AnalysisException(s"Name $cteName is conflict in nested CTE. " +
Review comment:
Ur, `is conflict`? `conflict` is a verb or a noun, isn't it? Shall we revise a little?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583183871
cc @peter-toth
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376426586
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2079,7 +2079,7 @@ object SQLConf {
.internal()
.doc("When true, outer CTE definitions takes precedence over inner definitions.")
Review comment:
Sure, done in 7249404.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583422773
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22804/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375768034
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
Review comment:
Thanks, done in c03920a.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582660083
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375045013
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2714,6 +2714,8 @@ class SQLConf extends Serializable with Logging {
def csvFilterPushDown: Boolean = getConf(CSV_FILTER_PUSHDOWN_ENABLED)
+ def legacyCTEPrecedenceEnabled: Option[Boolean] = getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
Review comment:
I think it's not encouraged to create a method now. We should call `getConf` directly.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582189655
**[Test build #117869 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117869/testReport)** for PR 27454 at commit [`53699d1`](https://github.com/apache/spark/commit/53699d168f1a048245253d89e46a3d1cb12cf713).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582219859
**[Test build #117875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117875/testReport)** for PR 27454 at commit [`f75b26a`](https://github.com/apache/spark/commit/f75b26a5aeab71bdc0a4a2f4f6da7075f6b2f996).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582189964
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582189970
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22631/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582561973
**[Test build #117944 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117944/testReport)** for PR 27454 at commit [`a55e82d`](https://github.com/apache/spark/commit/a55e82dc3275f6e877eacdceed2de8e55daa2f5d).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583686911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22819/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375044673
##########
File path: docs/sql-migration-guide.md
##########
@@ -103,7 +103,7 @@ license: |
- Since Spark 3.0, if files or subdirectories disappear during recursive directory listing (i.e. they appear in an intermediate listing but then cannot be read or listed during later phases of the recursive directory listing, due to either concurrent file deletions or object store consistency issues) then the listing will fail with an exception unless `spark.sql.files.ignoreMissingFiles` is `true` (default `false`). In previous versions, these missing files or subdirectories would be ignored. Note that this change of behavior only applies during initial table file listing (or during `REFRESH TABLE`), not during query execution: the net change is that `spark.sql.files.ignoreMissingFiles` is now obeyed during table file listing / query planning, not only at query execution time.
- - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.
+ - Since Spark 3.0, an AnalysisException will happen by default in case of nested WITH clauses, it forces the users to choose the specific substitution order they wanted, which is controlled by `spark.sql.legacy.ctePrecedence.enabled`. If set to false, inner CTE definitions take precedence over outer definitions. For example, set the config to `false`, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` returns `2`, while setting it to `true`, the result is `1` which is the behavior in version 2.4 and earlier.
Review comment:
Since Spark 3.0, Spark throws an AnalysisException if name conflict is detected in the nested WITH clause.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581964846
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22606/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375799960
##########
File path: sql/core/src/test/resources/sql-tests/inputs/cte-nonlegacy.sql
##########
@@ -0,0 +1,2 @@
+--SET spark.sql.legacy.ctePrecedence.enabled = false
+--IMPORT cte.sql
Review comment:
Copy that, will do it later.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583706064
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/118053/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375767892
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
+ s"please select the CTE precedence via ${LEGACY_CTE_PRECEDENCE_ENABLED.key}, " +
+ "see more detail in SPARK-28228.")
+ } else {
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
}
}
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def detectConflictInNestedCTE(
+ plan: LogicalPlan,
+ inTraverse: Boolean,
+ cteNames: Set[String] = Set.empty): Boolean = {
+ plan.foreach {
+ case w @ With(child, relations) =>
+ val newNames = relations.map {
+ case (cteName, _) =>
+ if (cteNames.contains(cteName)) {
+ return true
Review comment:
Thanks, done in c03920a.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376554744
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2077,9 +2077,11 @@ object SQLConf {
val LEGACY_CTE_PRECEDENCE_ENABLED = buildConf("spark.sql.legacy.ctePrecedence.enabled")
.internal()
- .doc("When true, outer CTE definitions takes precedence over inner definitions.")
+ .doc("When true, outer CTE definitions takes precedence over inner definitions. If set to " +
+ "false, inner CTE definitions take precedence. The default value is empty, which keep the " +
Review comment:
Actually, `keep the behavior` is not correct because the 2.4.x behavior is `true`. The default value is new behavior in 3.0, isn't 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375448403
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -17,24 +17,57 @@
package org.apache.spark.sql.catalyst.analysis
+import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, With}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.internal.SQLConf
-import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
/**
* Analyze WITH nodes and substitute child plan with CTE definitions.
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ if (SQLConf.get.legacyCTEPrecedenceEnabled.isEmpty) {
+ if (hasNestedCTE(plan, inTraverse = false)) {
Review comment:
Thanks for the advice, done in a55e82d and change tests correspondingly.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582847630
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582562626
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376426871
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,10 +29,49 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ assertNoNameConflictsInCTE(plan, inTraverse = false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ }
+
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def assertNoNameConflictsInCTE(
+ plan: LogicalPlan,
+ inTraverse: Boolean,
+ cteNames: Set[String] = Set.empty): Unit = {
+ plan.foreach {
+ case w @ With(child, relations) =>
+ val newNames = relations.map {
+ case (cteName, _) =>
+ if (cteNames.contains(cteName)) {
+ throw new AnalysisException(s"Name $cteName is conflict in nested CTE. " +
Review comment:
Thanks, rephrase it to `Name $name is ambiguous in nested CTE`. Done in 7249404.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582562626
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375767952
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
+ s"please select the CTE precedence via ${LEGACY_CTE_PRECEDENCE_ENABLED.key}, " +
+ "see more detail in SPARK-28228.")
+ } else {
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
}
}
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def detectConflictInNestedCTE(
Review comment:
Thanks, done in c03920a.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375767557
##########
File path: docs/sql-migration-guide.md
##########
@@ -103,7 +103,7 @@ license: |
- Since Spark 3.0, if files or subdirectories disappear during recursive directory listing (i.e. they appear in an intermediate listing but then cannot be read or listed during later phases of the recursive directory listing, due to either concurrent file deletions or object store consistency issues) then the listing will fail with an exception unless `spark.sql.files.ignoreMissingFiles` is `true` (default `false`). In previous versions, these missing files or subdirectories would be ignored. Note that this change of behavior only applies during initial table file listing (or during `REFRESH TABLE`), not during query execution: the net change is that `spark.sql.files.ignoreMissingFiles` is now obeyed during table file listing / query planning, not only at query execution time.
- - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.
+ - Since Spark 3.0, Spark throws an AnalysisException if name conflict is detected in the nested WITH clause. It forces the users to choose the specific substitution order they wanted, which is controlled by `spark.sql.legacy.ctePrecedence.enabled`. If set to false, inner CTE definitions take precedence over outer definitions. For example, set the config to `false`, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `2`, while setting it to `true`, the result is `1` which is the behavior in version 2.4 and earlier.
Review comment:
Thanks, done in c03920a.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582943465
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117988/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582943446
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583686663
**[Test build #118053 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/118053/testReport)** for PR 27454 at commit [`ebd337b`](https://github.com/apache/spark/commit/ebd337b9250b9a1426232fe16d5eda92e16b23c8).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582847630
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375448692
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -17,24 +17,57 @@
package org.apache.spark.sql.catalyst.analysis
+import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, With}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.internal.SQLConf
-import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
/**
* Analyze WITH nodes and substitute child plan with CTE definitions.
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ if (SQLConf.get.legacyCTEPrecedenceEnabled.isEmpty) {
Review comment:
Thanks, done in a55e82d.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582561973
**[Test build #117944 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117944/testReport)** for PR 27454 at commit [`a55e82d`](https://github.com/apache/spark/commit/a55e82dc3275f6e877eacdceed2de8e55daa2f5d).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583422773
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22804/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582210959
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582288443
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117875/
Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582220153
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581967927
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582288433
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582287656
**[Test build #117875 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117875/testReport)** for PR 27454 at commit [`f75b26a`](https://github.com/apache/spark/commit/f75b26a5aeab71bdc0a4a2f4f6da7075f6b2f996).
* This patch **fails due to an unknown error code, -9**.
* 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375448772
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2714,6 +2714,8 @@ class SQLConf extends Serializable with Logging {
def csvFilterPushDown: Boolean = getConf(CSV_FILTER_PUSHDOWN_ENABLED)
+ def legacyCTEPrecedenceEnabled: Option[Boolean] = getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
Review comment:
Thanks, deleted in a55e82d.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376426468
##########
File path: docs/sql-migration-guide.md
##########
@@ -103,7 +103,7 @@ license: |
- Since Spark 3.0, if files or subdirectories disappear during recursive directory listing (i.e. they appear in an intermediate listing but then cannot be read or listed during later phases of the recursive directory listing, due to either concurrent file deletions or object store consistency issues) then the listing will fail with an exception unless `spark.sql.files.ignoreMissingFiles` is `true` (default `false`). In previous versions, these missing files or subdirectories would be ignored. Note that this change of behavior only applies during initial table file listing (or during `REFRESH TABLE`), not during query execution: the net change is that `spark.sql.files.ignoreMissingFiles` is now obeyed during table file listing / query planning, not only at query execution time.
- - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.
+ - Since Spark 3.0, Spark throws an AnalysisException if name conflict is detected in the nested WITH clause. It forces the users to choose the specific substitution order they wanted, which is controlled by `spark.sql.legacy.ctePrecedence.enabled`. If set to false (which is recommended), inner CTE definitions take precedence over outer definitions. For example, set the config to `false`, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `2`, while setting it to `true`, the result is `1` which is the behavior in version 2.4 and earlier.
Review comment:
Thanks, done in 7249404.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375668308
##########
File path: sql/core/src/test/resources/sql-tests/inputs/cte-nonlegacy.sql
##########
@@ -0,0 +1,115 @@
+create temporary view t as select * from values 0, 1, 2 as t(id);
+create temporary view t2 as select * from values 0, 1 as t(id);
+
+-- CTE non-legacy substitution
+SET spark.sql.legacy.ctePrecedence.enabled=false;
+
Review comment:
we can use `--IMPORT cte.sql` to include the existing 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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581963987
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117844/
Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582210891
**[Test build #117869 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117869/testReport)** for PR 27454 at commit [`53699d1`](https://github.com/apache/spark/commit/53699d168f1a048245253d89e46a3d1cb12cf713).
* This patch **fails Spark unit 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581968572
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22609/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583686911
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22819/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582220154
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22637/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582943446
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583705968
**[Test build #118053 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/118053/testReport)** for PR 27454 at commit [`ebd337b`](https://github.com/apache/spark/commit/ebd337b9250b9a1426232fe16d5eda92e16b23c8).
* 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582562634
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22708/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] peter-toth commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
peter-toth commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376701418
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2077,9 +2077,11 @@ object SQLConf {
val LEGACY_CTE_PRECEDENCE_ENABLED = buildConf("spark.sql.legacy.ctePrecedence.enabled")
.internal()
- .doc("When true, outer CTE definitions takes precedence over inner definitions.")
+ .doc("When true, outer CTE definitions takes precedence over inner definitions. If set to " +
+ "false, inner CTE definitions take precedence. The default value is empty, which keep the " +
Review comment:
I'm not sure how exact and detailed this description needs to be, but
- if it is set to `false` then an inner CTE always takes precedence,
- if it is set to `true` then it complies with the (bit weird) 2.4 behaviour, where an outer CTE takes precedence in almost all of the cases (ie. inner CTE is in another CTE definition or in a subquery expression), except for the case where inner CTE is in a subquery.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582847641
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22753/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376166053
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2079,7 +2079,7 @@ object SQLConf {
.internal()
.doc("When true, outer CTE definitions takes precedence over inner definitions.")
Review comment:
Now, it became `three-state conf`. Shall we mentioned more about `false` and `empty`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375044785
##########
File path: docs/sql-migration-guide.md
##########
@@ -103,7 +103,7 @@ license: |
- Since Spark 3.0, if files or subdirectories disappear during recursive directory listing (i.e. they appear in an intermediate listing but then cannot be read or listed during later phases of the recursive directory listing, due to either concurrent file deletions or object store consistency issues) then the listing will fail with an exception unless `spark.sql.files.ignoreMissingFiles` is `true` (default `false`). In previous versions, these missing files or subdirectories would be ignored. Note that this change of behavior only applies during initial table file listing (or during `REFRESH TABLE`), not during query execution: the net change is that `spark.sql.files.ignoreMissingFiles` is now obeyed during table file listing / query planning, not only at query execution time.
- - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.
+ - Since Spark 3.0, an AnalysisException will happen by default in case of nested WITH clauses, it forces the users to choose the specific substitution order they wanted, which is controlled by `spark.sql.legacy.ctePrecedence.enabled`. If set to false, inner CTE definitions take precedence over outer definitions. For example, set the config to `false`, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` returns `2`, while setting it to `true`, the result is `1` which is the behavior in version 2.4 and earlier.
Review comment:
returns `1` returns `2` ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375045470
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -17,24 +17,57 @@
package org.apache.spark.sql.catalyst.analysis
+import org.apache.spark.sql.AnalysisException
import org.apache.spark.sql.catalyst.expressions.SubqueryExpression
import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, With}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.internal.SQLConf
-import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
/**
* Analyze WITH nodes and substitute child plan with CTE definitions.
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ if (SQLConf.get.legacyCTEPrecedenceEnabled.isEmpty) {
+ if (hasNestedCTE(plan, inTraverse = false)) {
Review comment:
we should only fail if there are name conflicts. It's a bad UX if we blindly forbid nested CTE by default.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582660088
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117944/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375667312
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
+ s"please select the CTE precedence via ${LEGACY_CTE_PRECEDENCE_ENABLED.key}, " +
+ "see more detail in SPARK-28228.")
+ } else {
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
}
}
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def detectConflictInNestedCTE(
Review comment:
nit: `assertNoNameConflictsInCTE`
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582210967
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117869/
Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582189655
**[Test build #117869 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117869/testReport)** for PR 27454 at commit [`53699d1`](https://github.com/apache/spark/commit/53699d168f1a048245253d89e46a3d1cb12cf713).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376426416
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,10 +29,49 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ assertNoNameConflictsInCTE(plan, inTraverse = false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ }
+
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def assertNoNameConflictsInCTE(
+ plan: LogicalPlan,
+ inTraverse: Boolean,
+ cteNames: Set[String] = Set.empty): Unit = {
+ plan.foreach {
+ case w @ With(child, relations) =>
+ val newNames = relations.map {
+ case (cteName, _) =>
+ if (cteNames.contains(cteName)) {
+ throw new AnalysisException(s"Name $cteName is conflict in nested CTE. " +
+ s"Please set ${LEGACY_CTE_PRECEDENCE_ENABLED.key} to false so that name defined " +
+ "in inner CTE takes precedence. See more details in SPARK-28228.")
+ } else {
+ cteName
+ }
+ }.toSet
+ (w.innerChildren :+ child).foreach { p =>
Review comment:
Thanks for the detailed checking! Yes, that makes sense, we should only give exception while legacy and non-legacy give different results. Done in 7249404
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376676128
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2077,9 +2077,11 @@ object SQLConf {
val LEGACY_CTE_PRECEDENCE_ENABLED = buildConf("spark.sql.legacy.ctePrecedence.enabled")
.internal()
- .doc("When true, outer CTE definitions takes precedence over inner definitions.")
+ .doc("When true, outer CTE definitions takes precedence over inner definitions. If set to " +
+ "false, inner CTE definitions take precedence. The default value is empty, which keep the " +
Review comment:
Yes, use `keep` here is ambiguous, thanks for pointing out! I want to express `always`.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583686663
**[Test build #118053 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/118053/testReport)** for PR 27454 at commit [`ebd337b`](https://github.com/apache/spark/commit/ebd337b9250b9a1426232fe16d5eda92e16b23c8).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581964832
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581968556
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581964846
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22606/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583551262
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r375767767
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,13 +29,55 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ if (detectConflictInNestedCTE(plan, inTraverse = false)) {
+ throw new AnalysisException("Name collision in nested CTE was founded in current plan, " +
Review comment:
Copy that, done in c03920a.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun closed pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582189964
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581967927
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] xuanyuanking commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
xuanyuanking commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581966365
retest this please.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582210967
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117869/
Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582189393
retest this please
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583422764
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582943465
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117988/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582288443
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117875/
Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583686905
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583706064
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/118053/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] peter-toth commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
peter-toth commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376279947
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CTESubstitution.scala
##########
@@ -28,10 +29,49 @@ import org.apache.spark.sql.internal.SQLConf.LEGACY_CTE_PRECEDENCE_ENABLED
*/
object CTESubstitution extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = {
- if (SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)) {
+ val isLegacy = SQLConf.get.getConf(LEGACY_CTE_PRECEDENCE_ENABLED)
+ if (isLegacy.isEmpty) {
+ assertNoNameConflictsInCTE(plan, inTraverse = false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ } else if (isLegacy.get) {
legacyTraverseAndSubstituteCTE(plan)
} else {
- traverseAndSubstituteCTE(plan, false)
+ traverseAndSubstituteCTE(plan, inTraverse = false)
+ }
+ }
+
+ /**
+ * Check the plan to be traversed has naming conflicts in nested CTE or not, traverse through
+ * child, innerChildren and subquery for the current plan.
+ */
+ private def assertNoNameConflictsInCTE(
+ plan: LogicalPlan,
+ inTraverse: Boolean,
+ cteNames: Set[String] = Set.empty): Unit = {
+ plan.foreach {
+ case w @ With(child, relations) =>
+ val newNames = relations.map {
+ case (cteName, _) =>
+ if (cteNames.contains(cteName)) {
+ throw new AnalysisException(s"Name $cteName is conflict in nested CTE. " +
+ s"Please set ${LEGACY_CTE_PRECEDENCE_ENABLED.key} to false so that name defined " +
+ "in inner CTE takes precedence. See more details in SPARK-28228.")
+ } else {
+ cteName
+ }
+ }.toSet
+ (w.innerChildren :+ child).foreach { p =>
Review comment:
This could be:
```
child.transformExpressions {
case e: SubqueryExpression =>
assertNoNameConflictsInCTE(e.plan, inTraverse = true, cteNames ++ newNames)
e
}
w.innerChildren.foreach { p =>
assertNoNameConflictsInCTE(p, inTraverse = true, cteNames ++ newNames)
}
```
If you check `CTE in subquery shadows outer` test cases you will see that legacy (https://github.com/apache/spark/blob/master/sql/core/src/test/resources/sql-tests/results/cte-legacy.sql.out#L113-L151) and new results (https://github.com/apache/spark/blob/master/sql/core/src/test/resources/sql-tests/results/cte.sql.out#L248-L286) are the same. We shouldn't give `AnalysisException` in those cases.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581967939
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/117847/
Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582846891
**[Test build #117988 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117988/testReport)** for PR 27454 at commit [`c03920a`](https://github.com/apache/spark/commit/c03920a5bf4efc3e6867b5ddde120003bc63f0c5).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582189970
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22631/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582659509
**[Test build #117944 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/117944/testReport)** for PR 27454 at commit [`a55e82d`](https://github.com/apache/spark/commit/a55e82dc3275f6e877eacdceed2de8e55daa2f5d).
* 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #27454: [SPARK-28228][SQL]
Change the default behavior for name conflict in nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582847641
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22753/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583422194
**[Test build #118039 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/118039/testReport)** for PR 27454 at commit [`7249404`](https://github.com/apache/spark/commit/72494043d5a431dbe024e7472b4df0e5d3b96352).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581964832
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-581968572
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/22609/
Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-583422764
Merged build finished. Test PASSed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
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 issue #27454:
[SPARK-28228][SQL] Change the default behavior for nested WITH clause
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #27454: [SPARK-28228][SQL] Change the default behavior for nested WITH clause
URL: https://github.com/apache/spark/pull/27454#issuecomment-582210959
Merged build finished. Test FAILed.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] viirya commented on a change in pull request #27454:
[SPARK-28228][SQL] Change the default behavior for name conflict in nested
WITH clause
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #27454: [SPARK-28228][SQL] Change the default behavior for name conflict in nested WITH clause
URL: https://github.com/apache/spark/pull/27454#discussion_r376171885
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala
##########
@@ -2079,7 +2079,7 @@ object SQLConf {
.internal()
.doc("When true, outer CTE definitions takes precedence over inner definitions.")
Review comment:
Sounds we should.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org