Skip to content

Unsafe comparison cast rewriting in ExprSimplifier silently produces wrong query results #22142

Closed
#22837
#22906
Closed
Unsafe comparison cast rewriting in ExprSimplifier silently produces wrong query results#22142
#22837
#22906
Labels
bugSomething isn't working

Description

@discord9
discord9
opened on May 13, 2026

Describe the bug

The unwrap_cast optimization in ExprSimplifier (both logical and physical) rewrites patterns like CAST(column AS target_type) <op> literal_target into column <op> CAST(literal_target AS source_type) without verifying that the rewrite preserves query semantics. This default-allow behavior silently produces incorrect results for many common cast patterns.

Specific broken scenarios

Timestamp precision downcast (many→one rewriting):

-- ts_ns has values 1704067200001000000 and 1704067200001234567
-- Both cast to the same timestamp(ms)
SELECT * FROM t WHERE CAST(ts_ns AS timestamp(ms)) = timestamp_ms_value;
-- Rewrites to: WHERE ts_ns = <nanosecond-literal>
-- Result: only matches ONE nanosecond value, not both → WRONG row count

Source domain not preserved (overflow/null loss):

-- column1 is Int64, literal is 9999999999 (doesn't fit in Int32)
SELECT * FROM t WHERE CAST(column1 AS Int32) < 9999999999;
-- Rewrites to: WHERE column1 < CAST(9999999999 AS Int32)
-- The literal cast silently truncates → WRONG predicate

Type semantics changed (string round-trip, float truncation):

SELECT * FROM t WHERE CAST(int_col AS Utf8) = '0123';
-- Rewrites to: WHERE int_col = 0123
-- '0123' is NOT equivalent to 123 → WRONG comparison

Timezone silently dropped:

SELECT * FROM t WHERE CAST(ts AS timestamp(ms, 'UTC')) = timestamp_literal;
-- Rewrites without preserving the timezone → semantics changed

Additional affected patterns

  • Int→UInt (negative values become NULL, rewrite doesn't account)
  • UInt64→Int64 (large UInt64 values above Int64::MAX)
  • Decimal scale downcast (fractional digits truncated)
  • Decimal→Int (truncation not preserved)
  • Float→Int / Float64→Float32 (precision loss)
  • Date↔Timestamp (many→one)
  • Dictionary↔Utf8 (dictionary key semantics)

Impact

The optimizer is silently producing wrong query results. There is no error or warning — the plan looks correct and the query returns results that are simply semantically wrong. This has existed since the feature was introduced.

Proposed fix

Replace the default-allow logic with a closed-by-default allowlist. Only cast patterns proven safe (with literal round-trip verification) should be eligible. Implementation in #21908.

  • Closed-by-default: no cast is unwrapped unless explicitly allowlisted
  • Current allowlist: same-timezone timestamp precision widening only
  • Require exact literal round-trip: lit to source_type and back must equal the original
  • All other casts (int widenings, string to int, decimals, dictionaries) intentionally kept in plan

To Reproduce

See datafusion/sqllogictest/test_files/unwrap_cast.slt or the unit tests in datafusion/expr-common/src/casts.rs, datafusion/optimizer/src/simplify_expressions/unwrap_cast.rs, datafusion/physical-expr/src/simplifier/unwrap_cast.rs.

Activity

  1. alamb commented on Jun 16, 2026

    @alamb
    alamb
    on Jun 16, 2026
    Contributor

    Ok, I have found this whole issue quite confusing as it is not clear to me what the scope of the issue is from this ticket as it describes a code problem, rather than the wrong results that can result. Specifically it is not clear to me from this issue, nor the various PRs that have been proposed, if this is something specific to timestamps or if it is a more general problem

    -- Two DISTINCT nanosecond timestamps that both truncate to the SAME millisecond.
CREATE TABLE t AS
SELECT arrow_cast(1704067200001000000, 'Timestamp(Nanosecond, None)') AS ts_ns
UNION ALL
SELECT arrow_cast(1704067200001234567, 'Timestamp(Nanosecond, None)');


-- WRONG: should return 2 rows, returns 1.
SELECT * FROM t
WHERE arrow_cast(ts_ns, 'Timestamp(Millisecond, None)') = arrow_cast(1704067200001, 'Timestamp(Millisecond, None)');

Elapsed 0.002 seconds.

+-------------------------+
| ts_ns                   |
+-------------------------+
| 2024-01-01T00:00:00.001 |
+-------------------------+
1 row(s) fetched.
Elapsed 0.000 seconds.

    You can see this is incorrect as you both rows have the truncated values

    SELECT arrow_cast(ts_ns, 'Timestamp(Millisecond, None)') from t;
+----------------------------------------------------------+
| arrow_cast(t.ts_ns,Utf8("Timestamp(Millisecond, None)")) |
+----------------------------------------------------------+
| 2024-01-01T00:00:00.001                                  |
| 2024-01-01T00:00:00.001                                  |
+----------------------------------------------------------+
2 row(s) fetched.
Elapsed 0.000 seconds.

    The same thing happens with decimals:

    CREATE TABLE t(d DECIMAL(20,4)) AS VALUES (1.2345), (1.2399), (1.2500);
-- 1.2345 -> 1.23 at scale 2, so this MUST return 1 row:
SELECT * FROM t
WHERE arrow_cast(d, 'Decimal128(20, 2)') = arrow_cast(1.23, 'Decimal128(20, 2)');
-- predicate rewritten to `d = 1.2300` (exact)  ->  returns 0 rows.  WRONG.
+---+
| d |
+---+
+---+
0 row(s) fetched.
Elapsed 0.001 seconds.

  2. alamb commented on Jun 16, 2026

    @alamb
    alamb
    on Jun 16, 2026
    Contributor

    The original optimization was based on Spark's UnwrapCastInBinaryComparison as I recall: https://github.com/apache/spark/blob/a6e3fdd504cbff4dff739801e0b0b0f2b6402502/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala#L30-L101

    It seems that that code has a 2 layer check,

    Layer 1: first on data types
    https://github.com/apache/spark/blob/a6e3fdd504cbff4dff739801e0b0b0f2b6402502/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala#L462-L485

    Layer 2: checks if the literal can be casted losslessly
    https://github.com/apache/spark/blob/a6e3fdd504cbff4dff739801e0b0b0f2b6402502/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala#L112
    https://github.com/apache/spark/blob/a6e3fdd504cbff4dff739801e0b0b0f2b6402502/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala#L282-L340

    This seems somewhat equivalent to try_cast_literal_to_type https://github.com/apache/datafusion/blob/eedae1154bf2745ea6d025f3e55901db1d8b7fb7/datafusion/expr-common/src/casts.rs#L59-L58

    So it seems to me that DataFusion is missing the Layer 1 check, and it seems like your PR in #22837 is adding this check.

    So in my opinion, we should proceed with #22837 and I will do so

  3. discord9 commented on Jun 17, 2026

    @discord9
    discord9
    on Jun 17, 2026
    ContributorAuthor

    Ok, I have found this whole issue quite confusing as it is not clear to me what the scope of the issue is from this ticket as it describes a code problem, rather than the wrong results that can result. Specifically it is not clear to me from this issue, nor the various PRs that have been proposed, if this is something specific to timestamps or if it is a more general problem

    -- Two DISTINCT nanosecond timestamps that both truncate to the SAME millisecond.
    CREATE TABLE t AS
    SELECT arrow_cast(1704067200001000000, 'Timestamp(Nanosecond, None)') AS ts_ns
    UNION ALL
    SELECT arrow_cast(1704067200001234567, 'Timestamp(Nanosecond, None)');

    -- WRONG: should return 2 rows, returns 1.
    SELECT * FROM t
    WHERE arrow_cast(ts_ns, 'Timestamp(Millisecond, None)') = arrow_cast(1704067200001, 'Timestamp(Millisecond, None)');

    Elapsed 0.002 seconds.

    +-------------------------+
    | ts_ns |
    +-------------------------+
    | 2024-01-01T00:00:00.001 |
    +-------------------------+
    1 row(s) fetched.
    Elapsed 0.000 seconds.

    You can see this is incorrect as you both rows have the truncated values

    SELECT arrow_cast(ts_ns, 'Timestamp(Millisecond, None)') from t;
    +----------------------------------------------------------+
    | arrow_cast(t.ts_ns,Utf8("Timestamp(Millisecond, None)")) |
    +----------------------------------------------------------+
    | 2024-01-01T00:00:00.001 |
    | 2024-01-01T00:00:00.001 |
    +----------------------------------------------------------+
    2 row(s) fetched.
    Elapsed 0.000 seconds.

    The same thing happens with decimals:

    CREATE TABLE t(d DECIMAL(20,4)) AS VALUES (1.2345), (1.2399), (1.2500);
    -- 1.2345 -> 1.23 at scale 2, so this MUST return 1 row:
    SELECT * FROM t
    WHERE arrow_cast(d, 'Decimal128(20, 2)') = arrow_cast(1.23, 'Decimal128(20, 2)');
    -- predicate rewritten to d = 1.2300 (exact) -> returns 0 rows. WRONG.
    +---+
    | d |
    +---+
    +---+
    0 row(s) fetched.
    Elapsed 0.001 seconds.

    sorry I didn't make it more clear, it's not just timestamp but all kind of types cast could have similar issues, I just want to keep pr small and fix them one by one without one massive pr blocking all wrong cast, the block list one I will get to merge first, then maybe could look into the preimage cast one which allow a wider range of correct cast unwrap

  4. alamb
    closed this as completedin #22837on Jun 22, 2026
  5. alamb
    added
    bugSomething isn't working
    on Jun 22, 2026
  6. mkleen
    added a commit that references this issue on Jun 22, 2026

    fix: block timestamp precision narrowing unwrap (apache#22837)

    7ced0ac
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions