ykisana opened a new pull request, #59443:
URL: https://github.com/apache/spark/pull/59443
### What changes were proposed in this pull request?
`InternalRow.getWriterDefault` copies `UTF8String` values before storing
them in the target row, but it matched strings with the stable identifier
pattern `case StringType`. That pattern is an equality check against the
`StringType` case object (UTF8_BINARY), and `StringType.equals` compares the
collation id and constraint. A collated `StringType` (e.g. `UTF8_LCASE`,
`UNICODE_CI`, `UTF8_LCASE_RTRIM`) and `CharType`/`VarcharType` therefore fell
through to `case _ => input.update(ordinal, v)` and were stored **without** a
copy.
This PR changes the pattern to `case _: StringType`, so every string type
takes the copying branch. This matches the codegen counterpart
`CodeGenerator.setColumn`, which already uses `case _: StringType | ...`
(changed in SPARK-46832 when collations were introduced); the interpreted path
was missed at the time.
### Why are the changes needed?
`InterpretedMutableProjection` (used by aggregation when
`spark.sql.codegen.factoryMode=NO_CODEGEN`) and `InterpretedSafeProjection`
write values through `InternalRow.getWriter`. Without the copy, the aggregation
buffer keeps a pointer into the input row's memory, which is overwritten when
the input `UnsafeRow` is reused. Interpreted MAX/MIN over collated strings then
returns a wrong value or corrupted bytes:
```python
df = spark.createDataFrame([("abcd",), ("xy",), ("ab",)], "s STRING COLLATE
UTF8_LCASE")
spark.conf.set("spark.sql.codegen.wholeStage", "false")
spark.conf.set("spark.sql.codegen.factoryMode", "NO_CODEGEN")
df.agg(F.max("s"), F.min("s")).collect()
# expected: [Row(max(s)='xy', min(s)='ab')]
# before: [Row(max(s)='ab', min(s)='ab')]
```
### Does this PR introduce _any_ user-facing change?
Yes, a bug fix. In interpreted (non-codegen) execution, aggregates such as
MAX/MIN over collated strings (and CHAR/VARCHAR) could return wrong or
corrupted values; they now return the correct result, matching codegen. Affects
released versions (reported on 4.0.0, 4.1.1 and 4.2.0).
### How was this patch tested?
Added `SPARK-60095: collated strings are copied into the buffer` to
`MutableProjectionSuite`, run in both codegen and interpreted modes. It
projects `UTF8_BINARY`, `UTF8_LCASE`, `UTF8_LCASE_RTRIM` and `UNICODE_CI`
columns backed by one byte array, overwrites that array (simulating a reused
input row), and checks the projected values are unchanged.
- Without the fix: the `NO_CODEGEN` variant fails with `xbc did not equal
abc`; the codegen variant passes.
- With the fix: `build/sbt 'catalyst/testOnly *MutableProjectionSuite'`
passes (22 tests).
### Was this patch authored or co-authored using generative AI tooling?
Generated by: Claude Opus 5.5
Fix written by and all code manually reviewed by @ykisana.
Tests, PR description, and comments by Claude.
--
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]
