Skip to content

[SPARK-39783][SQL] Quote qualifiedName to fix backticks for column candidates in error messages #38256

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.

Sign up for GitHub

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

Closed
EnricoMi wants to merge 6 commits into from
Closed

[SPARK-39783][SQL] Quote qualifiedName to fix backticks for column candidates in error messages #38256

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f59e9df
Fix backticks for column candidates in error message
EnricoMi Oct 14, 2022
1d8a919
Adjust expected proposal to correct one
EnricoMi Oct 14, 2022
dc80706
Use checkError and errorClassTest
EnricoMi Oct 14, 2022
cef2711
Make ResolveUserSpecifiedColumns use qualifiedName for candidates
EnricoMi Oct 14, 2022
f919303
Test for UNRESOLVED_MAP_KEY.WITH_SUGGESTION
EnricoMi Oct 14, 2022
b9c9907
Update documentation of qualifiedName
EnricoMi Oct 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ trait NamedExpression extends Expression {
* multiple qualifiers, it is possible that there are other possible way to refer to this
* attribute.
*/
def qualifiedName: String = (qualifier :+ name).mkString(".")
def qualifiedName: String = (qualifier :+ name).map(quoteIfNeeded).mkString(".")

/**
* Optional qualifier for the expression.
Expand Down
18 changes: 18 additions & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,24 @@ class AnalysisErrorSuite extends AnalysisTest {
"[`a`, `b`, `c`, `d`, `e`]"
:: Nil)

errorTest(
Copy link
Member

@MaxGekk MaxGekk Oct 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you invoke errorClassTest(). This will allow to make the test independent from error message text, so, tech editors could edit error-classes.json and don't depend on Spark's tests.

Copy link
Contributor Author

@EnricoMi EnricoMi Oct 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

"SPARK-39783: backticks in error message for candidate column with dots",
// This selects a column that does not exist,
// the error message suggest the existing column with correct backticks
testRelation6.select($"`the`.`id`"),
"[UNRESOLVED_COLUMN.WITH_SUGGESTION] A column or function parameter with name `the`.`id` " +
"cannot be resolved. Did you mean one of the following? [`the.id`]"
:: Nil)

errorTest(
"SPARK-39783: backticks in error message for candidate struct column",
// This selects a column that does not exist,
// the error message suggest the existing column with correct backticks
nestedRelation2.select($"`top.aField`"),
"[UNRESOLVED_COLUMN.WITH_SUGGESTION] A column or function parameter with name `top.aField` " +
"cannot be resolved. Did you mean one of the following? [`top`]"
:: Nil)

test("SPARK-35080: Unsupported correlated equality predicates in subquery") {
val a = AttributeReference("a", IntegerType)()
val b = AttributeReference("b", IntegerType)()
Expand Down
2 changes: 2 additions & 0 deletions sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/TestRelations.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ object TestRelations {

val testRelation5 = LocalRelation(AttributeReference("i", StringType)())

val testRelation6 = LocalRelation(AttributeReference("the.id", LongType)())

val nestedRelation = LocalRelation(
AttributeReference("top", StructType(
StructField("duplicateField", StringType) ::
Expand Down
27 changes: 27 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/DatasetSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1992,6 +1992,33 @@ class DatasetSuite extends QueryTest
}
}

test("SPARK-39783: Fix error messages for columns with dots/periods") {
forAll(dotColumnTestModes) { (caseSensitive, colName) =>
val ds = Seq(SpecialCharClass("1", "2")).toDS
withSQLConf(SQLConf.CASE_SENSITIVE.key -> caseSensitive) {
val errorMsg = intercept[AnalysisException] {
// Note: ds(colName) has different semantics than ds.select(colName)
ds.select(colName)
}
assert(errorMsg.getMessage.contains(
Copy link
Member

@MaxGekk MaxGekk Oct 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, use checkError().

Copy link
Contributor Author

@EnricoMi EnricoMi Oct 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

s"A column or function parameter with name `${colName.replace(".", "`.`")}` " +
s"cannot be resolved. Did you mean one of the following? [`field.1`, `field 2`]"))
}
}
}

test("SPARK-39783: backticks in error message for candidate column with dots") {
checkError(
exception = intercept[AnalysisException] {
Seq(0).toDF("the.id").select("the.id")
},
errorClass = "UNRESOLVED_COLUMN.WITH_SUGGESTION",
sqlState = None,
parameters = Map(
"objectName" -> "`the`.`id`",
"proposal" -> "`the.id`"))
}

test("groupBy.as") {
val df1 = Seq(DoubleData(1, "one"), DoubleData(2, "two"), DoubleData(3, "three")).toDS()
.repartition($"id").sortWithinPartitions("id")
Expand Down