Skip to content

[SPARK-58959][SQL] Duplicate PIVOT values corrupt the aggregation buffer on the PivotFirst fast path - #58235

Draft
jiwen624 wants to merge 1 commit into
apache:masterfrom
jiwen624:SPARK-58959
Draft

[SPARK-58959][SQL] Duplicate PIVOT values corrupt the aggregation buffer on the PivotFirst fast path#58235
jiwen624 wants to merge 1 commit into
apache:masterfrom
jiwen624:SPARK-58959

Conversation

@jiwen624

@jiwen624 jiwen624 commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

PivotFirst indexed the pivot values with Map(pivotColumnValues.zipWithIndex), so values that compare as equal collapsed onto one entry holding the last of their indices while indexSize shrank to the distinct count, leaving the stored index pointing past the end of the buffer. With this PR, every entry now gets its own slot, shared between entries that compare as equal, and eval expands back to one array element per entry.

Why are the changes needed?

A PIVOT whose IN list repeats a value writes outside its allocated slots in the aggregation buffer:

SELECT * FROM VALUES (1, 1, 10), (1, 2, 20) AS t(id, k, v)
PIVOT (sum(v) FOR k IN (1 AS x, 1 AS y));

java.lang.AssertionError: index (1) should < 1

The bounds check is a Java assert which lives only under -ea. A normal build writes outside the slots unchecked, then fails with [INVALID_ARRAY_INDEX]. The index 1 is out of bounds under ANSI, or returns [1, null, null] instead of [1, 10, 10] without it. Duplicates are not limited to identical literals: (0.0D, -0.0D) and ('a', 'A') under UTF8_LCASE reach the same path. The non-optimized path already answers these queries correctly.

Note: This also fixes a NULL in the IN list failing the query with an INTERNAL_ERROR, when the pivot column is one that PivotFirst indexes with a comparison-based TreeMap (a non-atomic type, or a collated string): such a value now gets a slot that no input row can match, so its output column is null.

Does this PR introduce any user-facing change?

Yes. The optimized path now returns one column per listed value, matching the non-optimized path, instead of failing with AssertionError or silently corrupting the buffer.

How was this patch tested?

Added UT cases.

Was this patch authored or co-authored using generative AI tooling?

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant