You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by xu...@apache.org on 2025/02/24 11:23:53 UTC
(datafusion) branch main updated: fix(physical-expr): Remove empty constants check when ordering is satisfied (#14829)
This is an automated email from the ASF dual-hosted git repository.
xudong963 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion.git
The following commit(s) were added to refs/heads/main by this push:
new a5cc031349 fix(physical-expr): Remove empty constants check when ordering is satisfied (#14829)
a5cc031349 is described below
commit a5cc0313499d7c4b795af449f6a63e557d8c44ec
Author: Rohan Krishnaswamy <47...@users.noreply.github.com>
AuthorDate: Mon Feb 24 03:23:47 2025 -0800
fix(physical-expr): Remove empty constants check when ordering is satisfied (#14829)
* fix: Remove empty constants check when ordering is satisfied
* fix: Update failing UNION [ALL] BY NAME SLTs
---
.../physical-expr/src/equivalence/properties.rs | 2 +-
.../sqllogictest/test_files/union_by_name.slt | 43 +++++++++++-----------
2 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/datafusion/physical-expr/src/equivalence/properties.rs b/datafusion/physical-expr/src/equivalence/properties.rs
index 838cb26807..1ad4093b1f 100755
--- a/datafusion/physical-expr/src/equivalence/properties.rs
+++ b/datafusion/physical-expr/src/equivalence/properties.rs
@@ -2259,7 +2259,7 @@ impl UnionEquivalentOrderingBuilder {
) -> AddedOrdering {
if ordering.is_empty() {
AddedOrdering::Yes
- } else if constants.is_empty() && properties.ordering_satisfy(ordering.as_ref()) {
+ } else if properties.ordering_satisfy(ordering.as_ref()) {
// If the ordering satisfies the target properties, no need to
// augment it with constants.
self.orderings.push(ordering);
diff --git a/datafusion/sqllogictest/test_files/union_by_name.slt b/datafusion/sqllogictest/test_files/union_by_name.slt
index 0ba4c32ee5..63a43a36ff 100644
--- a/datafusion/sqllogictest/test_files/union_by_name.slt
+++ b/datafusion/sqllogictest/test_files/union_by_name.slt
@@ -92,15 +92,16 @@ NULL 1
NULL 3
5 NULL
-# TODO: This should pass, but the sanity checker isn't allowing it.
-# Commenting out the ordering check in the sanity checker produces the correct result.
-query error
+query II
(SELECT x FROM t1 UNION ALL SELECT x FROM t1) UNION ALL BY NAME SELECT 5 ORDER BY x;
----
-DataFusion error: SanityCheckPlan
-caused by
-Error during planning: Plan: ["SortPreservingMergeExec: [x@1 ASC NULLS LAST]", " UnionExec", " SortExec: expr=[x@1 ASC NULLS LAST], preserve_partitioning=[true]", " ProjectionExec: expr=[NULL as Int64(5), x@0 as x]", " UnionExec", " DataSourceExec: partitions=1, partition_sizes=[1]", " DataSourceExec: partitions=1, partition_sizes=[1]", " ProjectionExec: expr=[5 as Int64(5), NULL as x]", " PlaceholderRowExec"] does not satisfy order requirements: [...]
-
+NULL 1
+NULL 1
+NULL 3
+NULL 3
+NULL 3
+NULL 3
+5 NULL
query II
(SELECT x FROM t1 UNION ALL SELECT y FROM t1) UNION BY NAME SELECT 5 ORDER BY x;
@@ -109,15 +110,16 @@ NULL 1
NULL 3
5 NULL
-# TODO: This should pass, but the sanity checker isn't allowing it.
-# Commenting out the ordering check in the sanity checker produces the correct result.
-query error
+query II
(SELECT x FROM t1 UNION ALL SELECT y FROM t1) UNION ALL BY NAME SELECT 5 ORDER BY x;
----
-DataFusion error: SanityCheckPlan
-caused by
-Error during planning: Plan: ["SortPreservingMergeExec: [x@1 ASC NULLS LAST]", " UnionExec", " SortExec: expr=[x@1 ASC NULLS LAST], preserve_partitioning=[true]", " ProjectionExec: expr=[NULL as Int64(5), x@0 as x]", " UnionExec", " DataSourceExec: partitions=1, partition_sizes=[1]", " ProjectionExec: expr=[y@0 as x]", " DataSourceExec: partitions=1, partition_sizes=[1]", " ProjectionExec: expr=[5 as Int64(5), NULL as x]", " Placeholde [...]
-
+NULL 1
+NULL 1
+NULL 3
+NULL 3
+NULL 3
+NULL 3
+5 NULL
# Ambiguous name
@@ -158,15 +160,14 @@ NULL NULL 4
NULL NULL 5
NULL NULL 6
-# TODO: This should pass, but the sanity checker isn't allowing it.
-# Commenting out the ordering check in the sanity checker produces the correct result.
-query error
+query III
SELECT 1 UNION ALL BY NAME SELECT * FROM unnest(range(2, 100)) UNION ALL BY NAME SELECT 999 ORDER BY 3, 1 LIMIT 5;
----
-DataFusion error: SanityCheckPlan
-caused by
-Error during planning: Plan: ["SortPreservingMergeExec: [UNNEST(range(Int64(2),Int64(100)))@2 ASC NULLS LAST, Int64(1)@0 ASC NULLS LAST], fetch=5", " UnionExec", " SortExec: TopK(fetch=5), expr=[UNNEST(range(Int64(2),Int64(100)))@2 ASC NULLS LAST], preserve_partitioning=[true]", " ProjectionExec: expr=[Int64(1)@0 as Int64(1), NULL as Int64(999), UNNEST(range(Int64(2),Int64(100)))@1 as UNNEST(range(Int64(2),Int64(100)))]", " UnionExec", " ProjectionExec: expr=[1 a [...]
-
+NULL NULL 2
+NULL NULL 3
+NULL NULL 4
+NULL NULL 5
+NULL NULL 6
# Order by
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org