-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-39497][SQL] Improve the analysis exception of missing map key column #36896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
f1dd983
00f8814
90a4fb3
c292150
9231eac
7bc90a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,6 +91,27 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog { | |
| } | ||
| } | ||
|
|
||
| private def isMapWithStringKey(e: Expression): Boolean = if (e.resolved) { | ||
| e.dataType match { | ||
| case m: MapType => m.keyType.isInstanceOf[StringType] | ||
| case _ => false | ||
| } | ||
| } else { | ||
| false | ||
| } | ||
|
|
||
| private def failUnresolvedAttribute( | ||
| operator: LogicalPlan, | ||
| a: Attribute, | ||
| errorClass: String): Nothing = { | ||
| val unresolvedAttribute = a.sql | ||
| val candidates = operator.inputSet.toSeq.map(_.qualifiedName) | ||
| val orderedCandidates = StringUtils.orderStringsBySimilarity(unresolvedAttribute, candidates) | ||
| a.failAnalysis( | ||
| errorClass = errorClass, | ||
| messageParameters = Array(unresolvedAttribute, orderedCandidates.mkString(", "))) | ||
| } | ||
|
|
||
| def checkAnalysis(plan: LogicalPlan): Unit = { | ||
| // We transform up and order the rules so as to catch the first possible failure instead | ||
| // of the result of cascading resolution failures. Inline all CTEs in the plan to help check | ||
|
|
@@ -160,28 +181,28 @@ trait CheckAnalysis extends PredicateHelper with LookupCatalog { | |
| throw QueryCompilationErrors.commandUnsupportedInV2TableError("SHOW TABLE EXTENDED") | ||
|
|
||
| case operator: LogicalPlan => | ||
| // Check argument data types of higher-order functions downwards first. | ||
| // If the arguments of the higher-order functions are resolved but the type check fails, | ||
| // the argument functions will not get resolved, but we should report the argument type | ||
| // check failure instead of claiming the argument functions are unresolved. | ||
| operator transformExpressionsDown { | ||
| // Check argument data types of higher-order functions downwards first. | ||
| // If the arguments of the higher-order functions are resolved but the type check fails, | ||
| // the argument functions will not get resolved, but we should report the argument type | ||
| // check failure instead of claiming the argument functions are unresolved. | ||
| case hof: HigherOrderFunction | ||
| if hof.argumentsResolved && hof.checkArgumentDataTypes().isFailure => | ||
| hof.checkArgumentDataTypes() match { | ||
| case TypeCheckResult.TypeCheckFailure(message) => | ||
| hof.failAnalysis( | ||
| s"cannot resolve '${hof.sql}' due to argument data type mismatch: $message") | ||
| } | ||
|
|
||
| // If an attribute can't be resolved as a map key of string type, either the key should be | ||
| // surrounded with single quotes, or there is a typo in the attribute name. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to match |
||
| case GetMapValue(map, key: Attribute, _) if isMapWithStringKey(map) && !key.resolved => | ||
| failUnresolvedAttribute(operator, key, "UNRESOLVED_MAP_KEY") | ||
| } | ||
|
|
||
| getAllExpressions(operator).foreach(_.foreachUp { | ||
| case a: Attribute if !a.resolved => | ||
| val missingCol = a.sql | ||
| val candidates = operator.inputSet.toSeq.map(_.qualifiedName) | ||
| val orderedCandidates = StringUtils.orderStringsBySimilarity(missingCol, candidates) | ||
| a.failAnalysis( | ||
| errorClass = "MISSING_COLUMN", | ||
| messageParameters = Array(missingCol, orderedCandidates.mkString(", "))) | ||
| failUnresolvedAttribute(operator, a, "MISSING_COLUMN") | ||
|
|
||
| case s: Star => | ||
| withPosition(s) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -401,6 +401,17 @@ class QueryCompilationErrorsSuite | |
| ) | ||
| } | ||
|
|
||
| test("UNRESOLVED_MAP_KEY: string type literal should be quoted") { | ||
| checkAnswer(sql("select m['a'] from (select map('a', 'b') as m, 'aa' as aa)"), Row("b")) | ||
| checkError( | ||
| exception = intercept[AnalysisException] { | ||
| sql("select m[a] from (select map('a', 'b') as m, 'aa' as aa)") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. m.a is equivalent to m['a'] so it won't fail analysis |
||
| }, | ||
| errorClass = "UNRESOLVED_MAP_KEY", | ||
| parameters = Map("columnName" -> "a", | ||
| "proposal" -> "__auto_generated_subquery_name.m, __auto_generated_subquery_name.aa")) | ||
| } | ||
|
|
||
| test("MISSING_COLUMN: SELECT distinct does not work correctly " + | ||
| "if order by missing attribute") { | ||
| checkAnswer( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both candidates and unresolvedAttribute should be wrapped with toSQLId
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I will update this one after #36891 is merged.