andygrove opened a new issue, #6624:
URL: https://github.com/apache/datafusion-comet/issues/6624
Part of #6615.
## What / Why
This issue moves cast coverage for **date, timestamp, timestamp_ntz, binary,
struct, array, map and variant sources** to Comet SQL tests under
`sql-tests/expressions/cast/`. The sources are `CometNativeCastSuite` and the
three timestamp/NTZ cast tests in `CometExpressionSuite`. Numeric and string
sources have their own issues. Static analysis on `main` at 3bc2faa934 found
the following:
| Suite | Covered | Convert | Blocked | Split | Keep | Total |
|---|---|---|---|---|---|---|
| `CometNativeCastSuite` | 0 | 40 | 0 | 0 | 1 | 41 |
| `CometExpressionSuite` | 0 | 3 | 0 | 0 | 0 | 3 |
| **Total** | **0** | **43** | **0** | **0** | **1** | **44** |
Follow the conventions in the numeric-cast issue:
- cast columns read from Parquet, never literals, because `CometCast` folds
literal casts on the JVM;
- a non-ANSI file with `cast` + `try_cast`, plus an ANSI companion with a
plain-`query` sentinel;
- one failing value per `expect_error` query;
- the 19-zone `representativeTimezones` ConfigMatrix wherever the Scala test
loops over time zones.
Deleting the per-pair Scala tests is blocked on replacing the cast-pair
coverage meta-test (`CometNativeCastSuite` L132), as described in that issue.
## Work
Line numbers refer to `CometNativeCastSuite` unless marked.
**Scalar temporal and binary sources**
- `cast_binary_to_string.sql` (L1756, L1760, L1764)
- Invalid UTF-8 rows: `X'FFFE'`, `X'EDA080'`, `X'61C3'`, `X'80'`,
`X'C0AF'`. Add valid multibyte rows too.
- A plain `query` implies L1764's explicit U+FFFD assertions, because
Spark's collected string goes through the same JVM decoder.
- `cast_from_date.sql` (L1803-L1839)
- The numeric targets are legacy-only and always NULL. CometCast folds
them to a NULL literal, so don't use `expect_native(cast)` for them.
- L1839's comment says `"999-01-01"` tests a year below 1000, but a date
year needs at least 4 digits, so that row is NULL. A real year below 1000 is
`DATE'0999-01-01'`, which needs
`spark.sql.parquet.datetimeRebaseModeInWrite=CORRECTED` on Spark 3.x.
- `cast_date_to_timestamp.sql` (L1846): use 19 zones; the DST windows
between 1970 and 2099 matter.
- `cast_from_timestamp.sql` (L1864-L1913)
- Seven of these tests are `ignore`d: Boolean (no issue link), the
integral and floating targets (#352) and decimal (#1280).
- Those pairs are Unsupported natively and now run through the codegen
dispatcher, so write them as `query expect_dispatch(cast)`. That adds coverage;
nothing runs for them today.
- L1887 needs the year-290000 timestamp.
- `cast_timestamp_ntz.sql` (existing)
- Covers L1854, L1917, L2326, L2336, L2344, plus `CometExpressionSuite`
L1012, L1034 and L1056.
- Add the generator values: fraction .123456, `9999-12-31T01:00:00-02:00`,
and the offsets.
- Add pre-epoch rows with millisecond fractions, `cast(ts as string)` and
`cast(ts as date)` on a TIMESTAMP column, and `cast(ntz as date)` before the
epoch, which must floor.
- Add `Asia/Kathmandu` to the matrix: its 1970 offset (+05:30) differs
from today's (+05:45).
- Add `try_cast` and ANSI column queries.
**Complex types**
- `cast_complex_types_to_string.sql` (existing) + `_ansi.sql` (new) (L1927,
L2084)
- Struct fields:
- add decimal(5,2) and decimal(38,37), which use the INT32 and FLBA
encodings;
- add decimal(20,0) and bigint, as stand-ins for the unsigned columns of
the raw-writer file;
- add a TIMESTAMP_MILLIS field, in a small separate file with
`outputTimestampType`;
- add the nested struct rows.
- Array element types: add tinyint, smallint, float, double, decimal(10,2)
and timestamp_ntz arrays, plus Unicode strings.
- Add `try_cast` and an ANSI run.
- `cast_array_to_string.sql` (L2105): add NULL float and double arrays, an
`X'80FF'` element, `try_cast` and an ANSI run.
- `cast_complex.sql` + `cast_complex_ansi.sql` (existing) (L1971, L2023,
L2268, and part of L2147)
- Add a CASE-wrapped struct of boolean/tinyint/smallint/int cast to a
struct of strings.
- Add the legacy and `try_cast` forms of struct and array long-to-byte
overflow; today only the ANSI side exists. Also add the top-level -129 overflow
row.
- Add a pure rename with no type change (L2023).
- Pin `expect_dispatch(cast)` on `cast_complex.sql:185-208` and
`expect_native(cast)` on `:211-214`, and add the missing targets (L2268).
- `cast_array_to_array.sql` + `_ansi.sql` (L2147, L2242)
- Use the 105-pair matrix that `testArrayCastMatrix` exercises: every
scalar pair that has a non-ignored test and that `CometCast` reports Compatible
in LEGACY. Pairs marked [L] below are LEGACY only, with no `try_cast` and no
ANSI run.
- The pairs, by source element type:
- boolean: string, tinyint, int, bigint, smallint, float, double,
timestamp [L]
- string: boolean, the integral types, float, double, decimal(10,2),
date, timestamp, timestamp_ntz, binary
- integral types: boolean, string, the other integral types, float,
double, decimal(10,2), timestamp, binary [L]
- float and double: boolean, string, the integral types, the other
floating type, decimal(10,2), timestamp
- decimal(10,2): boolean, string, the integral types, float, double,
timestamp
- decimal(38,18): boolean, string, the integral types, float, double
- date: string, int [L]
- timestamp: string, bigint, date, timestamp_ntz
- timestamp_ntz: string, date, timestamp
- binary: string
- Also add `expect_dispatch(cast)` for `array<date>` to the 7 unsupported
element types.
- `cast_nested_array_to_array.sql` + `_ansi.sql` (L2306): the same matrix
for `array<array<T>>`, without timestamp_ntz (100 pairs).
- `cast_map_to_map.sql` (L2179): `map<int,int>` to `map<bigint,bigint>`,
`map<int,string>` and `map<string,double>`, over NULL, empty and min/max
entries.
- `routing_cast_nested_disabled.sql` (new, L2227). It must not set
`castComplexTypesToString.enabled=true`, which changes the inner reason; that's
why it can't go in `routing_cast_disabled.sql`. The reason half of L2242 goes
in `routing_cast_disabled.sql`.
- `cast_variant.sql` (L2352, `MinSparkVersion: 4.0`).
## Stays in Scala
- **L2205** calls `isSupported` on `map<int, decimal(7,-2)>`. A
negative-scale decimal type can't be built from SQL.
## Notes on the Scala tests
- **L1760**, "valid UTF-8 inputs", casts a STRING column to STRING, which
does nothing. The fixture should use a binary column.
- **L1864**, the ignored `cast TimestampType to BooleanType`, has no issue
link. With the dispatcher it can simply run as above.
## Done when
- Every test in the table below is accounted for. Either a fixture covers it
(the same inputs, configs and assertion strength, or stronger), the fixture
that already covers it is cited, or it stays in Scala for the reason given
above.
- Each converted Scala test is deleted in the PR that adds its coverage. A
suite left empty is deleted and removed from the suite lists in
`.github/workflows/pr_build_linux.yml` and `pr_build_macos.yml`.
- The new and changed fixtures pass on every Spark profile. Apply
`run-all-spark-profiles`, because pull requests run only the default profile.
<details><summary>Every Scala test in scope, with its verdict and target
fixture</summary>
| Suite | Line | Test | Verdict | Target |
|---|---|---|---|---|
| `CometExpressionSuite` | 1012 | cast timestamp and timestamp_ntz | convert
| cast/cast_timestamp_ntz.sql |
| `CometExpressionSuite` | 1034 | cast timestamp and timestamp_ntz to string
| convert | cast/cast_timestamp_ntz.sql |
| `CometExpressionSuite` | 1056 | cast timestamp and timestamp_ntz to long,
date | convert | cast/cast_timestamp_ntz.sql |
| `CometNativeCastSuite` | 1756 | cast BinaryType to StringType | convert |
cast/cast_binary_to_string.sql (new) |
| `CometNativeCastSuite` | 1760 | cast BinaryType to StringType - valid
UTF-8 inputs | convert | cast/cast_binary_to_string.sql (new) |
| `CometNativeCastSuite` | 1764 | cast BinaryType to StringType - invalid
UTF-8 bytes are replaced | convert | cast/cast_binary_to_string.sql (new) |
| `CometNativeCastSuite` | 1803 | cast DateType to BooleanType | convert |
cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1807 | cast DateType to ByteType | convert |
cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1811 | cast DateType to ShortType | convert |
cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1815 | cast DateType to IntegerType | convert |
cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1819 | cast DateType to LongType | convert |
cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1823 | cast DateType to FloatType | convert |
cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1827 | cast DateType to DoubleType | convert |
cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1831 | cast DateType to DecimalType(10,2) |
convert | cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1839 | cast DateType to StringType | convert |
cast/cast_from_date.sql (new) |
| `CometNativeCastSuite` | 1846 | cast DateType to TimestampType | convert |
cast/cast_date_to_timestamp.sql (new) |
| `CometNativeCastSuite` | 1854 | cast DateType to TimestampNTZType |
convert | cast/cast_timestamp_ntz.sql; cast/cast_timestamp_ntz_ansi.sql |
| `CometNativeCastSuite` | 1864 | cast TimestampType to BooleanType
[ignored] | convert | cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1869 | cast TimestampType to ByteType [ignored] |
convert | cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1875 | cast TimestampType to ShortType [ignored]
| convert | cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1881 | cast TimestampType to IntegerType
[ignored] | convert | cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1887 | cast TimestampType to LongType | convert |
cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1891 | cast TimestampType to FloatType [ignored]
| convert | cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1897 | cast TimestampType to DoubleType [ignored]
| convert | cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1903 | cast TimestampType to DecimalType(10,2)
[ignored] | convert | cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1909 | cast TimestampType to StringType | convert
| cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1913 | cast TimestampType to DateType | convert |
cast/cast_from_timestamp.sql (new) |
| `CometNativeCastSuite` | 1917 | cast TimestampType to TimestampNTZType |
convert | cast/cast_timestamp_ntz.sql; cast/cast_timestamp_ntz_ansi.sql |
| `CometNativeCastSuite` | 1927 | cast StructType to StringType | convert |
cast/cast_complex_types_to_string.sql;
cast/cast_complex_types_to_string_ansi.sql (new) |
| `CometNativeCastSuite` | 1971 | cast StructType to StructType | convert |
cast/cast_complex.sql; cast/cast_complex_ansi.sql |
| `CometNativeCastSuite` | 2023 | cast StructType to StructType with
different names | convert | cast/cast_complex.sql |
| `CometNativeCastSuite` | 2084 | cast ArrayType to StringType | convert |
cast/cast_complex_types_to_string.sql;
cast/cast_complex_types_to_string_ansi.sql (new) |
| `CometNativeCastSuite` | 2105 | cast ArrayType to StringType - float
double binary edge cases | convert | cast/cast_array_to_string.sql |
| `CometNativeCastSuite` | 2147 | cast ArrayType to ArrayType | convert |
cast/cast_array_to_array.sql (new); cast/cast_array_to_array_ansi.sql (new);
cast/cast_complex.sql; cast/cast_complex_ansi.sql |
| `CometNativeCastSuite` | 2179 | cast MapType to MapType | convert |
cast/cast_map_to_map.sql (new) |
| `CometNativeCastSuite` | 2205 | cast MapType propagates Incompatible from
inner value cast | keep | |
| `CometNativeCastSuite` | 2227 | cast MapType propagates Unsupported from
nested value cast | convert | cast/routing_cast_nested_disabled.sql (new) |
| `CometNativeCastSuite` | 2242 | cast ArrayType(DateType) to unsupported
ArrayType routes through codegen dispatch | convert |
cast/cast_array_to_array.sql (new); cast/routing_cast_disabled.sql |
| `CometNativeCastSuite` | 2268 | cast StructType and MapType with DateType
to numeric routes through codegen dispatch | convert | cast/cast_complex.sql |
| `CometNativeCastSuite` | 2306 | cast nested ArrayType to nested ArrayType
| convert | cast/cast_nested_array_to_array.sql (new);
cast/cast_nested_array_to_array_ansi.sql (new) |
| `CometNativeCastSuite` | 2326 | cast TimestampNTZType to StringType |
convert | cast/cast_timestamp_ntz.sql; cast/cast_timestamp_ntz_ansi.sql |
| `CometNativeCastSuite` | 2336 | cast TimestampNTZType to DateType |
convert | cast/cast_timestamp_ntz.sql; cast/cast_timestamp_ntz_ansi.sql |
| `CometNativeCastSuite` | 2344 | cast TimestampNTZType to TimestampType |
convert | cast/cast_timestamp_ntz.sql; cast/cast_timestamp_ntz_ansi.sql |
| `CometNativeCastSuite` | 2352 | cast to and from VariantType matches Spark
| convert | cast/cast_variant.sql (new) |
</details>
--
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]
