LuciferYang commented on code in PR #59545:
URL: https://github.com/apache/spark/pull/59545#discussion_r4346092312
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/constraints.scala:
##########
@@ -142,6 +160,17 @@ case class CheckConstraint(
.build()
}
+ /**
+ * Whether the rules that compute the current time, date, catalog and the
like change part of `e`,
+ * applied to `e` as the optimizer has it, with its `RuntimeReplaceable`s
replaced.
+ */
+ private def readsCurrentContext(e: Expression): Boolean = {
+ val replaced = ReplaceExpressions.replace(e)
+ replaced.containsPattern(CURRENT_LIKE) ||
+ (ComputeCurrentTime.applyForExpression(replaced) ne replaced) ||
+ (SpecialDatetimeValues.applyForExpression(replaced) ne replaced)
Review Comment:
Thanks, fixed in e2322baf889. The check now counts only 'now', 'today',
'tomorrow' and 'yesterday', through a new `DateTimeUtils.isCurrentSpecialValue`
around `extractSpecialValue`, instead of checking whether
`SpecialDatetimeValues` changes the tree. The new test "Check constraint added
by ALTER TABLE on a fixed datetime" asserts that `CAST('epoch' AS TIMESTAMP)`,
`'epoch'` and a full datetime string keep their predicate, and that writes
check `tbl.created >= 'epoch'` instead of failing with UNRESOLVED_COLUMN.
`DateTimeUtilsSuite` covers the helper itself.
##########
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"),
Review Comment:
Done in e2322baf889. After each ALTER TABLE the test now asserts
`predicateSql` and that `predicate()` is null, so every row now fails if the
predicate is frozen, whatever the date. I also added `CAST(TIME
'23:59:59.999999' AS TIMESTAMP)`, which fails when the `ComputeCurrentTime`
term is removed.
##########
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)
extends UnaryExpression
with TableConstraint
- with ImplicitCastInputTypes {
+ with ImplicitCastInputTypes
+ with AnalysisAwareExpression[CheckConstraint] {
// scalastyle:on line.size.limit
+ final override val nodePatterns: Seq[TreePattern] =
Seq(ANALYSIS_AWARE_EXPRESSION)
+
override def inputTypes: Seq[AbstractDataType] = Seq(BooleanType)
+ override def stringArgs: Iterator[Any] =
+ Iterator(child, condition, userProvidedName, tableName,
userProvidedCharacteristic)
+
+ override def markAsAnalyzed(): CheckConstraint = copy(analyzedChild =
Some(child))
+
+ // ALTER TABLE builds the constraint from the optimized plan, where the
current time, date,
+ // catalog, user and the like are replaced by their values at the time of
the ALTER TABLE. A
+ // condition that reads one is stored without a predicate, so that writes
check `predicateSql`.
def toV2Constraint: Constraint = {
- val predicate = new V2ExpressionBuilder(child,
true).buildPredicate().orNull
+ val predicate = if (analyzedChild.exists(readsCurrentContext)) {
Review Comment:
Confirmed: under ANSI the CREATE TABLE fails with CAST_INVALID_INPUT, and
with ANSI off it stores `d <= NULL` and accepts `DATE'9999-12-31'`. Since the
cause is the `contextIndependentFoldable` issue in your next comment, I filed
both as SPARK-60451, covering CREATE and REPLACE TABLE, and linked it from the
description. The comment above `toV2Constraint` now says that CREATE and
REPLACE TABLE convert their constraints before analysis finishes.
--
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]
