kosiew commented on code in PR #26776:
URL: https://github.com/apache/datafusion/pull/26776#discussion_r4562830016
##########
datafusion/physical-expr/src/equivalence/properties/mod.rs:
##########
@@ -1708,6 +1812,39 @@ fn get_expr_properties(
}
}
+fn column_range(
+ expr: &Arc<dyn PhysicalExpr>,
+ schema: &SchemaRef,
+ column_ranges: Option<&ColumnRanges>,
+) -> Result<Interval> {
+ let data_type = expr.data_type(schema)?;
+ if let Some(column) = expr.downcast_ref::<Column>()
+ && let Some(range) = column_ranges.and_then(|ranges|
ranges.get(&column.index()))
+ && range.data_type() == data_type
+ {
+ return Ok(range.clone());
+ }
+ Interval::make_unbounded(&data_type)
+}
+
+fn integer_range(
+ data_type: &DataType,
+ statistics: &ColumnStatistics,
+) -> Option<Interval> {
+ if data_type.is_integer()
+ && let (Precision::Exact(min), Precision::Exact(max)) =
+ (&statistics.min_value, &statistics.max_value)
+ && !min.is_null()
Review Comment:
Consider adding table-driven tests through `with_column_statistics` for
inexact, missing, null, reversed, and type-mismatched extrema. Verify that
rejected bounds cannot prove ordering for `a + 1`, while exact bounds `[1, 3]`
can. Nullable columns should remain eligible when their extrema are valid.
##########
datafusion/sqllogictest/test_files/parquet_sorted_statistics.slt:
##########
@@ -542,13 +542,288 @@ DROP TABLE nulls_first_bad;
statement ok
DROP TABLE nulls_first_ok;
-# Config reset
-
# The SLT runner sets `target_partitions` to 4 instead of using the default, so
# reset it explicitly.
statement ok
set datafusion.execution.target_partitions = 4;
+# Exact integer bounds prove that these projections cannot wrap (#25220).
+statement ok
+SET datafusion.explain.physical_plan_only = true;
+
+statement ok
+CREATE TABLE arithmetic_bounds_source (a BIGINT, b BIGINT, u BIGINT UNSIGNED)
AS VALUES
+(1, 9223372036854775805, 0),
+(2, 9223372036854775806, 1),
+(3, 9223372036854775807, 2),
+(NULL, NULL, NULL);
+
+query I
+COPY (SELECT * FROM arithmetic_bounds_source ORDER BY a)
+TO 'test_files/scratch/parquet_sorted_statistics/arithmetic_bounds.parquet'
+STORED AS PARQUET;
+----
+4
+
+statement ok
+CREATE EXTERNAL TABLE arithmetic_bounds
+STORED AS PARQUET
+WITH ORDER (a ASC NULLS LAST)
+WITH ORDER (b ASC NULLS LAST)
+WITH ORDER (u ASC NULLS LAST)
+LOCATION
'test_files/scratch/parquet_sorted_statistics/arithmetic_bounds.parquet';
+
+query TT
+EXPLAIN SELECT 1 - a AS x FROM arithmetic_bounds ORDER BY x DESC NULLS LAST;
+----
+physical_plan DataSourceExec: file_groups={1 group:
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/parquet_sorted_statistics/arithmetic_bounds.parquet]]},
projection=[1 - a@0 as x], file_type=parquet
+
+query I
+SELECT 1 - a AS x FROM arithmetic_bounds ORDER BY x DESC NULLS LAST;
+----
+0
+-1
+-2
+NULL
+
+# Window outputs retain the input bounds needed by the outer projection.
+query TT
+EXPLAIN SELECT a + 1 AS x, rn
+FROM (SELECT a, row_number() OVER (ORDER BY a) AS rn FROM arithmetic_bounds)
+ORDER BY x;
+----
+physical_plan
+01)ProjectionExec: expr=[a@0 + 1 as x, row_number() ORDER BY
[arithmetic_bounds.a ASC NULLS LAST] RANGE BETWEEN UNBOUNDED PRECEDING AND
CURRENT ROW@1 as rn]
+02)--BoundedWindowAggExec: wdw=[row_number() ORDER BY [arithmetic_bounds.a ASC
NULLS LAST] RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW: Field {
"row_number() ORDER BY [arithmetic_bounds.a ASC NULLS LAST] RANGE BETWEEN
UNBOUNDED PRECEDING AND CURRENT ROW": UInt64 }, frame: RANGE BETWEEN UNBOUNDED
PRECEDING AND CURRENT ROW], mode=[Sorted]
+03)----DataSourceExec: file_groups={1 group:
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/parquet_sorted_statistics/arithmetic_bounds.parquet]]},
projection=[a], output_ordering=[a@0 ASC NULLS LAST], file_type=parquet
+
+query II
+SELECT a + 1 AS x, rn
+FROM (SELECT a, row_number() OVER (ORDER BY a) AS rn FROM arithmetic_bounds)
+ORDER BY x;
+----
+2 1
+3 2
+4 3
+NULL 4
+
+# Computed bounds survive consecutive projections and column reordering.
+query TT
+EXPLAIN SELECT u, a + 1 AS x, (a + 1) - 2 AS y FROM arithmetic_bounds ORDER BY
y;
+----
+physical_plan
+01)ProjectionExec: expr=[u@1 as u, __common_expr_1@0 as x, __common_expr_1@0 -
2 as y]
+02)--DataSourceExec: file_groups={1 group:
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/parquet_sorted_statistics/arithmetic_bounds.parquet]]},
projection=[a@0 + 1 as __common_expr_1, u], output_ordering=[u@1 ASC NULLS
LAST], file_type=parquet
+
+query III
+SELECT u, a + 1 AS x, (a + 1) - 2 AS y FROM arithmetic_bounds ORDER BY y;
+----
+0 2 0
+1 3 1
+2 4 2
+NULL NULL NULL
+
+# Reordered bounds must still detect signed overflow.
+query TT
+EXPLAIN SELECT a, b - 1 AS x, (b - 1) + 2 AS y FROM arithmetic_bounds ORDER BY
y;
+----
+physical_plan
+01)SortExec: expr=[y@2 ASC NULLS LAST], preserve_partitioning=[false]
+02)--ProjectionExec: expr=[a@1 as a, __common_expr_1@0 as x, __common_expr_1@0
+ 2 as y]
+03)----DataSourceExec: file_groups={1 group:
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/parquet_sorted_statistics/arithmetic_bounds.parquet]]},
projection=[b@1 - 1 as __common_expr_1, a], output_ordering=[a@1 ASC NULLS
LAST], file_type=parquet
+
+query III
+SELECT a, b - 1 AS x, (b - 1) + 2 AS y FROM arithmetic_bounds ORDER BY y;
+----
+3 9223372036854775806 -9223372036854775808
+1 9223372036854775804 9223372036854775806
+2 9223372036854775805 9223372036854775807
+NULL NULL NULL
+
+statement ok
+CREATE EXTERNAL TABLE expression_ordered_bounds
+STORED AS PARQUET
+WITH ORDER (a + 1 ASC NULLS LAST)
+LOCATION
'test_files/scratch/parquet_sorted_statistics/arithmetic_bounds.parquet';
+
+query TT
+EXPLAIN SELECT (a + 1) - 2 AS x FROM expression_ordered_bounds ORDER BY x;
+----
+physical_plan DataSourceExec: file_groups={1 group:
[[WORKSPACE_ROOT/datafusion/sqllogictest/test_files/scratch/parquet_sorted_statistics/arithmetic_bounds.parquet]]},
projection=[a@0 + 1 - 2 as x], file_type=parquet
+
+statement ok
+DROP TABLE expression_ordered_bounds;
+
+# The same range information is available to grouping-expression analysis.
+query TT
+EXPLAIN SELECT a + 1 AS x, count(*) FROM arithmetic_bounds GROUP BY x ORDER BY
x;
Review Comment:
Consider adding a result check for the new grouped aggregation case. Expect
`(2, 1)`, `(3, 1)`, `(4, 1)`, and `(NULL, 1)` to verify correctness with
`group_clustering_mode=Full`, beyond the existing plan assertion.
##########
datafusion/physical-expr/src/equivalence/properties/mod.rs:
##########
@@ -1335,11 +1379,43 @@ impl EquivalenceProperties {
constraints:
self.projected_constraints(mapping).unwrap_or_default(),
schema: output_schema,
eq_group,
+ column_ranges,
};
result.add_groupings(groupings);
result
}
+ fn projected_column_ranges(
+ &self,
+ mapping: &ProjectionMapping,
+ output_schema: &SchemaRef,
+ column_ranges: &ColumnRangeCache,
+ ) -> Option<Arc<ColumnRanges>> {
+ self.column_ranges.as_ref()?;
+ let mut ranges = ColumnRanges::default();
+ let dependencies = Dependencies::default();
+ for (source, targets) in mapping.iter() {
+ let Ok(properties) =
+ get_expr_properties(source, &dependencies, &self.schema,
column_ranges)
Review Comment:
When trusted bounds exist, `projected_column_ranges` analyzes every
projection source, even when no sort is removed. The cache reuses column
intervals but not expression-property results. Please benchmark using safe
arithmetic, overflow, and unchanged projections. Report planning and execution
times separately, with reproducible commands and settings. Include a
bounded-integer projection workload because the existing
`project/wide_struct_fields` benchmarks do not exercise this path. This
requests performance evidence, not a redesign.
--
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]
