[SPARK-51903][SQL] Validate data on adding a CHECK constraint #50839
+238
−37
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @aokolnychyi |
cloud-fan
reviewed
May 9, 2025
| * The logical plan of the ALTER TABLE ... ADD CONSTRAINT command for Check constraints. | ||
| */ | ||
| case class AddCheckConstraint( | ||
| override val table: LogicalPlan, |
There was a problem hiding this comment.
this is not really a table, but a validation query plan.
There was a problem hiding this comment.
We should either not extend AlterTableCommand for AddCheckConstraint, or have both the table and validationPlan fields.
There was a problem hiding this comment.
The current code is fragile as we have code like the following
case a: AlterTableCommand if a.table.resolved && hasUnresolvedFieldName(a) =>
val table = a.table.asInstanceOf[ResolvedTable]
There was a problem hiding this comment.
Updated.
The code in CheckAnalysis looks a bit ugly now.
Do we consider remove the final from
trait AlterTableCommand extends UnaryCommand {
def changes: Seq[TableChange]
def table: LogicalPlan
final override def child: LogicalPlan = table
}
aokolnychyi
reviewed
May 9, 2025
| @@ -4120,6 +4120,12 @@ | |||
| ], | |||
| "sqlState" : "07501" | |||
| }, | |||
| "NEW_CHECK_CONSTRAINT_VIOLATION" : { | |||
| "message" : [ | |||
| "The new check constraint (<expression>) is violated by the existing data in the table <tableName>." | |||
There was a problem hiding this comment.
I would reword to something like The check constraint (<expression>) cannot be added because ....
| case _ => | ||
| null | ||
| } | ||
| Seq(TableChange.addConstraint(constraint, validatedTableVersion)) |
There was a problem hiding this comment.
Do we set validation status to VALIDATED?
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
This PR adds support for enforcing CHECK constraints when executing the
ALTER TABLE ... ADD CONSTRAINTstatement on V2 tables. Specifically, before adding a new CHECK constraint, Spark will validate the constraint against existing table data to ensure data integrity and compliance with the constraint definition.Why are the changes needed?
CHECK constraint enforcement in the
ALTER TABLE ... ADD CONSTRAINTstatement is part of the ANSI SQL standard. Supporting this feature ensures Spark's compliance with ANSI SQL and enhances data integrity by preventing invalid constraints from being added to existing tables.Does this PR introduce any user-facing change?
No
How was this patch tested?
New UT
Was this patch authored or co-authored using generative AI tooling?
No