felipepessoto opened a new pull request, #12829:
URL: https://github.com/apache/gluten/pull/12829

   ## What changes are proposed in this pull request?
   
   The Delta Spark UT gate quarantines two native error signatures:
   
   ```
   Delta RoaringBitmapArray row index \d+ exceeds max representable value
   Delta bitmap row index cannot be negative: -\d+
   ```
   
   Both come from the native Delta bitmap aggregator receiving a garbage row 
index during a `MERGE` that writes deletion vectors. Because the abort landed 
on a different `*DVs*Suite` test 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 to `numValues` but sized the synthesized fields 
from `outputRows()`, which is empty when the scan has a filter and no deletion. 
The row-index child therefore came back with zero rows inside a `RowVector` 
reporting 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 like 
`9223372036854775807`, `-1` and pointer-shaped values such as 
`0xe43315c000007f00`. 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:
   
   | file | change |
   |---|---|
   | `flaky-error-patterns.txt` | both patterns and their rationale block 
removed; the generic header is kept so the mechanism stays documented |
   | `flaky-tests.txt` | drops the note claiming the DV bug "is handled there, 
which is why no `*DVs*Suite` MERGE entries are listed below" -- no longer true |
   | `README.md` | keeps the worked example, marked historical and linked to 
the upstream issue, since it documents how signature quarantine works |
   
   **This must not merge before the Velox bump that includes `1f971d3`.** 
Gluten currently pins `dft-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's `paths:` 
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_operators` across the Delta suite, which turns the 
malformed vector into an immediate, reproducible error:
   
   * #12783 -- validation on, stock Velox: **6 of 8 shards failed**, `Child 
vector has size 0 less than parent and parent has no nulls`.
   * #12808 -- identical, plus `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) and `rowIndexWithDynamicFilterOnPartitionKey` (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.py` was exercised against the edited file: 
`load_patterns()` returns `[]`, and the signature matcher returns `False` for 
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)
   


-- 
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]

Reply via email to