MaxGekk commented on code in PR #39501:
URL: https://github.com/apache/spark/pull/39501#discussion_r1096563857
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -738,12 +744,24 @@
],
"sqlState" : "42K05"
},
+ "INVALID_EXTRACT_BASE_FIELD_TYPE" : {
+ "message" : [
+ "Can't extract a value from <base>. Need a complex type [struct, array,
map] but got <other>."
Review Comment:
Please, upper case the complex types to be consistent w/ other places, and
w/ the function `toSQLType()`.
##########
sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala:
##########
@@ -1097,7 +1097,7 @@ class ColumnExpressionSuite extends QueryTest with
SharedSparkSession {
nullable = false))))
structLevel2.withColumn("a", $"a".withField("a.b", lit(2)))
- }.getMessage should include("Ambiguous reference to fields")
+ }.getMessage should include("Ambiguous reference to the field `a`")
Review Comment:
Please, use `checkError`, and don't check the error message. The idea of
moving error formats to `error-classes.json` is to allow tech editors modify
error messages w/o changing tests.
##########
sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala:
##########
@@ -2211,7 +2211,7 @@ class ColumnExpressionSuite extends QueryTest with
SharedSparkSession {
intercept[AnalysisException] {
sql("SELECT named_struct('a', named_struct('b', 1), 'a',
named_struct('c', 2)) struct_col")
.select($"struct_col".dropFields("a.c"))
- }.getMessage should include("Ambiguous reference to fields")
+ }.getMessage should include("Ambiguous reference to the field `a`")
Review Comment:
Don't check error message, use `checkError` instead of it.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala:
##########
@@ -372,13 +372,13 @@ class AnalysisErrorSuite extends AnalysisTest {
errorTest(
Review Comment:
Please, migrate on `errorClassTest()`.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisErrorSuite.scala:
##########
@@ -372,13 +372,13 @@ class AnalysisErrorSuite extends AnalysisTest {
errorTest(
"ambiguous field",
nestedRelation.select($"top.duplicateField"),
- "Ambiguous reference to fields" :: "duplicateField" :: Nil,
+ "Ambiguous reference to the field" :: "duplicateField" :: "2" :: Nil,
caseSensitive = false)
errorTest(
Review Comment:
Use `errorClassTest()`.
##########
sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala:
##########
@@ -1872,7 +1872,7 @@ class ColumnExpressionSuite extends QueryTest with
SharedSparkSession {
nullable = false))))
structLevel2.withColumn("a", $"a".dropFields("a.b"))
- }.getMessage should include("Ambiguous reference to fields")
+ }.getMessage should include("Ambiguous reference to the field `a`")
Review Comment:
use `checkError`
##########
sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala:
##########
@@ -1650,7 +1650,7 @@ class ColumnExpressionSuite extends QueryTest with
SharedSparkSession {
intercept[AnalysisException] {
sql("SELECT named_struct('a', named_struct('b', 1), 'a',
named_struct('c', 2)) struct_col")
.select($"struct_col".withField("a.c", lit(3)))
- }.getMessage should include("Ambiguous reference to fields")
+ }.getMessage should include("Ambiguous reference to the field `a`")
Review Comment:
ditto: use `checkError`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
