This is an automated email from the ASF dual-hosted git repository.
gengliangwang pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 41e9d5f4a536 [SPARK-57642][SQL] Require predicateSql to be present for
the DSv2 CHECK constraint
41e9d5f4a536 is described below
commit 41e9d5f4a5364e7ba83b82f43f3f084d7db35d80
Author: Gengliang Wang <[email protected]>
AuthorDate: Wed Jun 24 05:48:37 2026 -0700
[SPARK-57642][SQL] Require predicateSql to be present for the DSv2 CHECK
constraint
### What changes were proposed in this pull request?
This PR makes `predicateSql` a mandatory field of the DSv2 `Check`
constraint (`org.apache.spark.sql.connector.catalog.constraints.Check`).
Previously, `Check.Builder.build()` only rejected the case where **both**
`predicateSql` and `predicate` were `null`, which allowed a `Check` to be
constructed with only a structured `predicate`. This PR tightens the validation
so that `predicateSql` must always be provided. `predicate` remains optional
and is the structured form used when the condition can be expressed with
supported expressions.
Specifically:
- `Check.Builder.build()` now throws when `predicateSql` is `null`,
regardless of `predicate`.
- `Check.definition()` is simplified to always render `predicateSql` (the
previous fallback to `predicate` is dead code now that `predicateSql` is
guaranteed to be present).
- Javadoc on the class, `predicateSql()`, and `predicate()` is updated to
document that `predicateSql` is the canonical representation and is always
present, while `predicate` is optional and may be `null`.
### Why are the changes needed?
`predicateSql` is the canonical representation of a CHECK condition. Spark
always populates it from the original SQL text in
`CheckConstraint.toV2Constraint`, while `predicate` is only set when the
condition can be translated to a supported `Predicate` (it is `null` otherwise,
e.g. for `from_json(j, 'a INT').a > 1`).
Several read paths already assume `predicateSql` is present. For example,
`ResolveTableConstraints.buildCatalystExpression` prefers the structured
`predicate` but falls back to parsing `predicateSql`:
```scala
Option(c.predicate())
.flatMap(V2ExpressionUtils.toCatalyst)
.getOrElse(catalogManager.v1SessionCatalog.parser.parseExpression(c.predicateSql()))
```
If a connector were to build a `Check` with a `null` `predicateSql` and a
`predicate` that cannot be converted back by `V2ExpressionUtils.toCatalyst`,
this would fall through to `parseExpression(null)` and fail with an NPE.
`predicateSql` is also used as the human-readable condition in CHECK violation
error messages. Requiring `predicateSql` makes the invariant explicit and keeps
these paths safe.
### Does this PR introduce _any_ user-facing change?
No. `Check` is an `Evolving` DSv2 API, and Spark itself always sets
`predicateSql`, so no existing Spark behavior changes. The only effect is
tighter validation for connector authors who construct `Check` directly:
building a `Check` without `predicateSql` now fails fast with a clear error
instead of producing a constraint that downstream code already assumes is
invalid.
### How was this patch tested?
Updated `ConstraintSuite`:
- The existing "CHECK constraint toDDL" `con2` case now also supplies
`predicateSql` (it previously relied on the predicate-only path).
- Added "CHECK constraint requires predicateSql", asserting that `build()`
fails with `INTERNAL_ERROR` when `predicateSql` is absent, both when no
condition is supplied at all and when only a `predicate` is supplied.
```
build/sbt 'catalyst/testOnly *ConstraintSuite'
```
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
Closes #56711 from gengliangwang/spark-57642.
Authored-by: Gengliang Wang <[email protected]>
Signed-off-by: Gengliang Wang <[email protected]>
(cherry picked from commit 1f185c09bf26d4b68fde698b01f0cb504f887f28)
Signed-off-by: Gengliang Wang <[email protected]>
---
.../sql/connector/catalog/constraints/Check.java | 27 ++++++++++++-------
.../sql/connector/catalog/ConstraintSuite.scala | 31 +++++++++++++++++-----
2 files changed, 41 insertions(+), 17 deletions(-)
diff --git
a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/constraints/Check.java
b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/constraints/Check.java
index ae005d946694..5addd4b09842 100644
---
a/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/constraints/Check.java
+++
b/sql/catalyst/src/main/java/org/apache/spark/sql/connector/catalog/constraints/Check.java
@@ -27,12 +27,13 @@ import
org.apache.spark.sql.connector.expressions.filter.Predicate;
/**
* A CHECK constraint.
* <p>
- * A CHECK constraint defines a condition each row in a table must satisfy.
Connectors can define
- * such constraints either in SQL (Spark SQL dialect) or using a {@link
Predicate predicate} if the
- * condition can be expressed using a supported expression. A CHECK constraint
can reference one or
- * more columns. Such constraint is considered violated if its condition
evaluates to {@code FALSE},
- * but not {@code NULL}. The search condition must be deterministic and cannot
contain subqueries
- * and certain functions like aggregates or UDFs.
+ * A CHECK constraint defines a condition each row in a table must satisfy.
The condition is always
+ * represented as a SQL string (Spark SQL dialect), accessible via {@link
#predicateSql()}, and is
+ * additionally exposed as a {@link Predicate predicate} via {@link
#predicate()} whenever it can be
+ * expressed using supported expressions. A CHECK constraint can reference one
or more columns. Such
+ * constraint is considered violated if its condition evaluates to {@code
FALSE}, but not
+ * {@code NULL}. The search condition must be deterministic and cannot contain
subqueries and
+ * certain functions like aggregates or UDFs.
* <p>
* Spark supports enforced and not enforced CHECK constraints, allowing
connectors to control
* whether data modifications that violate the constraint must fail. Each
constraint is either
@@ -63,13 +64,19 @@ public class Check extends BaseConstraint {
/**
* Returns the SQL representation of the search condition (Spark SQL
dialect).
+ * <p>
+ * This is the canonical representation of the condition and is always
present (never
+ * {@code null}). The optional {@link #predicate()} provides a structured
form when the condition
+ * can be expressed using supported {@link Predicate} expressions.
*/
public String predicateSql() {
return predicateSql;
}
/**
- * Returns the search condition.
+ * Returns the search condition as a {@link Predicate}, or {@code null} if
the condition cannot be
+ * expressed using supported predicate expressions. Use {@link
#predicateSql()} for the canonical
+ * SQL representation, which is always present.
*/
public Predicate predicate() {
return predicate;
@@ -77,7 +84,7 @@ public class Check extends BaseConstraint {
@Override
protected String definition() {
- return String.format("CHECK (%s)", predicateSql != null ? predicateSql :
predicate);
+ return String.format("CHECK (%s)", predicateSql);
}
@Override
@@ -123,10 +130,10 @@ public class Check extends BaseConstraint {
}
public Check build() {
- if (predicateSql == null && predicate == null) {
+ if (predicateSql == null) {
throw new SparkIllegalArgumentException(
"INTERNAL_ERROR",
- Map.of("message", "Predicate SQL and expression can't be both null
in CHECK"));
+ Map.of("message", "Predicate SQL can't be null in CHECK"));
}
return new Check(name(), predicateSql, predicate, enforced(),
validationStatus(), rely());
}
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/ConstraintSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/ConstraintSuite.scala
index d63e3095a2ef..2902bef2cda0 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/ConstraintSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/connector/catalog/ConstraintSuite.scala
@@ -17,7 +17,7 @@
package org.apache.spark.sql.connector.catalog
-import org.apache.spark.SparkFunSuite
+import org.apache.spark.{SparkFunSuite, SparkIllegalArgumentException}
import org.apache.spark.sql.connector.catalog.constraints.Constraint
import
org.apache.spark.sql.connector.catalog.constraints.Constraint.ValidationStatus
import org.apache.spark.sql.connector.expressions.{Expression, FieldReference,
LiteralValue, NamedReference}
@@ -37,12 +37,13 @@ class ConstraintSuite extends SparkFunSuite {
assert(con1.validationStatus() == ValidationStatus.VALID)
val con2 = Constraint.check("con2")
- .predicate(
- new Predicate(
- "=",
- Array[Expression](
- FieldReference(Seq("a", "b.c", "d")),
- LiteralValue(1, IntegerType))))
+ .predicateSql("a.`b.c`.d = 1")
+ .predicate(
+ new Predicate(
+ "=",
+ Array[Expression](
+ FieldReference(Seq("a", "b.c", "d")),
+ LiteralValue(1, IntegerType))))
.enforced(false)
.validationStatus(ValidationStatus.VALID)
.rely(true)
@@ -70,6 +71,22 @@ class ConstraintSuite extends SparkFunSuite {
assert(con4.validationStatus() == ValidationStatus.UNVALIDATED)
}
+ test("CHECK constraint requires predicateSql") {
+ // predicateSql is the canonical representation of a CHECK condition and
must always be present,
+ // even when a structured predicate is provided.
+ val noCondition = Constraint.check("con1")
+ val predicateOnly = Constraint.check("con2").predicate(
+ new Predicate(
+ "=",
+ Array[Expression](FieldReference(Seq("a")), LiteralValue(1,
IntegerType))))
+ Seq(noCondition, predicateOnly).foreach { builder =>
+ checkError(
+ exception = intercept[SparkIllegalArgumentException](builder.build()),
+ condition = "INTERNAL_ERROR",
+ parameters = Map("message" -> "Predicate SQL can't be null in CHECK"))
+ }
+ }
+
test("UNIQUE constraint toDDL") {
val con1 = Constraint.unique(
"con1",
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
