andygrove commented on code in PR #5032:
URL: https://github.com/apache/datafusion-comet/pull/5032#discussion_r3969700455
##########
spark/src/test/resources/sql-tests/expressions/csv/to_csv.sql:
##########
Review Comment:
Both were worth adding, and both went in as 7a7942c5 — I never came back to
this thread to say so, sorry.
**The ANSI cast.** `to_csv_ansi.sql` covers exactly the shape you named,
including the specific #5219 one:
```sql
query expect_error(CAST_INVALID_INPUT)
SELECT to_csv(named_struct('a', CAST(s AS INT))) FROM test_to_csv_ansi
query expect_error(CAST_INVALID_INPUT)
SELECT to_csv(named_struct('a', CAST(s AS INT), 'b', i)) FROM
test_to_csv_ansi
```
The second is the #5219 configuration — two input ordinals where the
non-cast one is NULL on the row whose cast raises. Worth noting what I found
writing it: `to_csv` does not actually reach that null short-circuit today,
because `CreateNamedStruct` is not `NullIntolerant` so `allNullIntolerant`
already fails. So these pin a shape that is one wire-up change away from
mattering rather than one that is live now. I said that in the file so the next
reader does not conclude the guard is load-bearing.
There is also a `TRY_CAST` case, since the interesting inverse is that it
must *not* raise and must render as the CSV nullValue.
The file needed a sentinel to be worth anything, which is the third query:
an `expect_error` fixture passes vacuously if Comet silently rejects the
expression and Spark raises the same error on the fallback path. That query
uses `checkSparkAnswerAndOperator` on the well-formed row, so a silent
dispatcher rejection fails there instead.
**Complex field types.** Also covered, and you were right that it is the
same class of gap. `to_csv.sql` now runs a struct with array / map /
nested-struct fields three ways — straight from the scan (so the converter's
per-field reads land on `CometInternalRow`), through `named_struct` over those
fields, and with complex types nested inside complex types so the converter
recurses through the element getters rather than stopping at the first level.
Those assert `IS NOT NULL` rather than a value, for the reason you
anticipated: on 3.4 / 3.5 `UnivocityGenerator.makeConverter` has no
complex-type branch and lands on `getter.get(ordinal, dataType).toString`, an
identity string whose class differs between Spark's converter input and the
kernel's. That generic `get` is precisely what
`CometSpecializedGettersDispatch` implements, so on those versions these
queries are its coverage. `to_csv_nested.sql` asserts the real values on 4.0+,
where Spark has deterministic converters.
All three fixtures pass on this head.
--
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]
