discord9 opened a new pull request, #26216: URL: https://github.com/apache/datafusion/pull/26216
## Which issue does this PR close? - Closes #. (Upstream report: spiceai/spiceai#14969 — tracked downstream; no apache/datafusion issue was filed for it yet.) **Stacked on #22906** (`1e127365510c83cfffec5fed6f72a56b5875d5b0`); please review/merge that one first. ## Rationale for this change `CAST(decimal_col AS BIGINT) = 173665` returned the wrong rows: the unwrap-cast rewrite folded it to the exact decimal comparison `price = 173665.00`, but decimal-to-integer casts truncate toward zero, so that predicate only matched the single boundary value. With `DECIMAL(15, 2)` prices 173665.29 / .47 / .73 / .91, four matching rows returned zero (spiceai/spiceai#14969). Range predicates on the same expression were likewise wrong. The DOUBLE route was not rewritten and returned correct rows, confirming the rewrite as the cause. ## What changes are included in this PR? - `datafusion/expr-common/src/casts.rs`: new decimal-to-integer `CastPredicatePreimage::Range` for `Decimal128(p, s)` sources with `s > 0` (one level of `Dictionary` unwrapped). `CAST(col AS INT) OP literal` rewrites to source-domain decimal bounds using the same truncation-toward-zero buckets as timestamp precision narrowing (`v > 0 → [v·10^s, (v+1)·10^s)`, `v == 0 → [1-10^s, 10^s)`, `v < 0 → [(v-1)·10^s+1, v·10^s+1)`). `trunc_toward_zero_bucket` generalized `i64 → i128` and reused; no second bucket implementation. Bounds use checked `i128` arithmetic validated against the source precision: any unrepresentable bound retains the cast. Decimal32/64/256, scale 0, and non-integer literals are out of scope. No exact preimage can be produced for decimal→int (`is_exact_cast_safe` still rejects it), so `IN` lists are untouched. - `datafusion/sqllogictest/test_files/simplify_expr.slt`: end-to-end cases on the issue's exact 8-row dataset, including the regression (`= 173665` → rows 2–5), strict/non-strict ranges, negative and zero buckets, NULL-row semantics for `IS [NOT] DISTINCT FROM` and `!=`, EXPLAIN plans showing the source-domain rewrite, and the retained-cast case for an unrepresentable bucket. - Logical (`optimizer`) and physical (`physical-expr`) consumers already dispatch Range preimages and guard volatile-input duplication, so they are unchanged. ## What is the testing strategy for this PR? - `cargo test -p datafusion-expr-common --lib`: 239 passed, including 7 new unit tests for bucket bounds per sign, all operators, unrepresentable-bound rejection, scale-0 rejection, Dictionary sources, Decimal256 rejection, out-of-range literal rejection. - `cargo test -p datafusion-optimizer --lib` (928) and `cargo test -p datafusion-physical-expr --lib` (1695, 2 ignored): pass. - `cargo test -p datafusion-sqllogictest --test sqllogictests -- simplify_expr`: pass. - `cargo fmt --all --check`, `cargo clippy -p datafusion-expr-common --all-targets -- -D warnings`, `git diff --check`: pass. ## Are there any user-facing changes? Yes: filters comparing `CAST(decimal AS integer)` to integer literals now return the correct rows (previously could silently return fewer/more rows). Queries that relied on the old wrong behavior would change results. No API changes. For values whose cast would error (out of target integer range), the rewritten predicate evaluates to a Boolean instead of erroring, matching the already-accepted overflow policy of the timestamp range preimages; this is documented on the new function. -- 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]
