You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "Hisoka-X (via GitHub)" <gi...@apache.org> on 2023/09/05 04:10:18 UTC

[GitHub] [spark] Hisoka-X commented on a diff in pull request #42810: [SPARK-45075][SQL] Fix alter table with invalid default value will not report error

Hisoka-X commented on code in PR #42810:
URL: https://github.com/apache/spark/pull/42810#discussion_r1315339168


##########
sql/core/src/test/scala/org/apache/spark/sql/connector/AlterTableTests.scala:
##########
@@ -363,6 +363,35 @@ trait AlterTableTests extends SharedSparkSession with QueryErrorsBase {
     }
   }
 
+  test("SPARK-45075: ALTER COLUMN with invalid default value") {
+    withSQLConf(SQLConf.DEFAULT_COLUMN_ALLOWED_PROVIDERS.key -> s"$v2Format, ") {
+      withTable("t") {
+        sql(s"create table t(i boolean) using $v2Format")
+        // The default value fails to analyze.
+        checkError(
+          exception = intercept[AnalysisException] {
+            sql("alter table t add column s bigint default badvalue")
+          },
+          errorClass = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION",
+          parameters = Map(
+            "statement" -> "ALTER TABLE",
+            "colName" -> "`s`",
+            "defaultValue" -> "badvalue"))

Review Comment:
   This test case will report error normally before this PR, but we don't have any test case to prove it.



##########
sql/core/src/test/scala/org/apache/spark/sql/connector/AlterTableTests.scala:
##########
@@ -363,6 +363,35 @@ trait AlterTableTests extends SharedSparkSession with QueryErrorsBase {
     }
   }
 
+  test("SPARK-45075: ALTER COLUMN with invalid default value") {
+    withSQLConf(SQLConf.DEFAULT_COLUMN_ALLOWED_PROVIDERS.key -> s"$v2Format, ") {
+      withTable("t") {
+        sql(s"create table t(i boolean) using $v2Format")
+        // The default value fails to analyze.
+        checkError(
+          exception = intercept[AnalysisException] {
+            sql("alter table t add column s bigint default badvalue")
+          },
+          errorClass = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION",
+          parameters = Map(
+            "statement" -> "ALTER TABLE",
+            "colName" -> "`s`",
+            "defaultValue" -> "badvalue"))
+
+        sql("alter table t add column s bigint default 3L")
+        checkError(
+          exception = intercept[AnalysisException] {
+            sql("alter table t alter column s set default badvalue")
+          },
+          errorClass = "INVALID_DEFAULT_VALUE.UNRESOLVED_EXPRESSION",
+          parameters = Map(
+            "statement" -> "ALTER TABLE ALTER COLUMN",
+            "colName" -> "`s`",
+            "defaultValue" -> "badvalue"))

Review Comment:
   Add this test case to prove the new change worked. But the more negative case in `sql/core/src/test/scala/org/apache/spark/sql/sources/InsertSuite.scala` and only worked for V1, after this PR merged, I will move it to `sql/core/src/test/scala/org/apache/spark/sql/SQLInsertTestSuite.scala` in #42802 



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org