Skip to content

Commit d5f735b

Browse files
committed
[SPARK-51905][SQL] Disallow NOT ENFORCED CHECK constraint
### What changes were proposed in this pull request? As we plan to enforce check constraints by default, it remains unclear what the behavior should be when it is "NOT ENFORCED". - Option 1: Never support NOVALIDATE/NOT VALID in ALTER TABLE. Use ENFORCED/NOT ENFORCED to check if validation must be done. - Option 2: Add NOVALIDATE/NOT VALID to control this. - Option 3: Don't support of "NOT ENFORCED" for now. Let's think about Option 1 or Option 2 later I would prefer option 3. We can make a decision later based on the user expectations. ### Why are the changes needed? Clarify the behavior of a "NOT ENFORCED" CHECK constraint. ### Does this PR introduce _any_ user-facing change? No, this is DSV2 framework and it is not released yet. ### How was this patch tested? New UT ### Was this patch authored or co-authored using generative AI tooling? No Closes #50772 from gengliangwang/disallowNotEnforcedCheck. Authored-by: Gengliang Wang <[email protected]> Signed-off-by: Gengliang Wang <[email protected]>
1 parent 633419a commit d5f735b

File tree

7 files changed

+123
-15
lines changed

7 files changed

+123
-15
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/constraints.scala

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,20 @@ case class CheckConstraint(
150150

151151
override def withTableName(tableName: String): TableConstraint = copy(tableName = tableName)
152152

153-
override def withUserProvidedCharacteristic(c: ConstraintCharacteristic): TableConstraint =
153+
override def withUserProvidedCharacteristic(c: ConstraintCharacteristic): TableConstraint = {
154+
if (c.enforced.contains(false)) {
155+
val origin = CurrentOrigin.get
156+
throw new ParseException(
157+
command = origin.sqlText,
158+
start = origin,
159+
errorClass = "UNSUPPORTED_CONSTRAINT_CHARACTERISTIC",
160+
messageParameters = Map(
161+
"characteristic" -> "NOT ENFORCED",
162+
"constraintType" -> "CHECK")
163+
)
164+
}
154165
copy(userProvidedCharacteristic = c)
166+
}
155167
}
156168

157169
// scalastyle:off line.size.limit

sql/core/src/test/scala/org/apache/spark/sql/execution/command/CheckConstraintParseSuite.scala

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,4 +316,87 @@ class CheckConstraintParseSuite extends ConstraintParseSuiteBase {
316316
}
317317
}
318318

319+
test("NOT ENFORCED is not supported for CHECK -- table level") {
320+
notEnforcedConstraintCharacteristics.foreach { case (c1, c2, _) =>
321+
val characteristic = if (c2.isEmpty) {
322+
c1
323+
} else {
324+
s"$c1 $c2"
325+
}
326+
val sql =
327+
s"""
328+
|CREATE TABLE a.b.t (a INT, b STRING, CONSTRAINT C1 CHECK (a > 0) $characteristic)
329+
|""".stripMargin
330+
331+
val expectedContext = ExpectedContext(
332+
fragment = s"CONSTRAINT C1 CHECK (a > 0) $characteristic"
333+
)
334+
335+
checkError(
336+
exception = intercept[ParseException] {
337+
parsePlan(sql)
338+
},
339+
condition = "UNSUPPORTED_CONSTRAINT_CHARACTERISTIC",
340+
parameters = Map(
341+
"characteristic" -> "NOT ENFORCED",
342+
"constraintType" -> "CHECK"),
343+
queryContext = Array(expectedContext))
344+
}
345+
}
346+
347+
test("NOT ENFORCED is not supported for CHECK -- column level") {
348+
notEnforcedConstraintCharacteristics.foreach { case (c1, c2, _) =>
349+
val characteristic = if (c2.isEmpty) {
350+
c1
351+
} else {
352+
s"$c1 $c2"
353+
}
354+
val sql =
355+
s"""
356+
|CREATE TABLE a.b.t (a INT CHECK (a > 0) $characteristic, b STRING)
357+
|""".stripMargin
358+
359+
val expectedContext = ExpectedContext(
360+
fragment = s"CHECK (a > 0) $characteristic"
361+
)
362+
363+
checkError(
364+
exception = intercept[ParseException] {
365+
parsePlan(sql)
366+
},
367+
condition = "UNSUPPORTED_CONSTRAINT_CHARACTERISTIC",
368+
parameters = Map(
369+
"characteristic" -> "NOT ENFORCED",
370+
"constraintType" -> "CHECK"),
371+
queryContext = Array(expectedContext))
372+
}
373+
}
374+
375+
test("NOT ENFORCED is not supported for CHECK -- ALTER TABLE") {
376+
notEnforcedConstraintCharacteristics.foreach { case (c1, c2, _) =>
377+
val characteristic = if (c2.isEmpty) {
378+
c1
379+
} else {
380+
s"$c1 $c2"
381+
}
382+
val sql =
383+
s"""
384+
|ALTER TABLE a.b.t ADD CONSTRAINT C1 CHECK (a > 0) $characteristic
385+
|""".stripMargin
386+
387+
val expectedContext = ExpectedContext(
388+
fragment = s"CONSTRAINT C1 CHECK (a > 0) $characteristic"
389+
)
390+
391+
checkError(
392+
exception = intercept[ParseException] {
393+
parsePlan(sql)
394+
},
395+
condition = "UNSUPPORTED_CONSTRAINT_CHARACTERISTIC",
396+
parameters = Map(
397+
"characteristic" -> "NOT ENFORCED",
398+
"constraintType" -> "CHECK"),
399+
queryContext = Array(expectedContext))
400+
}
401+
}
319402
}

sql/core/src/test/scala/org/apache/spark/sql/execution/command/ConstraintParseSuiteBase.scala

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,8 @@ import org.apache.spark.sql.types.{IntegerType, StringType}
2626
abstract class ConstraintParseSuiteBase extends AnalysisTest with SharedSparkSession {
2727
protected def validConstraintCharacteristics = Seq(
2828
("", "", ConstraintCharacteristic(enforced = None, rely = None)),
29-
("NOT ENFORCED", "", ConstraintCharacteristic(enforced = Some(false), rely = None)),
3029
("", "RELY", ConstraintCharacteristic(enforced = None, rely = Some(true))),
31-
("", "NORELY", ConstraintCharacteristic(enforced = None, rely = Some(false))),
32-
("NOT ENFORCED", "RELY",
33-
ConstraintCharacteristic(enforced = Some(false), rely = Some(true))),
34-
("NOT ENFORCED", "NORELY",
35-
ConstraintCharacteristic(enforced = Some(false), rely = Some(false)))
30+
("", "NORELY", ConstraintCharacteristic(enforced = None, rely = Some(false)))
3631
)
3732

3833
protected def enforcedConstraintCharacteristics = Seq(
@@ -43,6 +38,19 @@ abstract class ConstraintParseSuiteBase extends AnalysisTest with SharedSparkSes
4338
("NORELY", "ENFORCED", ConstraintCharacteristic(enforced = Some(true), rely = Some(false)))
4439
)
4540

41+
protected def notEnforcedConstraintCharacteristics = Seq(
42+
("NOT ENFORCED", "RELY",
43+
ConstraintCharacteristic(enforced = Some(false), rely = Some(true))),
44+
("NOT ENFORCED", "NORELY",
45+
ConstraintCharacteristic(enforced = Some(false), rely = Some(false))),
46+
("RELY", "NOT ENFORCED",
47+
ConstraintCharacteristic(enforced = Some(false), rely = Some(true))),
48+
("NORELY", "NOT ENFORCED",
49+
ConstraintCharacteristic(enforced = Some(false), rely = Some(false))),
50+
("NOT ENFORCED", "",
51+
ConstraintCharacteristic(enforced = Some(false), rely = None))
52+
)
53+
4654
protected val invalidConstraintCharacteristics = Seq(
4755
("ENFORCED", "ENFORCED"),
4856
("ENFORCED", "NOT ENFORCED"),

sql/core/src/test/scala/org/apache/spark/sql/execution/command/ForeignKeyConstraintParseSuite.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import org.apache.spark.sql.catalyst.parser.ParseException
2323
import org.apache.spark.sql.catalyst.plans.logical.AddConstraint
2424

2525
class ForeignKeyConstraintParseSuite extends ConstraintParseSuiteBase {
26+
27+
override val validConstraintCharacteristics =
28+
super.validConstraintCharacteristics ++ notEnforcedConstraintCharacteristics
29+
2630
test("Create table with foreign key - table level") {
2731
val sql = "CREATE TABLE t (a INT, b STRING," +
2832
" FOREIGN KEY (a) REFERENCES parent(id)) USING parquet"

sql/core/src/test/scala/org/apache/spark/sql/execution/command/PrimaryKeyConstraintParseSuite.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import org.apache.spark.sql.catalyst.parser.ParseException
2424
import org.apache.spark.sql.catalyst.plans.logical.AddConstraint
2525

2626
class PrimaryKeyConstraintParseSuite extends ConstraintParseSuiteBase {
27+
override val validConstraintCharacteristics =
28+
super.validConstraintCharacteristics ++ notEnforcedConstraintCharacteristics
2729

2830
test("Create table with primary key - table level") {
2931
val sql = "CREATE TABLE t (a INT, b STRING, PRIMARY KEY (a)) USING parquet"

sql/core/src/test/scala/org/apache/spark/sql/execution/command/UniqueConstraintParseSuite.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import org.apache.spark.sql.catalyst.parser.ParseException
2323
import org.apache.spark.sql.catalyst.plans.logical.AddConstraint
2424

2525
class UniqueConstraintParseSuite extends ConstraintParseSuiteBase {
26+
override val validConstraintCharacteristics =
27+
super.validConstraintCharacteristics ++ notEnforcedConstraintCharacteristics
28+
2629
test("Create table with unnamed unique constraint") {
2730
Seq(
2831
"CREATE TABLE t (a INT, b STRING, UNIQUE (a)) USING parquet",

sql/core/src/test/scala/org/apache/spark/sql/execution/command/v2/CheckConstraintSuite.scala

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,15 +143,11 @@ class CheckConstraintSuite extends QueryTest with CommandSuiteBase with DDLComma
143143
val validStatus = "UNVALIDATED"
144144
Seq(
145145
("", s"ENFORCED $validStatus NORELY"),
146-
("NOT ENFORCED", s"NOT ENFORCED $validStatus NORELY"),
147-
("NOT ENFORCED NORELY", s"NOT ENFORCED $validStatus NORELY"),
148-
("NORELY NOT ENFORCED", s"NOT ENFORCED $validStatus NORELY"),
149146
("NORELY", s"ENFORCED $validStatus NORELY"),
150-
("NOT ENFORCED RELY", s"NOT ENFORCED $validStatus RELY"),
151-
("RELY NOT ENFORCED", s"NOT ENFORCED $validStatus RELY"),
152-
("NOT ENFORCED RELY", s"NOT ENFORCED $validStatus RELY"),
153-
("RELY NOT ENFORCED", s"NOT ENFORCED $validStatus RELY"),
154-
("RELY", s"ENFORCED $validStatus RELY")
147+
("RELY", s"ENFORCED $validStatus RELY"),
148+
("ENFORCED", s"ENFORCED $validStatus NORELY"),
149+
("ENFORCED NORELY", s"ENFORCED $validStatus NORELY"),
150+
("ENFORCED RELY", s"ENFORCED $validStatus RELY")
155151
)
156152
}
157153

0 commit comments

Comments
 (0)