[FLINK-40477][table] Fix constraint enforcer for partial deletes - #29067
Merged
fhueske merged 2 commits intoSep 3, 2026
Conversation
The sink NOT NULL enforcer inspected every row regardless of RowKind, so a by-key (partial) delete tombstone, whose non-key columns are legitimately null, was rejected (ERROR) or silently dropped (DROP), losing the delete. Relax NOT NULL for the non-key columns of DELETE rows in key-only-delete pipelines; key columns and all other row kinds stay enforced. Co-Generated: Claude Opus 4.8 (1M context)
fhueske
marked this pull request as ready for review
September 2, 2026 08:01
Collaborator
gustavodemorais
approved these changes
Sep 3, 2026
gustavodemorais
left a comment
gustavodemorais
left a comment
Contributor
There was a problem hiding this comment.
Change looks easier than I expected tbh. Small blast radius and makes sense. LGTM. Only added two nits
Comment on lines
+942
to
+944
| // Delete-by-key: a by-key delete legitimately carries null in its non-key columns ("regardless | ||
| // of nullability constraints"). The constraint enforcer only checks not-null key columns for | ||
| // by-key delete messages and ignores all value columns. |
Contributor
There was a problem hiding this comment.
Currently, it's a bit of a mix in the value columns/there are no guarantees. Suggestionm
Suggested change
| // Delete-by-key: a by-key delete legitimately carries null in its non-key columns ("regardless | |
| // of nullability constraints"). The constraint enforcer only checks not-null key columns for | |
| // by-key delete messages and ignores all value columns. | |
| // Delete-by-key: a by-key delete might carry null in its non-key columns, regardless | |
| // of nullability constraints. The constraint enforcer should only check key columns for | |
| // by-key delete messages and ignores all value columns. |
| .mapToObj(idx -> fieldNames[idx]) | ||
| .toArray(String[]::new); | ||
|
|
||
| final Set<Integer> primaryKeySet = |
Contributor
There was a problem hiding this comment.
Rest of the class uses BitSet for the same pattern (see buildCouldPad). Suggestion: swap it instead of Set, cheaper and consistent with the file
fhueske
deleted the
fhueske-FLINK-40477-Fix-Constraint-Enforcer-for-partial-deletes
branch
September 3, 2026 10:52
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 is the purpose of the change
The sink NOT NULL enforcer inspected every row regardless of RowKind, so a by-key (partial) delete tombstone (whose non-key columns are legitimately null) was rejected (ERROR) or silently dropped (DROP), losing the delete.
This PR relaxes NOT NULL for the non-key columns of DELETE rows in key-only-delete pipelines; key columns and all other row kinds stay enforced.
Brief change log
Verifying this change
Does this pull request potentially affect one of the following parts:
@Public(Evolving): noDocumentation
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.8, 1M)