anew commented on PR #57625:
URL: https://github.com/apache/spark/pull/57625#issuecomment-5114759096
Here is codex' finding on test coverage in this PR:
1. Coverage is not quite enough for upgrade/backcompat.
The PR makes SCD2 validation require the new auxiliary-table property
pipelines.autocdc.trackHistoryColumnNames. Existing SCD2 AutoCDC
auxiliary tables created before this
PR will not have that property, and validateNoTrackHistoryDrift will
fail with
AUTOCDC_INVALID_STATE.AUXILIARY_TABLE_PROPERTY_MISSING. I don’t see a
test for “existing SCD2 aux
table without the new property”. That is the main gap, and possibly a
product compatibility issue
unless full refresh is explicitly intended for all existing SCD2
AutoCDC targets.
2. Track-history coverage is too narrow.
The new suite covers explicit TRACK HISTORY name changing to explicit
TRACK HISTORY amount, plus
reorder of the same explicit set. It does not cover:
- default track-history set vs explicit same set
- default all tracked columns vs explicit subset
- EXCEPT/exclude-based track-history selection
- resolver/case-sensitivity behavior
Those are the riskier paths because the implementation stores the
result of
Scd2BatchProcessor.computeTrackedHistoryColumns(...), not merely the
user’s explicit include list.
3. Sequencing coverage is mostly good, with one small gap.
The tests cover SCD1 drift, SCD1 same-type expression change, and SCD2
drift. I’d add one SCD2 same-
type expression-change test for symmetry, but this is lower priority
than the SCD2 track-history/
backcompat gaps.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
