Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(planner): Fix order by column with invalid table name #6880

Merged
merged 15 commits into from
Jul 29, 2022
Merged
4 changes: 2 additions & 2 deletions query/src/sql/executor/physical_plan_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,8 @@ impl PhysicalPlanBuilder {
.items
.iter()
.map(|v| SortDesc {
asc: v.asc.unwrap_or(true),
nulls_first: v.nulls_first.unwrap_or_default(),
asc: v.asc,
nulls_first: v.nulls_first,
order_by: v.index.to_string(),
})
.collect(),
Expand Down
13 changes: 8 additions & 5 deletions query/src/sql/planner/binder/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ impl<'a> Binder {
// We first search the identifier in select list
let mut found = false;
for item in projections.iter() {
if item.column_name == ident.name {
if item.column_name == ident.name
&& table_name.is_none()
&& database_name.is_none()
{
order_items.push(OrderItem {
expr: order.clone(),
index: item.index,
Expand Down Expand Up @@ -229,8 +232,8 @@ impl<'a> Binder {
}
let order_by_item = SortItem {
index: order.index,
asc: order.expr.asc,
nulls_first: order.expr.nulls_first,
asc: order.expr.asc.unwrap_or(true),
nulls_first: order.expr.nulls_first.unwrap_or(false),
};
order_by_items.push(order_by_item);
}
Expand Down Expand Up @@ -266,8 +269,8 @@ impl<'a> Binder {
Scalar::BoundColumnRef(BoundColumnRef { column }) => {
let order_by_item = SortItem {
index: column.index,
asc: order.asc,
nulls_first: order.nulls_first,
asc: order.asc.unwrap_or(true),
nulls_first: order.nulls_first.unwrap_or(false),
};
order_by_items.push(order_by_item);
}
Expand Down
6 changes: 1 addition & 5 deletions query/src/sql/planner/format/display_rel_operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,11 +330,7 @@ pub fn format_sort(
"{} (#{}) {}",
name,
item.index,
if item.asc.unwrap_or(false) {
"ASC"
} else {
"DESC"
}
if item.asc { "ASC" } else { "DESC" }
)
})
.collect::<Vec<String>>()
Expand Down
4 changes: 2 additions & 2 deletions query/src/sql/planner/plans/sort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ pub struct Sort {
#[derive(Clone, Debug)]
pub struct SortItem {
pub index: IndexType,
pub asc: Option<bool>,
pub nulls_first: Option<bool>,
pub asc: bool,
pub nulls_first: bool,
}

impl Operator for Sort {
Expand Down
41 changes: 41 additions & 0 deletions tests/logictest/suites/query/order.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
statement ok
set enable_planner_v2 = 1;

statement ok
create table order_test(a int null);

statement ok
insert into order_test values(1),(2),(null);

statement query I
select * from order_test order by a;

----
1
2
NULL

statement query I
select * from order_test order by a desc;

----
2
1
NULL

statement query I
select * from order_test order by a nulls first;

----
NULL
1
2

statement error
select number from numbers(10) as a order by b.number;

statement error
select number from (select * from numbers(10) as b) as a order by b.number;

statement ok
drop table order_test;