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

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. 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 an assert, so a normal build corrupts the buffer silently instead of failing. 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.

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