LuciferYang commented on code in PR #59545:
URL: https://github.com/apache/spark/pull/59545#discussion_r4346094229
##########
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 = {
Review Comment:
Right, the parser has already folded them, so this PR cannot see them: `DATE
'today'` is stored as the date of the ALTER TABLE. The description now says
typed literals are not covered, and SPARK-60452 tracks recording them in the
parser, including the case where the rest of the condition decides.
##########
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) ||
Review Comment:
Fixed in e2322baf889: a string with only a time part cast to TIMESTAMP now
counts as reading the current date (`DateTimeUtils.isTimeOnlyTimestamp`), and
the test has a `CAST('23:59:59.999999' AS TIMESTAMP)` row, which fails without
it.
##########
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`.
Review Comment:
Confirmed: with the legacy flag every later INSERT failed with
UNBOUND_SQL_PARAMETER, while before this PR the frozen predicate let it pass.
Fixed in e2322baf889: `visitCheckConstraint` records whether the condition has
a marker, and such a condition keeps the predicate built from the optimized
condition, as before. The new test "Check constraint added by ALTER TABLE with
a parameter marker" runs with the flag on and off. I didn't reject markers
outright, since that would also reject `CHECK (id > :m)`, which works in that
mode today because its predicate is stored as `id > 0`. CREATE TABLE already
fails the same way on master, so SPARK-60457 covers that and whether to reject
markers as DEFAULT does.
--
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]
