[SPARK-52236][SQL] Standardize analyze exceptions for default value #50960
Conversation
| } | ||
| } | ||
| } | ||
|
|
||
| case cmd: AddColumns if cmd.columnsToAdd.exists(_.default.isDefined) => | ||
| // Wrap analysis errors for default values in a more user-friendly message. | ||
| val statement = "ALTER TABLE ADD COLUMNS" |
There was a problem hiding this comment.
This now matches messages like: https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala#L298
| cmd.specs.foreach { c => | ||
| c.newDefaultExpression.foreach { d => | ||
| // Eagerly check resolved, as accessing datatype requires it to be resolved |
There was a problem hiding this comment.
the AlterColumnSpec object doesnt have a type (unlike the others). If i dont eagerly check here check if d is resolved, the d.dataType call to get the argument for validateDefaultValueExpr() itself throws another UnresolvedError, in the cases where the default value is an unresvoled expression like 'badvalue'
There was a problem hiding this comment.
how about we make the targetType parameter of def validateDefaultValueExpr to be Option[DataType]? For AlterColumns command, we don't really need to check type castability, as we use the expression's own data type as the target type.
There was a problem hiding this comment.
makes sense, done
| @@ -3576,7 +3612,7 @@ class DataSourceV2SQLSuiteV1Filter | |||
| s"REPLACE TABLE tab (col1 DOUBLE DEFAULT rand()) USING $v2Source") | |||
| assert(exception.getSqlState == "42623") | |||
| assert(exception.errorClass.get == "INVALID_DEFAULT_VALUE.NON_DETERMINISTIC") | |||
| assert(exception.messageParameters("statement") == "CREATE TABLE") | |||
| assert(exception.messageParameters("statement") == "REPLACE TABLE") | |||
There was a problem hiding this comment.
This looks like an error message bug fix, right?
There was a problem hiding this comment.
yea , my mistake before :)
| @@ -1170,7 +1170,7 @@ class InsertSuite extends DataSourceTest with SharedSparkSession { | |||
| exception = intercept[AnalysisException] { | |||
| sql("create table t(i boolean, s bigint default badvalue) using parquet") | |||
| }, | |||
| condition = "INVALID_DEFAULT_VALUE.NOT_CONSTANT", | |||
| condition = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION", | |||
What changes were proposed in this pull request?
Default values are checked for various things like resolved, and deterministic. Standardize the errors for various statements (CREATE TABLE/REPLACE TABLE/ALTER TABLE ADD COLUMNS/ALTER TABLE ALTER COLUMNS). This pr just fixes the ones that were not standard before.
Why are the changes needed?
Cleanup some exceptions and messages. This is from the comment of https://github.com/apache/spark/pull/50864/files#r2087361821
Does this PR introduce any user-facing change?
No
How was this patch tested?
Add test in DatasourceV2SQLSuite
Was this patch authored or co-authored using generative AI tooling?