You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "johanl-db (via GitHub)" <gi...@apache.org> on 2023/05/12 13:55:16 UTC

[GitHub] [spark] johanl-db opened a new pull request, #41155: [SPARK-43487] Fix Nested CTE error message

johanl-db opened a new pull request, #41155:
URL: https://github.com/apache/spark/pull/41155

   
   ### What changes were proposed in this pull request?
   The batch of errors migrated to error classes as part of spark-40540 contains an error that got mixed up with the wrong error message:
   
   [ambiguousRelationAliasNameInNestedCTEError](https://github.com/apache/spark/commit/43a6b932759865c45ccf36f3e9cf6898c1b762da#diff-744ac13f6fe074fddeab09b407404bffa2386f54abc83c501e6e1fe618f6db56R1983) uses the same error message as the following commandUnsupportedInV2TableError:
   
   WITH t AS (SELECT 1), t2 AS ( WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2;
   AnalysisException: t is not supported for v2 tables
   
   The error should be:
   
   AnalysisException: Name tis ambiguous in nested CTE.
   Please set spark.sql.legacy.ctePrecedencePolicy to CORRECTED so that name defined in inner CTE takes precedence. If set it to LEGACY, outer CTE definitions will take precedence. See more details in SPARK-28228.
   
   ### Does this PR introduce _any_ user-facing change?
   Fix user-facing error message for ambiguous name in nested CTEs.
   
   
   ### How was this patch tested?
   


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on a diff in pull request #41155: [SPARK-43487][SQL] Fix Nested CTE error message

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #41155:
URL: https://github.com/apache/spark/pull/41155#discussion_r1192953587


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -3233,6 +3225,14 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
       messageParameters = Map.empty)
   }
 
+  def ambiguousRelationAliasNameInNestedCTEError(name: String): Throwable = {
+    new AnalysisException(
+      errorClass = "_LEGACY_ERROR_TEMP_1348",
+      messageParameters = Map(
+        "name" -> name,

Review Comment:
   Please, wrap it by `toSQLId`, please.



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -3783,6 +3783,12 @@
       "Failed to execute command because subquery expressions are not allowed in DEFAULT values."
     ]
   },
+  "_LEGACY_ERROR_TEMP_1348" : {
+    "message" : [
+      "Name <name> is ambiguous in nested CTE.",
+      "Please set <config> to CORRECTED so that name defined in inner CTE takes precedence. If set it to LEGACY, outer CTE definitions will take precedence. See more details in SPARK-28228."

Review Comment:
   > See more details in SPARK-28228.
   
   It would be better to update the public docs and refer to them.



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -3233,6 +3225,14 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
       messageParameters = Map.empty)
   }
 
+  def ambiguousRelationAliasNameInNestedCTEError(name: String): Throwable = {
+    new AnalysisException(
+      errorClass = "_LEGACY_ERROR_TEMP_1348",
+      messageParameters = Map(
+        "name" -> name,
+        "config" -> LEGACY_CTE_PRECEDENCE_POLICY.key))

Review Comment:
   Use `toSQLConf`



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -3783,6 +3783,12 @@
       "Failed to execute command because subquery expressions are not allowed in DEFAULT values."
     ]
   },
+  "_LEGACY_ERROR_TEMP_1348" : {
+    "message" : [
+      "Name <name> is ambiguous in nested CTE.",
+      "Please set <config> to CORRECTED so that name defined in inner CTE takes precedence. If set it to LEGACY, outer CTE definitions will take precedence. See more details in SPARK-28228."

Review Comment:
   ```suggestion
         "Please set <config> to \"CORRECTED\" so that name defined in inner CTE takes precedence. If set it to \"LEGACY\", outer CTE definitions will take 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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] johanl-db commented on pull request #41155: [SPARK-43487][SQL] Fix Nested CTE error message

Posted by "johanl-db (via GitHub)" <gi...@apache.org>.
johanl-db commented on PR #41155:
URL: https://github.com/apache/spark/pull/41155#issuecomment-1557081376

   > @johanl-db Are you working on the PR? Could you, please, address the comments above.
   
   I was away the past few days, I'm picking this up now. I addressed your comments, please take another look.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk closed pull request #41155: [SPARK-43487][SQL] Fix Nested CTE error message

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk closed pull request #41155: [SPARK-43487][SQL] Fix Nested CTE error message
URL: https://github.com/apache/spark/pull/41155


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] johanl-db commented on a diff in pull request #41155: [SPARK-43487][SQL] Fix Nested CTE error message

Posted by "johanl-db (via GitHub)" <gi...@apache.org>.
johanl-db commented on code in PR #41155:
URL: https://github.com/apache/spark/pull/41155#discussion_r1200406987


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -3233,6 +3225,14 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
       messageParameters = Map.empty)
   }
 
+  def ambiguousRelationAliasNameInNestedCTEError(name: String): Throwable = {
+    new AnalysisException(
+      errorClass = "_LEGACY_ERROR_TEMP_1348",
+      messageParameters = Map(
+        "name" -> name,

Review Comment:
   👍 



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala:
##########
@@ -3233,6 +3225,14 @@ private[sql] object QueryCompilationErrors extends QueryErrorsBase {
       messageParameters = Map.empty)
   }
 
+  def ambiguousRelationAliasNameInNestedCTEError(name: String): Throwable = {
+    new AnalysisException(
+      errorClass = "_LEGACY_ERROR_TEMP_1348",
+      messageParameters = Map(
+        "name" -> name,
+        "config" -> LEGACY_CTE_PRECEDENCE_POLICY.key))

Review Comment:
   👍 



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -3783,6 +3783,12 @@
       "Failed to execute command because subquery expressions are not allowed in DEFAULT values."
     ]
   },
+  "_LEGACY_ERROR_TEMP_1348" : {

Review Comment:
   Renamed to `AMBIGUOUS_ALIAS_IN_NESTED_CTE`



##########
core/src/main/resources/error/error-classes.json:
##########
@@ -3783,6 +3783,12 @@
       "Failed to execute command because subquery expressions are not allowed in DEFAULT values."
     ]
   },
+  "_LEGACY_ERROR_TEMP_1348" : {
+    "message" : [
+      "Name <name> is ambiguous in nested CTE.",
+      "Please set <config> to CORRECTED so that name defined in inner CTE takes precedence. If set it to LEGACY, outer CTE definitions will take precedence. See more details in SPARK-28228."

Review Comment:
   I linked to the existing documentation that covers this configuration in the migration guide: https://spark.apache.org/docs/latest/sql-migration-guide.html#query-engine



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on a diff in pull request #41155: [SPARK-43487] Fix Nested CTE error message

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on code in PR #41155:
URL: https://github.com/apache/spark/pull/41155#discussion_r1192431888


##########
core/src/main/resources/error/error-classes.json:
##########
@@ -3783,6 +3783,12 @@
       "Failed to execute command because subquery expressions are not allowed in DEFAULT values."
     ]
   },
+  "_LEGACY_ERROR_TEMP_1348" : {

Review Comment:
   Could you assign proper name to the error class, please. The `_LEGACY_ERROR_TEMP_` prefix was used for migration purpose in batch way to switch to error classes in all existing exceptions.



-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on pull request #41155: [SPARK-43487][SQL] Fix Nested CTE error message

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #41155:
URL: https://github.com/apache/spark/pull/41155#issuecomment-1550794232

   @johanl-db Are you working on the PR? Could you, please, address the comments above.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] [spark] MaxGekk commented on pull request #41155: [SPARK-43487][SQL] Fix Nested CTE error message

Posted by "MaxGekk (via GitHub)" <gi...@apache.org>.
MaxGekk commented on PR #41155:
URL: https://github.com/apache/spark/pull/41155#issuecomment-1557963875

   +1, LGTM. Merging to master.
   Thank you, @johanl-db.


-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org