[SPARK-41062][SQL] Rename UNSUPPORTED_CORRELATED_REFERENCE to CORRELATED_REFERENCE
#38576
Conversation
| }, | ||
| "CORRELATED_REFERENCE" : { | ||
| "message" : [ | ||
| "Expressions referencing the outer query are not supported outside of WHERE/HAVING clauses<treeNode>" |
There was a problem hiding this comment.
Let's don't confuse users by treeNode, and show something more useful like problematic SQL expr.
There was a problem hiding this comment.
Agreed, let's show the expression. (having the expression in the context would also be acceptable). No treenode
There was a problem hiding this comment.
@MaxGekk May I ask if there is any way to extract the SQL expr from LogicalPlan ??
I'm trying to switch planToString(p) to SQL expression but still not sure how.
There was a problem hiding this comment.
Expressions referencing the outer query ...
Can you show the expression?
There was a problem hiding this comment.
Yes, just fixed it to show SQL expression!
| }, | ||
| "CORRELATED_REFERENCE" : { | ||
| "message" : [ | ||
| "Expressions referencing the outer query are not supported outside of WHERE/HAVING clauses<treeNode>" |
There was a problem hiding this comment.
Agreed, let's show the expression. (having the expression in the context would also be acceptable). No treenode
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
Outdated
Show resolved
Hide resolved
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
Outdated
Show resolved
Hide resolved
| val exprs = new ListBuffer[String]() | ||
| for (expr <- p.expressions) { |
There was a problem hiding this comment.
This can be simpler using filter instead of exists and for
There was a problem hiding this comment.
Thanks! Just addressed the comments
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveSubquerySuite.scala
Outdated
Show resolved
Hide resolved
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala
Outdated
Show resolved
Hide resolved
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/ResolveSubquerySuite.scala
Outdated
Show resolved
Hide resolved
| errorClass = "UNSUPPORTED_SUBQUERY_EXPRESSION_CATEGORY.CORRELATED_REFERENCE", | ||
| parameters = Map("sqlExprs" -> "outer(arr_c2#427264)"), | ||
| sqlState = None, | ||
| parameters = Map("sqlExprs" -> "explode\\(outer\\(arr_c2#.*"), |
There was a problem hiding this comment.
I squeeze out the outer expression here, but seems like this shows explode(arr_c2) with current behavior of stripOuterReference.
@MaxGekk WDYT? Is there any way, or should we fix the stripOuterReference to display the t2.arr_c2 here?
There was a problem hiding this comment.
should we fix the stripOuterReference to display the t2.arr_c2 here?
No, let's fix it separately.
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala
Outdated
Show resolved
Hide resolved
Co-authored-by: allisonwang-db <[email protected]>
|
+1, LGTM. Merging to master. |
4 participants
What changes were proposed in this pull request?
This PR proposes to rename
UNSUPPORTED_CORRELATED_REFERENCEtoCORRELATED_REFERENCE.Also, show
sqlExprsrather thantreeNodewhich is more useful information to users.Why are the changes needed?
The sub-error class name is duplicated with its main class,
UNSUPPORTED_SUBQUERY_EXPRESSION_CATEGORY.We should make the all error class name clear and briefly.
Does this PR introduce any user-facing change?
No
How was this patch tested?