apache / apache/datafusion-comet

[Doc] CAST collated-string handling on Spark 4.0+ is implicit and untested

Open
#4,489 0 comments 0 reactions 0 assignees View on GitHub
area:expressions documentation priority:low
Dominant language
Scala
Stars
1.3k
Forks
373
Avg merge
2d 6h
Merged PRs (30d)
190

Description

## Describe the bug

Spark 4.0+ supports collated `StringType` (e.g. `STRING COLLATE UTF8_LCASE`). `CometCast.isSupported` matches both source and target string types via `case (DataTypes.StringType, _) => ...` and `case (_, DataTypes.StringType) => ...`, where `DataTypes.StringType` is the singleton default-collation `StringType` instance.

Scala pattern equality means a non-default-collation `StringType` instance should NOT match `DataTypes.StringType`, and the cast would fall through to the default `unsupported(...)` branch and fall back to Spark. This appears to be the intended (safe) behaviour, but it is implicit: there is no `isStringCollationType` guard like the other string-touching serdes use (`arrays.scala::CometArrayIntersect`, `QueryPlanSerde::supportedScalarSortElementType`). And there is no test that asserts the fallback for `CAST(c AS STRING COLLATE UTF8_LCASE)` and `CAST(c AS STRING)` when `c` is collated.

If the equality ever yields true (e.g. via a future refactor of `StringType.equals`), Comet would silently route collated-string casts through the byte-oriented native path, producing incorrect results for downstream collation-aware comparisons / aggregations / hashing.

Surfaced by the cast audit (collection PR queue). Tracked under the umbrella #2190 for Spark 4.0 collation support.

## Expected behavior

Either:

1. Add an explicit `isStringCollationType` guard in `CometCast.isSupported` so the fallback is declared rather than relying on Scala pattern semantics, OR
2. Add tests asserting that `CAST` to/from a non-default `StringType` collation falls back to Spark and does not run native.

## Additional context

- Comet matrix: `CometCast.scala`, lines around the `case (DataTypes.StringType, _)` and `case (_, DataTypes.StringType)` matches.
- Umbrella issue: #2190 (`[Spark 4.0] Add string collation support`).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.