advitrocks9 opened a new pull request, #24802:
URL: https://github.com/apache/datafusion/pull/24802
## Which issue does this PR close?
- No issue filed. `find_in_set.slt` is one of the porting stubs tracked by
#15914 and the bug fell out of filling it. I can open one if the changelog
entry needs a link.
## Rationale for this change
`find_in_set` with two `LargeUtf8` scalar arguments does not return a value,
it trips the planner's type assertion:
```sql
> SELECT find_in_set(arrow_cast('ab', 'LargeUtf8'),
arrow_cast('abc,b,ab,c,def', 'LargeUtf8'));
Internal error: Assertion failed: result_data_type == *expected_type:
Function 'find_in_set'
returned value of type 'Int32' while the following type was promised at
planning time and
expected: 'Int64'.
```
`return_type` goes through `utf8_to_int_type`, which widens to `Int64` for
`LargeUtf8`. The three array branches are generic over `Int32Type`/`Int64Type`
and honour that. The `(Scalar, Scalar)` branch computed `Some(position as i32)`
and passed it to `ScalarValue::from`, which is always `ScalarValue::Int32`, so
it was the one branch that could contradict its own `return_type`. Nothing had
executed it: `string_literal.slt` covers scalar/scalar for `Utf8` and
`Utf8View` only, and the `LargeUtf8` block in `string_query.slt.part` casts
columns, so they land in the array branches.
## What changes are included in this PR?
- Build the scalar result at the width `return_field` promised, with the
same `match` on the return type the array branches already use four times.
- Fill `spark/string/find_in_set.slt`, previously a stub. Expected values
come from Spark 4.2.0, read off `UTF8String.findInSet` and confirmed on
`pyspark==4.2.0`, not from running DataFusion.
The `.slt` is the larger half of the diff at 37 queries. Coverage that
exists nowhere else in the tree: a needle containing a comma, empty and
consecutive and trailing-comma elements, no trimming around delimiters, first
occurrence wins on duplicates, and a 20-element list that crosses
`FIND_IN_SET_LOOKUP_THRESHOLD` so the lookup path from #23460 gets its first
end-to-end test. DataFusion matches Spark on all 37.
## Are these changes tested?
Yes, the `LargeUtf8` query above is the regression test. Reverting the
source change and rebuilding gives exactly one failure, on that query, with the
assertion above.
```
cargo test --profile=ci --test sqllogictests -- spark/string/find_in_set.slt
cargo test --profile=ci --test sqllogictests -- string/
# 65 files
cargo test --profile=ci -p datafusion-functions --lib find_in_set
# 6 passed
cargo clippy --profile ci -p datafusion-functions --lib -- -D warnings
```
I left `string/string_literal.slt` alone. The new case runs in the same CI
job, so a second copy next to the `Utf8` block there catches nothing extra.
## Are there any user-facing changes?
Yes. `find_in_set` over two `LargeUtf8` scalars returns a value instead of
raising an internal error. No API change.
--
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]
