andygrove opened a new issue, #6619:
URL: https://github.com/apache/datafusion-comet/issues/6619
Part of #6615.
### What is the problem the feature request solves?
Auditing the Scala expression suites against the SQL fixtures turned up
several ways a fixture can look like it tests Comet when it does not.
`docs/source/contributor-guide/sql-file-tests.md` doesn't mention them, and in
one place recommends the affected pattern. Each item below was found in
existing fixtures or Scala tests:
1. **NULL literal arguments never reach Comet.** Spark's `NullPropagation`
rule, which the suite does not exclude, folds every null-intolerant expression
with a NULL literal child into a typed NULL. The guide's own example `SELECT
ascii('A'), ascii(''), ascii(NULL)` is affected. So are `last_day(NULL)`,
`datediff(NULL, ..)`, `unix_date(NULL)`, `next_day(NULL, ..)` and
`make_date(NULL, ..)` in the datetime fixtures. To test NULL handling, read the
NULL from a column.
2. **Casts of literals are evaluated on the JVM.** `CometCast.convert`
replaces `Cast(Literal)` with `Literal(cast.eval())` at planning time, even
though the suite excludes ConstantFolding. All-literal casts in `cast.sql`,
`cast_timestamp_ntz.sql`, `cast_timestamp_ntz_ansi.sql` and
`cast_boolean_to_decimal*.sql` therefore never run a native cast kernel. Cast a
column read from Parquet instead.
3. **Expressions over inline `VALUES` are folded.** `ConvertToLocalRelation`
evaluates a `Project` over a `LocalRelation` in the optimizer, in both runs.
`SELECT f(x) FROM VALUES ... AS t(x)` never runs `f` in Comet, so expression
inputs must come from a Parquet table. A `Generate` or `Aggregate` over
`VALUES` is not folded.
4. **`INSERT INTO t VALUES (...)` writes one file per row** for small
inserts under `local[5]`. Every batch then has one row and rows arrive in no
particular order. Most array and map fixtures look like this. When batch shape,
order or multi-row batches matter, use `INSERT INTO t SELECT /*+ COALESCE(1) */
* FROM VALUES ... AS v(...)` or `range(0, n, 1, 1)`.
5. **Regex backslashes.** Spark's string-literal unescaping drops the
backslash before an unknown escape: `'\d'` is `d`, `'\w'` is `w`, `'\1'` is
`1`, `'\Z'` is U+001A. Seven regex fixtures are affected; they are tracked in
the regex sub-issue of #6615. Write `'\\d'` or `r'\d'`.
6. **`query tolerance=` semantics.** It skips NaN, ignores the sign of
infinity and does not sort rows; see #6616. Until that is fixed, NaN, infinity
and signed-zero results belong in a plain `query`, and grouped tolerance
queries need `ORDER BY`.
7. **Bare `-0.0` is a decimal literal** and lands as `+0.0`. The guide
already says this, but several fixtures still have it: `explode.sql`,
`posexplode.sql`, `element_at_map.sql`, `get_map_value.sql`.
8. **Pin the session time zone** whenever a result depends on it.
`trunc_timestamp.sql` sets `TruncTimestamp.allowIncompatible=true` without a
time zone, so it runs in the machine's zone.
9. **Mid-file `statement SET`** restores only keys the file also declares in
`Config` / `ConfigMatrix`; other keys leak into later fixtures. Document this,
or fix it in #6618.
10. **Wrong example.** The `expect_dispatch(bit_length)` example on a binary
column contradicts `string/bit_length.sql`, which asserts
`expect_fallback(bit_length on BinaryType is not supported)` for that shape.
### Describe the potential solution
Add a "Traps" section to `sql-file-tests.md` covering the items above. Fix
the `ascii(NULL)` and `bit_length` examples. Mention the one-file insert idiom
in the "Adding a new test" steps.
The "Adding a New Expression" guide should point at SQL file tests as the
default (#3406).
--
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]
