Skip to content

Commit ded3893

Browse files
authored
fix(query): fix order by derived column with limit return wrong values (#17457)
1 parent 7441d33 commit ded3893

File tree

4 files changed

+62
-9
lines changed

4 files changed

+62
-9
lines changed

โ€Žsrc/query/sql/src/executor/physical_plans/physical_table_scan.rsโ€Ž

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -497,12 +497,7 @@ impl PhysicalPlanBuilder {
497497
internal_column.column_name().to_owned(),
498498
internal_column.data_type(),
499499
),
500-
ColumnEntry::VirtualColumn(VirtualColumn {
501-
column_name,
502-
data_type,
503-
..
504-
}) => (column_name.clone(), DataType::from(data_type)),
505-
ColumnEntry::DerivedColumn(_) => {
500+
ColumnEntry::VirtualColumn(_) | ColumnEntry::DerivedColumn(_) => {
506501
return None;
507502
}
508503
};
@@ -520,6 +515,16 @@ impl PhysicalPlanBuilder {
520515
.collect::<Vec<_>>()
521516
});
522517

518+
let order_by = order_by.unwrap_or_default();
519+
let mut limit = scan.limit;
520+
if let Some(scan_order_by) = &scan.order_by {
521+
// If some order by columns can't be pushed down, then the limit can't be pushed down either,
522+
// as this may cause some blocks are pruned by the limit pruner.
523+
if scan_order_by.len() != order_by.len() {
524+
limit = None;
525+
}
526+
}
527+
523528
let virtual_column = self.build_virtual_column(&scan.columns);
524529

525530
Ok(PushDownInfo {
@@ -528,8 +533,8 @@ impl PhysicalPlanBuilder {
528533
filters: push_down_filter,
529534
is_deterministic,
530535
prewhere: prewhere_info,
531-
limit: scan.limit,
532-
order_by: order_by.unwrap_or_default(),
536+
limit,
537+
order_by,
533538
virtual_column,
534539
lazy_materialization: !metadata.lazy_columns().is_empty(),
535540
agg_index: None,

โ€Žsrc/query/sql/src/planner/binder/select.rsโ€Ž

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,18 @@ impl Binder {
520520
return Ok(());
521521
}
522522

523+
for order_by_item in order_by {
524+
let column = metadata.column(order_by_item.index);
525+
// If order by contains derived column or virtual column,
526+
// limit can't be pushed down, so there's no need row fetcher.
527+
if matches!(
528+
column,
529+
ColumnEntry::DerivedColumn(_) | ColumnEntry::VirtualColumn(_)
530+
) {
531+
return Ok(());
532+
}
533+
}
534+
523535
// As we don't if this is subquery, we need add required cols to metadata's non_lazy_columns,
524536
// so if the inner query not match the lazy materialized but outer query matched, we can prevent
525537
// the cols that inner query required not be pruned when analyze outer query.

โ€Žtests/sqllogictests/suites/base/03_common/03_0004_select_order_by.testโ€Ž

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,16 @@ select * from t1 order by id desc limit 3,3
5959
5
6060
4
6161

62+
statement ok
63+
insert into t1 select number as id from numbers(10)
64+
65+
query I
66+
select * from t1 order by id::string desc limit 3
67+
----
68+
9
69+
9
70+
8
71+
6272
statement ok
6373
drop table t1
6474

@@ -188,4 +198,4 @@ SELECT * from t1 order by '1', 1;
188198
3 2
189199

190200
statement ok
191-
DROP TABLE if EXISTS t1
201+
DROP TABLE if EXISTS t1

โ€Žtests/sqllogictests/suites/mode/standalone/explain/sort.testโ€Ž

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,32 @@ Sort
6464
โ”œโ”€โ”€ push downs: [filters: [is_true(t1.a (#0) > 1)], limit: NONE]
6565
โ””โ”€โ”€ estimated rows: 0.00
6666

67+
query T
68+
explain select a from t1 order by a::string limit 1;
69+
----
70+
Limit
71+
โ”œโ”€โ”€ output columns: [t1.a (#0), a::STRING (#2)]
72+
โ”œโ”€โ”€ limit: 1
73+
โ”œโ”€โ”€ offset: 0
74+
โ”œโ”€โ”€ estimated rows: 0.00
75+
โ””โ”€โ”€ Sort
76+
โ”œโ”€โ”€ output columns: [t1.a (#0), a::STRING (#2)]
77+
โ”œโ”€โ”€ sort keys: [a::STRING ASC NULLS LAST]
78+
โ”œโ”€โ”€ estimated rows: 0.00
79+
โ””โ”€โ”€ EvalScalar
80+
โ”œโ”€โ”€ output columns: [t1.a (#0), a::STRING (#2)]
81+
โ”œโ”€โ”€ expressions: [CAST(t1.a (#0) AS String NULL)]
82+
โ”œโ”€โ”€ estimated rows: 0.00
83+
โ””โ”€โ”€ TableScan
84+
โ”œโ”€โ”€ table: default.default.t1
85+
โ”œโ”€โ”€ output columns: [a (#0)]
86+
โ”œโ”€โ”€ read rows: 0
87+
โ”œโ”€โ”€ read size: 0
88+
โ”œโ”€โ”€ partitions total: 0
89+
โ”œโ”€โ”€ partitions scanned: 0
90+
โ”œโ”€โ”€ push downs: [filters: [], limit: NONE]
91+
โ””โ”€โ”€ estimated rows: 0.00
92+
6793
statement ok
6894
set max_threads = 4;
6995

0 commit comments

Comments
ย (0)