LuciferYang commented on code in PR #59545:
URL: https://github.com/apache/spark/pull/59545#discussion_r4346101704


##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/CheckConstraintSuite.scala:
##########
@@ -170,6 +170,60 @@ class CheckConstraintSuite extends QueryTest with 
CommandSuiteBase with DDLComma
     }
   }
 
+  test("SPARK-60032: Check constraint added by ALTER TABLE reading the current 
time") {
+    // The check compares with the time of the write, not the time of the 
ALTER TABLE.
+    Seq(
+      ("TIMESTAMP", "current_timestamp()", "current_timestamp()", "INTERVAL 1 
DAY"),
+      ("TIMESTAMP", "CAST('now' AS TIMESTAMP)", "current_timestamp()", 
"INTERVAL 1 DAY"),
+      ("TIMESTAMP", "to_timestamp('now')", "current_timestamp()", "INTERVAL 1 
DAY"),
+      ("TIMESTAMP_NTZ", "CAST('now' AS TIMESTAMP_NTZ)", "localtimestamp()", 
"INTERVAL 1 DAY"),
+      ("DATE", "current_date()", "current_date()", "1"),
+      ("DATE", "CAST('today' AS DATE)", "current_date()", "1")
+    ).foreach { case (dataType, now, current, step) =>
+      Seq("true", "false").foreach { ansi =>
+        withSQLConf(SQLConf.ANSI_ENABLED.key -> ansi) {
+          withNamespaceAndTable("ns", "tbl", nonPartitionCatalog) { t =>
+            sql(s"CREATE TABLE $t (id INT, created $dataType) $defaultUsing")
+            sql(s"ALTER TABLE $t ADD CONSTRAINT valid_time CHECK (created <= 
$now)")

Review Comment:
   Confirmed for `current_date() > DATE'2023-01-01'` and `1 > 0`, both fail 
with INTERNAL_ERROR. Filed SPARK-60455.
   



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/constraints.scala:
##########
@@ -120,16 +122,32 @@ case class CheckConstraint(
     condition: String,
     override val userProvidedName: String = null,
     override val tableName: String = null,
-    override val userProvidedCharacteristic: ConstraintCharacteristic = 
ConstraintCharacteristic.empty)
+    override val userProvidedCharacteristic: ConstraintCharacteristic = 
ConstraintCharacteristic.empty,
+    analyzedChild: Option[Expression] = None)

Review Comment:
   Done in e2322baf889: the field is now `readsCurrentContext: Boolean`, 
computed in `markAsAnalyzed`, and the `stringArgs` override is gone.
   



##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/CheckConstraintSuite.scala:
##########
@@ -170,6 +170,60 @@ class CheckConstraintSuite extends QueryTest with 
CommandSuiteBase with DDLComma
     }
   }
 
+  test("SPARK-60032: Check constraint added by ALTER TABLE reading the current 
time") {
+    // The check compares with the time of the write, not the time of the 
ALTER TABLE.
+    Seq(
+      ("TIMESTAMP", "current_timestamp()", "current_timestamp()", "INTERVAL 1 
DAY"),
+      ("TIMESTAMP", "CAST('now' AS TIMESTAMP)", "current_timestamp()", 
"INTERVAL 1 DAY"),
+      ("TIMESTAMP", "to_timestamp('now')", "current_timestamp()", "INTERVAL 1 
DAY"),
+      ("TIMESTAMP_NTZ", "CAST('now' AS TIMESTAMP_NTZ)", "localtimestamp()", 
"INTERVAL 1 DAY"),
+      ("DATE", "current_date()", "current_date()", "1"),
+      ("DATE", "CAST('today' AS DATE)", "current_date()", "1")
+    ).foreach { case (dataType, now, current, step) =>
+      Seq("true", "false").foreach { ansi =>
+        withSQLConf(SQLConf.ANSI_ENABLED.key -> ansi) {
+          withNamespaceAndTable("ns", "tbl", nonPartitionCatalog) { t =>
+            sql(s"CREATE TABLE $t (id INT, created $dataType) $defaultUsing")
+            sql(s"ALTER TABLE $t ADD CONSTRAINT valid_time CHECK (created <= 
$now)")
+
+            sql(s"INSERT INTO $t VALUES (1, $current)")
+            checkAnswer(spark.table(t).select("id"), Seq(Row(1)))
+
+            val error = intercept[SparkRuntimeException] {
+              sql(s"INSERT INTO $t VALUES (2, $current + $step)")
+            }
+            assert(error.getCondition == "CHECK_CONSTRAINT_VIOLATION")
+            assert(error.getMessageParameters.get("constraintName") == 
"valid_time")
+            assert(error.getMessageParameters.get("expression") == s"created 
<= $now")
+          }
+        }
+      }
+    }
+  }
+
+  test("SPARK-60032: Check constraint added by ALTER TABLE on a column 
qualified by the table") {
+    // ALTER TABLE resolves `tbl.id` as the column `id` of the table `tbl`, 
not as the field `id` of
+    // its struct column `tbl`. A write that parses `predicateSql` reads 
`tbl.id` as the field, so
+    // the test fails if this constraint is stored without a predicate.
+    withNamespaceAndTable("ns", "tbl", nonPartitionCatalog) { t =>
+      sql(s"CREATE TABLE $t (id BIGINT, tbl STRUCT<id: BIGINT>) $defaultUsing")
+      sql(s"ALTER TABLE $t ADD CONSTRAINT c1 CHECK (nullif(tbl.id, 3) > 0)")

Review Comment:
   Confirmed: with a row `id = 5` and a view over `id > 50`, the ALTER TABLE 
adds `c` to the base table. Filed SPARK-60454.
   



-- 
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]

Reply via email to