Repository navigation
Conversation
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <discord9@163.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Merge Apache main at 9547b09 without rewriting the reviewed PR history. Reject naive-to-timezone-aware timestamp preimages in both comparison and direct exact-conversion paths, retaining Arrow's timezone adjustment. Remove redundant Date64 narrowing guards and return an internal error for unsupported physical comparison operators. Add result-level coverage for equal-unit, widening, narrowing and multi-item IN predicates. Correct the existing cross-timezone equality expectations using raw-instant and column-versus-column controls, while preserving a true matching positive case. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Merge Apache main at 97c7593 after publishing the reviewed cast-predicate follow-up. Preserve the existing review history and timezone-aware cast safeguards without rebasing. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Deprecate the retained date-narrowing helper for 56.0.0 and keep its existing behavior covered. Extract input-duplication metadata onto CastPredicatePreimage and use it in the logical volatility guard. Preserve existing cast comparison, timezone and NULL semantics without changing physical rewrite logic or the supported cast allowlist. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
Merge Apache main 55b2f09, the base used by the failing PR CI checkout, without rewriting review history. Preserve the original decimal(38,2) = 4.00 query and its result and statistics assertions. Its retained decimal cast narrows integer capacity, prevents a literal guarantee, and does not evaluate Bloom filters. Account for that explicitly under the updated Bloom metrics. Add a source-typed decimal equality case that actually evaluates both surviving Bloom filters, and cover a representable comparison literal whose column cast must still be retained because valid source values can overflow the strict Arrow cast. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
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>
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #26216 +/- ##
==========================================
+ Coverage 82.74% 82.83% +0.08%
==========================================
Files 1147 1149 +2
Lines 449755 453468 +3713
Branches 449755 453468 +3713
==========================================
+ Hits 372168 375629 +3461
- Misses 54915 55015 +100
- Partials 22672 22824 +152 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
discord9
marked this pull request as draft
October 12, 2026 04:58
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 #22906 (
1e127365510c83cfffec5fed6f72a56b5875d5b0); please review/merge that one first. The wrong-rows regression itself is fixed by #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 #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 (#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.