adriangb opened a new issue, #24702:
URL: https://github.com/apache/datafusion/issues/24702
### 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 <narrower type>) 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.
### To Reproduce
On `main` (b9a0053677).
**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
+------------+---+------+
```
**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.
**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
```
### 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 <T::MIN>
AND <T::MAX>`, 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.
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:
- **PostgreSQL 17** 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`, and its planner leaves the comparison intact in
`EXPLAIN (VERBOSE)` rather than simplifying it away.
- **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. 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.
Found while investigating `IN`-list statistics pruning (#24526); this rule
runs well before pruning, so it is independent of that work.
--
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]
