You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/09/28 03:43:40 UTC
(datafusion) branch gh-readonly-queue/main/pr-25795-39ca2c74a9f678ee0f61cd7753b035858c2b9df2 created (now ed43e69fdc)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-25795-39ca2c74a9f678ee0f61cd7753b035858c2b9df2
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at ed43e69fdc fix: preserve unbounded multiplication interval endpoints (#25795)
This branch includes the following new commits:
new ed43e69fdc fix: preserve unbounded multiplication interval endpoints (#25795)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: preserve unbounded multiplication interval endpoints (#25795)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25795-39ca2c74a9f678ee0f61cd7753b035858c2b9df2
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit ed43e69fdca8737c9d065b53b5fc5e4183302775
Author: Huaijin <ha...@gmail.com>
AuthorDate: Mon Sep 28 03:43:20 2026 +0000
fix: preserve unbounded multiplication interval endpoints (#25795)
## Which issue does this PR close?
Closes #25794
## Rationale for this change
Integer multiplication overflow can produce false-empty filter
statistics despite returning matching rows.
For TINYINT inputs `(0, -3)` and `(20, 7)`, `SELECT a, b, a * b AS x
FROM mul_wrap WHERE a * b < -100` has the following behavior:
| | Before | After |
| --- | --- | --- |
| Returned row | `(20, 7, -116)` | `(20, 7, -116)` |
| Estimated rows | `Inexact(0)` | `Inexact(2)` |
| Column a bounds | `Min=Exact(Int8(NULL)), Max=Exact(Int8(NULL))` |
`Min=Inexact(Int8(0)), Max=Inexact(Int8(20))` |
| Column b bounds | `Min=Exact(Int8(NULL)), Max=Exact(Int8(NULL))` |
`Min=Inexact(Int8(-3)), Max=Inexact(Int8(7))` |
| Distinct counts | `Exact(0)` for both columns | Unknown |
The estimate of 2 is conservative, not the exact count of the one
surviving row. The fix removes the false-empty inference without
changing runtime arithmetic or query results in these reproductions.
Related: #25232 and the merged fix #25234. That fix guards wrapping
arithmetic when the inferred interval is unbounded. Here, the
lower-level multiplication helper discards an overflow-generated
unbounded endpoint, preventing that guard from activating.
## What changes are included in this PR?
Distinguish lower- and upper-bound comparisons so multiplication
preserves overflow-generated unbounded endpoints. Reuse the helpers in
intersection and union, preserving their behavior.
In the example, interval multiplication now retains `[-60, +infinity]`
instead of `[-60, 0]`. The existing wrapping-arithmetic protection then
widens the expression range to fully unbounded, allowing the wrapped
value `-116`.
## What is the testing strategy for this PR?
Add interval overflow cases and a Parquet SQL regression checking
results and statistics. Validation on the PR branch:
- `cargo test -p datafusion-expr-common --lib
interval_arithmetic::tests` — 45 passed.
- `cargo test --test sqllogictests -- parquet_statistics` — passed.
## Are there any user-facing changes?
Affected filters report conservative statistics instead of false-empty
statistics. No public API changes.
---
datafusion/expr-common/src/interval_arithmetic.rs | 78 +++++++++++++++-------
.../sqllogictest/test_files/parquet_statistics.slt | 36 ++++++++++
2 files changed, 90 insertions(+), 24 deletions(-)
diff --git a/datafusion/expr-common/src/interval_arithmetic.rs b/datafusion/expr-common/src/interval_arithmetic.rs
index 9f1291353d..48bf8a391d 100644
--- a/datafusion/expr-common/src/interval_arithmetic.rs
+++ b/datafusion/expr-common/src/interval_arithmetic.rs
@@ -663,8 +663,8 @@ impl Interval {
return Ok(None);
}
- let lower = max_of_bounds(&lhs.lower, &rhs.lower);
- let upper = min_of_bounds(&lhs.upper, &rhs.upper);
+ let lower = max_lower_bound(&lhs.lower, &rhs.lower);
+ let upper = min_upper_bound(&lhs.upper, &rhs.upper);
// New lower and upper bounds must always construct a valid interval.
debug_assert!(
@@ -686,18 +686,8 @@ impl Interval {
let lhs = lhs_owned.as_ref().unwrap_or(self);
let rhs = rhs_owned.as_ref().unwrap_or(rhs);
- let lower =
- if lhs.lower.is_null() || (!rhs.lower.is_null() && lhs.lower <= rhs.lower) {
- lhs.lower.clone()
- } else {
- rhs.lower.clone()
- };
- let upper =
- if lhs.upper.is_null() || (!rhs.upper.is_null() && lhs.upper >= rhs.upper) {
- lhs.upper.clone()
- } else {
- rhs.upper.clone()
- };
+ let lower = min_lower_bound(&lhs.lower, &rhs.lower);
+ let upper = max_upper_bound(&lhs.upper, &rhs.upper);
// New lower and upper bounds must always construct a valid interval.
debug_assert!(
@@ -1346,9 +1336,17 @@ fn next_value_helper<const INC: bool>(value: ScalarValue) -> ScalarValue {
}
}
-/// Returns the greater of the given interval bounds. Assumes that a `NULL`
-/// value represents `NEG_INF`.
-fn max_of_bounds(first: &ScalarValue, second: &ScalarValue) -> ScalarValue {
+/// Returns the lesser of two lower bounds, treating `NULL` as `NEG_INF`.
+fn min_lower_bound(first: &ScalarValue, second: &ScalarValue) -> ScalarValue {
+ if first.is_null() || (!second.is_null() && first <= second) {
+ first.clone()
+ } else {
+ second.clone()
+ }
+}
+
+/// Returns the greater of two lower bounds, treating `NULL` as `NEG_INF`.
+fn max_lower_bound(first: &ScalarValue, second: &ScalarValue) -> ScalarValue {
if !first.is_null() && (second.is_null() || first >= second) {
first.clone()
} else {
@@ -1356,9 +1354,8 @@ fn max_of_bounds(first: &ScalarValue, second: &ScalarValue) -> ScalarValue {
}
}
-/// Returns the lesser of the given interval bounds. Assumes that a `NULL`
-/// value represents `INF`.
-fn min_of_bounds(first: &ScalarValue, second: &ScalarValue) -> ScalarValue {
+/// Returns the lesser of two upper bounds, treating `NULL` as `INF`.
+fn min_upper_bound(first: &ScalarValue, second: &ScalarValue) -> ScalarValue {
if !first.is_null() && (second.is_null() || first <= second) {
first.clone()
} else {
@@ -1366,6 +1363,15 @@ fn min_of_bounds(first: &ScalarValue, second: &ScalarValue) -> ScalarValue {
}
}
+/// Returns the greater of two upper bounds, treating `NULL` as `INF`.
+fn max_upper_bound(first: &ScalarValue, second: &ScalarValue) -> ScalarValue {
+ if first.is_null() || (!second.is_null() && first >= second) {
+ first.clone()
+ } else {
+ second.clone()
+ }
+}
+
/// This function updates the given intervals by enforcing (i.e. propagating)
/// the inequality `left > right` (or the `left >= right` inequality, if `strict`
/// is `true`).
@@ -1482,13 +1488,14 @@ fn mul_helper_multi_zero_inclusive(
{
return Interval::make_unbounded(dt).unwrap();
}
- // Since unbounded cases are handled above, we can safely
- // use the utility functions here to eliminate code duplication.
- let lower = min_of_bounds(
+ // Even finite inputs can produce unbounded endpoints through overflow.
+ // When combining the products, NULL lower bounds represent NEG_INF and
+ // NULL upper bounds represent INF, so either must dominate finite bounds.
+ let lower = min_lower_bound(
&mul_bounds::<false>(dt, &lhs.lower, &rhs.upper),
&mul_bounds::<false>(dt, &rhs.lower, &lhs.upper),
);
- let upper = max_of_bounds(
+ let upper = max_upper_bound(
&mul_bounds::<true>(dt, &lhs.upper, &rhs.upper),
&mul_bounds::<true>(dt, &lhs.lower, &rhs.lower),
);
@@ -3481,6 +3488,29 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_mul_zero_inclusive_overflow() -> Result<()> {
+ // Both operands contain zero. Overflow in either candidate endpoint
+ // must not be hidden by the other, finite candidate.
+ let cases = [
+ ((0_i8, 20_i8), (-3, 7), (Some(-60_i8), None)),
+ ((-20, 0), (-7, 3), (Some(-60), None)),
+ ((-20, 0), (-3, 7), (None, Some(60))),
+ ((0, 20), (-7, 3), (None, Some(60))),
+ ((-20, 20), (-7, 7), (None, None)),
+ ((-3, 3), (-7, 7), (Some(-21), Some(21))),
+ ((0, 0), (i8::MIN, i8::MAX), (Some(0), Some(0))),
+ ];
+ for ((lo, hi), (rlo, rhi), (expected_lo, expected_hi)) in cases {
+ let lhs = Interval::make(Some(lo), Some(hi))?;
+ let rhs = Interval::make(Some(rlo), Some(rhi))?;
+ let expected = Interval::make(expected_lo, expected_hi)?;
+ assert_eq!(lhs.mul(&rhs)?, expected, "{lhs:?} * {rhs:?}");
+ assert_eq!(rhs.mul(&lhs)?, expected, "{rhs:?} * {lhs:?}");
+ }
+ Ok(())
+ }
+
#[test]
fn test_mul() -> Result<()> {
let cases = vec![
diff --git a/datafusion/sqllogictest/test_files/parquet_statistics.slt b/datafusion/sqllogictest/test_files/parquet_statistics.slt
index 2c9a29c74b..71de383370 100644
--- a/datafusion/sqllogictest/test_files/parquet_statistics.slt
+++ b/datafusion/sqllogictest/test_files/parquet_statistics.slt
@@ -186,6 +186,42 @@ physical_plan
statement ok
DROP TABLE typed_table;
+######
+# Wrapping multiplication must not produce empty filter statistics
+######
+
+statement ok
+SET datafusion.execution.target_partitions = 1;
+
+statement ok
+COPY (
+ SELECT CAST(a AS TINYINT) AS a, CAST(b AS TINYINT) AS b
+ FROM (VALUES (0, -3), (20, 7)) AS t(a, b)
+)
+TO 'test_files/scratch/parquet_statistics/mul_wrap.parquet' STORED AS PARQUET;
+
+statement ok
+CREATE EXTERNAL TABLE mul_wrap STORED AS PARQUET
+LOCATION 'test_files/scratch/parquet_statistics/mul_wrap.parquet';
+
+# 20 * 7 wraps to -116 in Int8. Overflow in [0,20] * [-3,7] must
+# remain visible to interval analysis, rather than producing [-60,0]
+# and incorrectly concluding that the filter cannot match any rows.
+query III
+SELECT a, b, a * b AS x FROM mul_wrap WHERE a * b < -100;
+----
+20 7 -116
+
+query TT
+EXPLAIN SELECT a, b FROM mul_wrap WHERE a * b < -100;
+----
+physical_plan
+01)FilterExec: a@0 * b@1 < -100, statistics=[Rows=Inexact(2), Bytes=Inexact(4), [(Col[0]: Min=Inexact(Int8(0)) Max=Inexact(Int8(20)) Null=Inexact(0) ScanBytes=Inexact(2)),(Col[1]: Min=Inexact(Int8(-3)) Max=Inexact(Int8(7)) Null=Inexact(0) ScanBytes=Inexact(2))]]
+02)--DataSourceExec: file_groups={1 group: [[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/parquet_statistics/mul_wrap.parquet]]}, projection=[a, b], file_type=parquet, predicate=a@0 * b@1 < -100, statistics=[Rows=Inexact(2), Bytes=Inexact(4), [(Col[0]: Min=Inexact(Int8(0)) Max=Inexact(Int8(20)) Null=Inexact(0) ScanBytes=Inexact(2)),(Col[1]: Min=Inexact(Int8(-3)) Max=Inexact(Int8(7)) Null=Inexact(0) ScanBytes=Inexact(2))]]
+
+statement ok
+DROP TABLE mul_wrap;
+
######
# Aggregate statistics through casts
######
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org