Repository navigation
Conversation
Casting DECIMAL(p, s) with s > 0 to an integer truncates toward zero, so an integer literal describes a bucket of source values. Folding such a comparison into an exact decimal equality keeps only the boundary value and silently drops the other rows: for DECIMAL(15, 2) prices 173665.29, 173665.47, 173665.73 and 173665.91, CAST(price AS BIGINT) = 173665 returned no rows instead of four (spiceai/spiceai#14969). Extend the cast-predicate preimage machinery with a decimal-to-integer Range preimage. CAST(col AS INT) OP literal now rewrites to source-domain decimal bounds using the same truncation-toward-zero buckets as timestamp precision narrowing, for every supported comparison operator. Buckets whose bounds the source decimal cannot represent retain the cast, and Decimal32/64/256, scale 0, and non-integer literals are out of scope. The logical and physical consumers already dispatch Range preimages for timestamp narrowing, so no consumer code changes; both guard input duplication for volatile expressions on the equality family. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Stacked on apache#22906 (
1e127365510c83cfffec5fed6f72a56b5875d5b0). The wrong-rows regression itself is fixed by apache#22906's allowlist; this PR adds the optimization on top.Rationale for this change
CAST(decimal_col AS BIGINT) = 173665is a many-to-one comparison (decimal→integer truncates toward zero), so apache#22906 correctly refuses to fold it and keeps the cast on the column. That is correct but leaves performance on the table: the predicate cannot be pushed into min/max statistics or bloom pruning, and every row pays the cast.The same predicate has an exact source-domain form:
price >= 173665.00 AND price < 173666.00. Rewriting to that bucket keeps identical results while removing the cast and enabling pruning.What changes are included in this PR?
datafusion/expr-common/src/casts.rs: new decimal-to-integerCastPredicatePreimage::RangeforDecimal128(p, s)sources withs > 0(one level ofDictionaryunwrapped).CAST(col AS INT) OP literalrewrites 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_bucketgeneralizedi64 → i128and reused; no second bucket implementation. Bounds use checkedi128arithmetic 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_safestill rejects it), soINlists are untouched.datafusion/sqllogictest/test_files/simplify_expr.slt: end-to-end cases on the spiceai#14969 dataset (8 rows), including= 173665→ rows 2–5, strict/non-strict ranges, negative and zero buckets, NULL-row semantics forIS [NOT] DISTINCT FROMand!=, EXPLAIN plans showing the source-domain rewrite, and the retained-cast case for an unrepresentable bucket.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) andcargo 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?
Performance: predicates comparing
CAST(decimal AS integer)to integer literals are now rewritten to source-domain decimal ranges, removing the per-row cast and enabling min/max and bloom pruning. Result sets are unchanged relative to this PR's base (apache#22906).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. No API changes.