Skip to content

Commit

Permalink
Merge pull request #6880 from leiysky/fix-order-by
Browse files Browse the repository at this point in the history
fix(planner): Fix order by column with invalid table name
  • Loading branch information
mergify[bot] authored Jul 29, 2022
2 parents 23fd8b1 + d2de751 commit ee7b06b
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 20 deletions.
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
31 changes: 26 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,28 @@ 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 {
let matched = match (
(&database_name, &table_name),
(&item.database_name, &item.table_name),
) {
(
(Some(ident_database), Some(ident_table)),
(Some(database), Some(table)),
) if &ident_database.name == database
&& &ident_table.name == table
&& ident.name == item.column_name =>
{
true
}
((None, Some(ident_table)), (_, Some(table)))
if &ident_table.name == table && ident.name == item.column_name =>
{
true
}
((None, None), (_, _)) if ident.name == item.column_name => true,
_ => false,
};
if matched {
order_items.push(OrderItem {
expr: order.clone(),
index: item.index,
Expand Down Expand Up @@ -229,8 +250,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 +287,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
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,9 @@ SELECT DISTINCT(id) FROM a.t ORDER BY a.t.id;
statement error 1065
SELECT DISTINCT(id) FROM a.t ORDER BY a.t.id2;

statement query I
statement error
SELECT SUM(id) as id2 FROM a.t ORDER BY a.t.id2;

----
3

statement query I
SELECT DISTINCT(id) as id2 FROM a.t ORDER BY a.t.id2;

Expand All @@ -67,7 +64,7 @@ SELECT * FROM a.t ORDER BY a.t.id ASC;
1 1
2 2

statement error 1003
statement error 1065
SELECT * FROM a.t ORDER BY B.T.id ASC;

statement query II
Expand All @@ -77,7 +74,7 @@ SELECT * FROM a.t ORDER BY a.t.id DESC;
2 2
1 1

statement error 1003
statement error 1065
SELECT * FROM a.t ORDER BY B.T.id DESC;

statement ok
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;

1 comment on commit ee7b06b

@vercel
Copy link

@vercel vercel bot commented on ee7b06b Jul 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

databend – ./

databend.vercel.app
databend-git-main-databend.vercel.app
databend.rs
databend-databend.vercel.app

Please sign in to comment.