snmvaughan opened a new pull request, #7159:
URL: https://github.com/apache/datafusion-comet/pull/7159
## Which issue does this PR close?
Closes #7158.
## Rationale for this change
The native Parquet scan's expression adapter (`SparkPhysicalExprAdapter`)
rewrites every DataFusion `CastExpr` it finds in a pushed-down predicate as
though it were a file-versus-table schema conversion, and applies Spark's
Parquet read rules to it. A predicate can also contain `CastExpr`s that have
nothing to do with the file schema: Comet's planner builds them with
`CastExpr::new` to coerce scalar-function arguments to the types the function
requires. `substring(s, 1, 2)` is one. Spark sends Int32 literals,
`SparkSubstring` takes Int64 positions, so the planner wraps them in `CAST(1 AS
Int64)` and `CAST(2 AS Int64)`.
On Spark 3.x (`allow_type_promotion = false`) the adapter reports that cast
as a disallowed `INT32 -> bigint` Parquet promotion. The check is deferred to
runtime, so it only fires when the predicate is evaluated against a decoded
batch, that is with `spark.comet.parquet.rowFilterPushdown.enabled=true`:
```
Error evaluating filter predicate: External(ParquetSchemaConvert {
file_path: "", column: "[]", physical_type: "INT32", spark_type: "bigint" })
```
The file's schema matches the requested schema exactly. The empty column
name is the tell: the cast is not over a column.
With the flag off (the default) the predicate is only used for row-group and
page pruning, so the problem stays hidden. It surfaced as TPC-H Q22, the only
TPC-H query that uses `substring`, failing on Spark 3.4 when the TPC-H suite
ran with row-level filter pushdown enabled. On Spark 4.x `allow_type_promotion`
is true, so this particular cast is accepted, but the planner's cast is still
silently swapped for a Spark `Cast`.
## What changes are included in this PR?
`replace_with_spark_cast` now leaves a cast alone unless it reconciles a
file column with its table-schema type (new `is_schema_adaptation_cast`).
DataFusion's `DefaultPhysicalExprAdapter` builds those with
`CastExpr::new_with_target_field`, passing the table schema's field, so the
target carries the column's name. The planner's `CastExpr::new` casts have an
unnamed target field.
One behavior change: casts the planner inserted into a pushed-down predicate
now stay DataFusion `CastExpr`s (with the planner's `safe: false` options) on
every Spark version, instead of being rewritten into Spark `Cast`s on Spark
4.x. That matches how the same expression is evaluated when it is not pushed
into the scan.
The check keys on the target field's name because `CastExpr` offers nothing
else to tell the two apart in the DataFusion version used here. A unit test
pins that naming convention, so a DataFusion upgrade that changes it fails
loudly rather than silently reintroducing the problem.
## How are these changes tested?
New tests in `schema_adapter.rs`:
- `schema_adaptation_cast_is_told_apart_by_target_field_name` pins the
naming convention the check relies on.
- `pushed_substring_filter_*` build `substring(s, 1, 2) = 'ab'` through
`PhysicalPlanner` from the proto `CometSubstring` serializes, then scan a
Parquet file whose schema equals the requested schema: without row-level
pushdown (control), with it using Spark 3.x settings (this fails without the
change, with the error above), and with it using Spark 4.x settings.
The existing tests for genuine schema mismatches (for example INT32 read as
bigint) are unchanged and still pass, because the adapter names those casts
after the column. The `datafusion-comet` lib tests pass (`cargo test -p
datafusion-comet --lib`), and `cargo fmt --check` and `cargo clippy
--all-targets -- -D warnings` are clean.
--
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]
