[SPARK-57251][SDP] Validate SCD2 reserved framework columns at AutoCDC flow construction - #57527
Closed
anew wants to merge 3 commits into
Closed
[SPARK-57251][SDP] Validate SCD2 reserved framework columns at AutoCDC flow construction#57527anew wants to merge 3 commits into
anew wants to merge 3 commits into
Conversation
…C flow construction AutoCdcMergeFlow only rejected source columns using the reserved AutoCDC prefix (`__spark_autocdc_`). SCD2 additionally persists the framework columns `__START_AT` and `__END_AT`, which do not carry that prefix, so a colliding source column would be silently overwritten during preprocessing. Add a constructor-time check that rejects source columns colliding (by exact name, resolver-aware) with SCD2's non-prefixed reserved framework columns. It runs before the flow schema is forced, so it surfaces this actionable error ahead of the temporary AUTOCDC_SCD2_NOT_SUPPORTED gate and remains correct once SCD2 support lands. SCD1 targets carry no non-prefixed framework columns, so the check is a no-op for SCD1. Adds the AUTOCDC_RESERVED_COLUMN_NAME_CONFLICT error condition and tests in AutoCdcFlowSuite. Co-authored-by: Isaac
…nces Per review feedback: - Remove the SPARK-57251 references from source comments and test names/section header (the merged PR already records provenance), and condense the surrounding comments to reduce bloat. - Clarify the reservedFrameworkColNames scaladoc to say the flow validates the source schema against the *non-prefixed* names in the set (the prefixed member is handled by requireReservedPrefixAbsentInSourceColumns), for precision. No behavior change; the reserved framework-column check still uses `find` (reporting the first collision), consistent with the sibling prefix guard. Co-authored-by: Opus 4.8
…PARK-58319) SPARK-58319 (now on master) added SCD2 target-schema derivation and removed the AUTOCDC_SCD2_NOT_SUPPORTED construction gate, so two tests need updating: - "differently-cased reserved framework-column name ... caseSensitive=true": previously relied on the gate throwing after the reserved-name check passed. It now asserts the flow constructs successfully and keeps the differently-cased column as an ordinary user column. - Behavior change: SPARK-58319 shipped a test asserting a source may contain __START_AT / __END_AT and exclude them via the column selection. This PR's requireReservedFrameworkColumnsAbsentInSourceColumns guard validates the RAW source schema (pre-selection), so such a source is now rejected outright -- an ExcludeColumns cannot rescue it. That test is replaced with one asserting the rejection. Allowing an explicit opt-out by validating post-selection is tracked separately by SPARK-58325. Co-authored-by: Opus 4.8
uros-b
approved these changes
Jul 26, 2026
Member
|
Thank you @anew! |
jose-torres
approved these changes
Jul 27, 2026
jose-torres
pushed a commit
that referenced
this pull request
Jul 27, 2026
…C flow construction ### What changes were proposed in this pull request? What changes were proposed in this pull request? This PR replaces #57488, which had to be abandoned due to severe semantic merge conflicts. It is an exact cherry-pick of the previous commits, plus a fix for the test cases that failed due to the merge conflicts. What were the merge conflicts? Two PRs disagreeing on the semantics when a source contains a reserved column but does not include that column in the column selection. This PR rejects that because it validates pre-column selection. The other PR (SPARK-58313) allowed that because it was doing the validation port column selection. I decided to fall back to the original behavior that already existed for SCD type 1 validate pre column selection. And we have SPARK-58325 to reconsider whether we want to do validation post column selection. AutoCdcMergeFlow validates at construction time that a flow's source change-data feed does not carry columns that collide with AutoCDC's internal columns. Until now, that validation (requireReservedPrefixAbsentInSourceColumns) only rejected column names starting with the reserved prefix __spark_autocdc_. SCD2, however, persists two framework columns to the target that do not carry that prefix — __START_AT and __END_AT (see Scd2BatchProcessor.reservedFrameworkColNames). A source column named __START_AT or __END_AT therefore slipped past the guard and would be silently overwritten during microbatch preprocessing. This PR closes that gap (the TODO(SPARK-57251) in Scd2BatchProcessor): - Adds requireReservedFrameworkColumnsAbsentInSourceColumns() to the AutoCdcMergeFlow constructor. For SCD2 flows it rejects any source column whose name collides (by exact name, resolver-aware so it respects spark.sql.caseSensitive) with a non-prefixed reserved framework column. SCD1 targets carry no such columns, so the check is a no-op for SCD1. - Runs the check before the flow's schema val is forced, so the actionable reserved-name error surfaces ahead of the temporary AUTOCDC_SCD2_NOT_SUPPORTED gate, and the check remains correct once SCD2 support lands. - Adds a new error condition AUTOCDC_RESERVED_COLUMN_NAME_CONFLICT (SQLSTATE 42710), distinct from the existing prefix-based AUTOCDC_RESERVED_COLUMN_NAME_PREFIX_CONFLICT, since this collision is by exact name rather than by prefix. - Widens the visibility of Scd2BatchProcessor.reservedFrameworkColNames to private[pipelines] so the flow layer can validate against the single source of truth. ### Why are the changes needed? Without this check, a user whose CDC source happens to contain a __START_AT or __END_AT column would have that data silently overwritten by AutoCDC's SCD2 framework columns, with no error and no diagnostic — a data-correctness footgun. Failing fast at flow construction with a user-actionable error ("rename or remove the column") is the intended UX, consistent with the existing reserved-prefix guard. ### Does this PR introduce _any_ user-facing change? Yes. An AutoCDC SCD2 flow whose source change-data feed contains a column named __START_AT or __END_AT (subject to case-sensitivity settings) now fails at flow construction with AUTOCDC_RESERVED_COLUMN_NAME_CONFLICT instead of silently overwriting the column. There is no change for SCD1 flows, and no change for SCD2 flows whose sources do not use these names. (Note: SCD2 AutoCDC flows are not yet generally supported on master — they are still gated by AUTOCDC_SCD2_NOT_SUPPORTED — so no released behavior changes.) ### How was this patch tested? New unit tests in AutoCdcFlowSuite covering: - an SCD2 flow with a __START_AT/__END_AT source column is rejected with AUTOCDC_RESERVED_COLUMN_NAME_CONFLICT; - the reserved-name check fires before the AUTOCDC_SCD2_NOT_SUPPORTED gate; - an SCD1 flow with the same column name is allowed and the column survives into the flow schema; - case-sensitivity behavior (spark.sql.caseSensitive true/false); - a guard test asserting the set of non-prefixed reserved names is exactly {__START_AT, __END_AT}, so a future rename can't silently un-cover the validation. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #57527 from anew/spark-57251-reserved-columns-v2. Authored-by: Andreas Neumann <andreas.neumann@databricks.com> Signed-off-by: Jose Torres <jtorres@apache.org> (cherry picked from commit 18ad811) Signed-off-by: Jose Torres <jtorres@apache.org>
Contributor
|
merged to master and 4.x |
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
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?
What changes were proposed in this pull request?
This PR replaces #57488, which had to be abandoned due to severe semantic merge conflicts. It is an exact cherry-pick of the previous commits, plus a fix for the test cases that failed due to the merge conflicts.
What were the merge conflicts? Two PRs disagreeing on the semantics when a source contains a reserved column but does not include that column in the column selection. This PR rejects that because it validates pre-column selection. The other PR (SPARK-58313) allowed that because it was doing the validation port column selection. I decided to fall back to the original behavior that already existed for SCD type 1 validate pre column selection. And we have SPARK-58325 to reconsider whether we want to do validation post column selection.
AutoCdcMergeFlow validates at construction time that a flow's source change-data feed does not carry columns that collide with AutoCDC's internal columns. Until now, that validation (requireReservedPrefixAbsentInSourceColumns) only rejected column names starting with the reserved prefix _spark_autocdc.
SCD2, however, persists two framework columns to the target that do not carry that prefix — __START_AT and __END_AT (see Scd2BatchProcessor.reservedFrameworkColNames). A source column named __START_AT or __END_AT therefore slipped past the guard and would be silently overwritten during microbatch preprocessing.
This PR closes that gap (the TODO(SPARK-57251) in Scd2BatchProcessor):
Why are the changes needed?
Without this check, a user whose CDC source happens to contain a __START_AT or __END_AT column would have that data silently overwritten by AutoCDC's SCD2 framework columns, with no error and no diagnostic — a data-correctness footgun. Failing fast at flow construction with a user-actionable error ("rename or remove the column") is the intended UX, consistent with the existing reserved-prefix guard.
Does this PR introduce any user-facing change?
Yes. An AutoCDC SCD2 flow whose source change-data feed contains a column named __START_AT or __END_AT (subject to case-sensitivity settings) now fails at flow construction with AUTOCDC_RESERVED_COLUMN_NAME_CONFLICT instead of silently overwriting the column. There is no change for SCD1 flows, and no change for SCD2 flows whose sources do not use these names. (Note: SCD2 AutoCDC flows are not yet generally supported on master — they are still gated by AUTOCDC_SCD2_NOT_SUPPORTED — so no released behavior changes.)
How was this patch tested?
New unit tests in AutoCdcFlowSuite covering:
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)