Narrowing `try_cast` is incorrectly unwrapped from comparisons, dropping NULL-on-overflow semantics
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
### Describe the bug
`unwrap_cast_in_comparison` unwraps a **narrowing** `try_cast` out of a comparison, which silently discards `try_cast`'s NULL-on-overflow semantics and produces wrong results.
`try_cast(col AS ) op lit` is rewritten to `col op lit`. The rule's guard checks that the *literal* round-trips into the cast's target type, but not that the *column's* value domain does. For a widening cast that is sound; for a narrowing one it is not, because column values outside the target range become NULL under `try_cast`, and after the unwrap they participate in an ordinary comparison instead.
The clearest symptom is a single row that is self-contradictory:
```sql
CREATE TABLE t AS VALUES (5::bigint), (9999999999::bigint);
SELECT column1,
try_cast(column1 AS INT) AS c,
try_cast(column1 AS INT) > 1 AS p
FROM t;
+------------+---+------+
| column1 | c | p |
+------------+---+------+
| 5 | 5 | true |
| 9999999999 | | true | <-- c IS NULL, yet NULL > 1 is reported as true
+------------+---+------+
```
`c` is NULL (9999999999 overflows int32), but `c > 1` in the same row of the same query reports `true`. Under SQL three-valued logic it must be UNKNOWN.
**There are two independent copies of this defect**, and fixing only the first leaves the parquet scan path wrong:
| | file | forms handled |
|---|---|---|
| logical | `datafusion/optimizer/src/simplify_expressions/unwrap_cast.rs` | binary comparison + `InList` |
| physical | `datafusion/physical-expr/src/simplifier/unwrap_cast.rs` (`try_unwrap_cast_comparison`) | binary comparison only |
The physical copy runs from `datasource-parquet/src/opener/mod.rs` (its `needs_rewrite` is true whenever a predicate exists), from `pruning/src/pruning_predicate.rs`, and from `datasource-parquet/src/row_group_filter.rs`. On `main` it is masked because the logical rule fires first, but it is independently reachable — see repro 5 below, where enabling `pushdown_filters` alone changes the answer.
### To Reproduce
On `main` (b9a0053677), DataFusion CLI v55.0.0.
**1. Wrong row set from `WHERE`:**
```sql
CREATE TABLE t AS VALUES (5::bigint), (9999999999::bigint);
SELECT column1 FROM t WHERE try_cast(column1 AS INT) > 1;
+------------+
| column1 |
+------------+
| 5 |
| 9999999999 | <-- must not match: try_cast is NULL, so NULL > 1 is UNKNOWN
+------------+
EXPLAIN SELECT column1 FROM t WHERE try_cast(column1 AS INT) > 1;
| logical_plan | Filter: t.column1 > Int64(1) <-- try_cast dropped
```
**2. Differential — the identical expression is correct when the rule cannot fire.** Routing it through a subquery projection gives the right answer:
```sql
SELECT column1, c, c > 1 AS p
FROM (SELECT column1, try_cast(column1 AS INT) c FROM t);
+------------+---+------+
| column1 | c | p |
+------------+---+------+
| 5 | 5 | true |
| 9999999999 | | | <-- NULL, correct
+------------+---+------+
```
The same holds under `SET datafusion.optimizer.max_passes = 0`, which disables the rule outright: the query returns one row and `p` is NULL. That pins the defect on the rewrite rather than on expression evaluation.
**3. Widening is unaffected (control).** `try_cast(column1 AS DECIMAL(38,0)) > 1` returns `true` for both rows, which is correct — 9999999999 is representable, so no NULL is involved. `SMALLINT` and `TINYINT` narrowing reproduce the same way as `INT`.
**4. `InList` has the same defect.** With `4294967301` (= 2^32 + 5):
```sql
CREATE TABLE t AS VALUES (5::bigint), (4294967301::bigint);
SELECT column1, try_cast(column1 AS INT) c, try_cast(column1 AS INT) IN (5) p FROM t;
-- 4294967301 | (NULL) | false <-- direct
SELECT column1, c, c IN (5) p FROM (SELECT column1, try_cast(column1 AS INT) c FROM t);
-- 4294967301 | (NULL) | (NULL) <-- via subquery, correct
EXPLAIN SELECT column1 FROM t WHERE try_cast(column1 AS INT) IN (5);
| logical_plan | Filter: t.column1 = Int64(5)
SELECT column1 FROM t WHERE try_cast(column1 AS INT) NOT IN (5);
-- returns 4294967301; correct answer is 0 rows, since NOT(UNKNOWN) is UNKNOWN
```
**5. The physical copy makes `pushdown_filters` change the result.** Write the rows to parquet, then disable the logical rule so only the physical simplifier is in play:
```sql
COPY (VALUES (5::bigint),(7::bigint),(9999999999::bigint),(10000000000::bigint))
TO 'pq/t.parquet' STORED AS PARQUET OPTIONS ('format.max_row_group_size' '2');
CREATE EXTERNAL TABLE p STORED AS PARQUET LOCATION 'pq/t.parquet';
CREATE TABLE m AS VALUES (5::bigint),(7::bigint),(9999999999::bigint),(10000000000::bigint);
SET datafusion.optimizer.max_passes = 0;
SELECT count(*) FROM WHERE try_cast(column1 AS INT) > 1;
```
| source | `pushdown_filters` | rows | |
|---|---|---|---|
| memtable | — | 2 | correct |
| parquet | `false` | 2 | correct |
| parquet | `true` | **4** | wrong |
| parquet | `true`, `pruning=false` | **4** | wrong |
Controls that isolate this to the unwrap rather than to a NULL-unsafe row filter, all with `pushdown_filters=true`:
- `(try_cast(column1 AS INT) + 0) > 1` — same semantics, unwrap cannot fire → **2**, correct
- `(CASE WHEN column1 < 100 THEN column1 END) > 1` — NULL from a non-cast source → **2**, correct
- `try_cast(column1 AS DECIMAL(38,0)) > 1` — widening → 4, correct
- `try_cast(column1 AS INT) NOT IN (5)` → 1 either way; the physical copy has no `InList` path
**6. Pruning is not involved (control).** On `main` with the logical rule active, a full matrix of `parquet.pruning` x `parquet.pushdown_filters` x `parquet.enable_page_index` over the 2-row-group file above gives identical results in all 8 cells:
| predicate | all 8 configs | correct |
|---|---|---|
| `try_cast(column1 AS INT) > 1` | 4 | 2 |
| `try_cast(column1 AS INT) NOT IN (5)` | 3 | 1 |
| `try_cast(column1 AS INT) < 100` | 2 | 2 |
| `try_cast(column1 AS INT) IN (5)` | 1 | 1 |
| `try_cast(column1 AS INT) = 5` | 1 | 1 |
The wrong answers are stable across the whole matrix — the rewrite happens well before pruning, and pruning never changes the result. Recorded here so the pruning path is not suspected.
### Expected behavior
`try_cast(col AS T) op lit` must preserve `try_cast`'s NULL-on-overflow behaviour: rows whose value is not representable in `T` yield NULL, so the comparison is UNKNOWN and the row does not pass a filter.
Two possible fixes:
- Decline to unwrap when the cast **narrows** the column's type (i.e. the source domain is not a subset of the cast target). Widening `try_cast` stays eligible.
- Or unwrap with a range guard, roughly `col op lit AND col BETWEEN AND `, preserving NULL for out-of-range values.
The first is simpler and loses little; narrowing `try_cast` in a predicate is uncommon compared with the widening case the rule mainly exists to serve. It also has direct precedent in the codebase: both copies of the rule already decline two other narrowing families for exactly this many-to-one reason, via `is_timestamp_precision_narrowing_cast` and `is_date_narrowing_cast` in `datafusion/expr-common/src/casts.rs`. A numeric sibling alongside them, checked in both the logical rule (binary **and** `InList`) and the physical one, is the natural shape.
Note the `InList` path needs the same treatment, and `NOT IN` makes the consequence worse: `false` instead of NULL flips to `true` under negation and admits rows that should be excluded.
### Additional context
**Cross-engine check.** I verified the expected semantics against other engines rather than asserting them:
- **ClickHouse 26.1.1.144** is the one engine here that both expresses the case natively and gets it right — it has a real `Int32` and a real try-cast (`accurateCastOrNull`). Projection, `WHERE`, `IN` and `NOT IN` all return NULL / exclude the overflow row; `EXPLAIN actions=1` keeps `FUNCTION accurateCastOrNull(column1, 'Int32') -> Nullable(Int32)` in the filter rather than unwrapping it; and it agrees with its own constant-folded evaluation of the same expression.
- **PostgreSQL 17.6** has no `TRY_CAST` at all (`ERROR: syntax error`), and `9999999999::bigint::int` raises `ERROR: integer out of range`. On the nearest expressible analogs — a `CASE ... BETWEEN ... END` narrowing conversion, and one built on `pg_input_is_valid` — PostgreSQL returns NULL for the overflow row, excludes it from `WHERE`, returns NULL for both `IN` and `NOT IN`, and its planner leaves the comparison intact in `EXPLAIN (VERBOSE)` rather than simplifying it away.
- **SQLite 3.51.0** cannot express the case: `INTEGER` is 64-bit and there is no narrower integer type, so `CAST(9999999999 AS INT)` simply returns `9999999999`. On the `CASE ... BETWEEN` analog it is correct (NULL for the overflow row, NULL for both `IN` and `NOT IN`, row excluded from `WHERE`), and `EXPLAIN` shows the guard still evaluated per row — the conditional is never simplified out of the comparison.
- **DuckDB v1.5.2** reproduces DataFusion's behaviour (`true`), via what looks like the same rewrite — its plan shows `Filters: column1>1`. But that is corroboration of the bug, not of the semantics: DuckDB's own constant-folded evaluation of the *same expression* returns NULL —
```sql
SELECT TRY_CAST(9999999999::bigint AS INTEGER) AS c,
TRY_CAST(9999999999::bigint AS INTEGER) > 1 AS p;
-- c = NULL, p = NULL
```
and DuckDB is internally inconsistent on the same row of the same table: `IN (5)` yields `false` (cast unwrapped) while `NOT IN (5)` yields `NULL` (cast preserved). No coherent semantics produces that pair, so this reads as the same class of optimizer bug in both engines.
**Secondary question, not part of this report.** Narrowing plain `cast` behaves the same way — `cast(9999999999::bigint AS INT) > 1` returns `true` rather than raising an overflow error, via the same rewrite, even though the bare `SELECT cast(9999999999::bigint AS INT)` does raise `Arrow error: Cast error: Can't cast value 9999999999 to type Int32`. Expected semantics for strict `cast` are less clear-cut than for `try_cast`, so I am raising it as a question rather than claiming it is a bug.
There is a related consequence on the schema-evolution path, with **default** settings, which may deserve its own issue: an external table declaring `column1 INT` over a parquet file whose column is `INT64` errors with `Can't cast value 9999999999 to type Int32` when `pushdown_filters=false`, but silently returns rows when `pushdown_filters=true` (`SELECT count(*) ... WHERE column1 > 100` gives 2 instead of an error). The adapter inserts a strict narrowing cast and the physical simplifier unwraps it out of the pushed-down filter, so the overflowing values are never materialised.
Found while investigating `IN`-list statistics pruning (#24526); this rule runs well before pruning, so it is independent of that work.
Contributor guide
Assessment
This issue has not been assessed yet.