[GLUTEN-12377][CI] Stop quarantining the Delta DV row-index failures - #12829
Draft
felipepessoto wants to merge 1 commit into
Draft
[GLUTEN-12377][CI] Stop quarantining the Delta DV row-index failures#12829felipepessoto wants to merge 1 commit into
felipepessoto wants to merge 1 commit into
Conversation
The native Delta bitmap aggregator intermittently aborted during a MERGE writing deletion vectors, with a garbage row index. Because it landed on a different *DVs*Suite test each run, it was quarantined by error signature rather than by test name. The root cause was not in the aggregator. It was a Velox scan defect: with no child readers, a filter and no deletion, 'outputRows()' was empty while the result carried numValues rows, so the row-index child came back with zero rows and downstream reads ran off the end of a zero-length buffer. Fixed upstream in facebookincubator/velox#18536, so the aggregator now receives real row indexes and the suite can be enforced again. Remove both patterns and the two cross-references that named them. The README keeps the example, marked historical, since it documents the mechanism. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 are proposed in this pull request?
The Delta Spark UT gate quarantines two native error signatures:
Both come from the native Delta bitmap aggregator receiving a garbage row index during a
MERGEthat writes deletion vectors. Because the abort landed on a different*DVs*Suitetest each run, matching by test name was whack-a-mole, so it was quarantined by error signature instead.The root cause was not in the aggregator, and not in Gluten. It was a Velox scan defect: in
SelectiveStructColumnReaderBase::next(), the no-child-readers branch sized the result vector tonumValuesbut sized the synthesized fields fromoutputRows(), which is empty when the scan has a filter and no deletion. The row-index child therefore came back with zero rows inside aRowVectorreporting N.BaseVector::wrapInDictionary()does not bounds-check indexes against the base, so downstream reads ran off the end of a zero-length buffer and returned whatever heap memory followed -- hence row indexes like9223372036854775807,-1and pointer-shaped values such as0xe43315c000007f00. Most were silently accepted; occasionally one failed the aggregator's bounds check and aborted the query.Fixed upstream in facebookincubator/velox#18536 (facebookincubator/velox#18535), merged as
1f971d3. The aggregator now receives real row indexes, so the suite can be enforced again.This PR removes both patterns, plus the two cross-references that named them:
flaky-error-patterns.txtflaky-tests.txt*DVs*SuiteMERGE entries are listed below" -- no longer trueREADME.mdThis must not merge before the Velox bump that includes
1f971d3. Gluten currently pinsdft-2026_08_17; the fix merged upstream on 2026-08-20, so it will arrive in a later tag. Merging ahead of that would un-quarantine a bug that is still present.Fixes #12377
How was this patch tested?
The Delta Spark UT runs on this PR:
.github/workflows/util/delta-spark-ut/**is in the workflow'spaths:filter. With the patterns removed, a DV abort is now counted as a regression instead of being dropped.A green run here is weak evidence on its own. The abort is intermittent -- that is precisely why it was quarantined by signature rather than by test name -- so the suite can pass on a given run whether or not the Velox fix is present. Red would be informative (the bug still fires); green would not prove much. The dependable check is simply whether the pinned Velox tag contains
1f971d3.The fix itself was therefore validated separately, by making the failure deterministic instead of relying on chance. Two throwaway PRs enabled Velox's
debug.validate_output_from_operatorsacross the Delta suite, which turns the malformed vector into an immediate, reproducible error:Child vector has size 0 less than parent and parent has no nulls.UPSTREAM_VELOX_PR_ID=18536: 8 of 8 shards green.That A/B, not this PR's own run, is the evidence that the row indexes were corrupt and that velox#18536 fixes them.
Upstream, the fix carries two regression tests in
TableScanTest, covering both ways the branch is reached:rowIndexWithFilterOnPartitionKeyOnly(static subfield filter) androwIndexWithDynamicFilterOnPartitionKey(no filter in the plan; a join on the partition key supplies a dynamic filter at runtime). Both fail before the fix and pass after it.Locally,
compare-test-results.pywas exercised against the edited file:load_patterns()returns[], and the signature matcher returnsFalsefor the old DV error -- i.e. such a failure is now enforced rather than ignored.Was this patch authored or co-authored using generative AI tooling?
Generated-by: GitHub Copilot CLI (Claude Opus 5)