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.

Sign up for GitHub

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: type assertion failed on subquery #9937

Merged
merged 4 commits into from
Feb 9, 2023
Merged

fix: type assertion failed on subquery #9937

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7a783ad
fix: type assertion failed on subquery
xudong963 Feb 9, 2023
c56ebbb
refactor code
xudong963 Feb 9, 2023
0b4366b
fix
xudong963 Feb 9, 2023
3dcdf3f
Merge branch 'main' into fix_subquery
mergify[bot] Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/query/sql/src/planner/binder/project.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ impl Binder {
child_expr,
compare_op,
data_type,
allow_multi_rows,
outer_columns,
output_column,
..
Expand All @@ -78,7 +77,6 @@ impl Binder {
output_column,
projection_index: Some(column_binding.index),
data_type,
allow_multi_rows,
outer_columns,
})
} else {
Expand Down
27 changes: 15 additions & 12 deletions src/query/sql/src/planner/optimizer/heuristic/decorrelate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,18 +309,21 @@ impl SubqueryRewriter {
&mut left_conditions,
&mut right_conditions,
)?;
let index = subquery.output_column;
let column_name = format!("subquery_{}", index);
let right_condition = ScalarExpr::BoundColumnRef(BoundColumnRef {
column: ColumnBinding {
database_name: None,
table_name: None,
column_name,
index,
data_type: subquery.data_type.clone(),
visibility: Visibility::Visible,
},
});
let output_column = subquery.output_column.clone();
let column_name = format!("subquery_{}", output_column.index);
let right_condition = wrap_cast(
&ScalarExpr::BoundColumnRef(BoundColumnRef {
column: ColumnBinding {
database_name: None,
table_name: None,
column_name,
index: output_column.index,
data_type: output_column.data_type,
visibility: Visibility::Visible,
},
}),
&subquery.data_type,
);
let child_expr = *subquery.child_expr.as_ref().unwrap().clone();
let op = subquery.compare_op.as_ref().unwrap().clone();
// Make <child_expr op right_condition> as non_equi_conditions even if op is equal operator.
Expand Down
39 changes: 23 additions & 16 deletions src/query/sql/src/planner/optimizer/heuristic/subquery_rewriter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use common_expression::types::NumberDataType;
use common_expression::Literal;
use common_functions::aggregates::AggregateCountFunction;

use crate::binder::wrap_cast;
use crate::binder::ColumnBinding;
use crate::binder::Visibility;
use crate::optimizer::RelExpr;
Expand Down Expand Up @@ -281,12 +282,15 @@ impl SubqueryRewriter {
(marker_index, marker_index.to_string())
} else if let UnnestResult::SingleJoin = result {
let mut output_column = subquery.output_column;
if let Some(index) = self.derived_columns.get(&output_column) {
output_column = *index;
if let Some(index) = self.derived_columns.get(&output_column.index) {
output_column.index = *index;
}
(output_column, format!("scalar_subquery_{output_column}"))
(
output_column.index,
format!("scalar_subquery_{:?}", output_column.index),
)
} else {
let index = subquery.output_column;
let index = subquery.output_column.index;
(index, format!("subquery_{}", index))
};

Expand Down Expand Up @@ -463,18 +467,21 @@ impl SubqueryRewriter {
))
}
SubqueryType::Any => {
let index = subquery.output_column;
let column_name = format!("subquery_{}", index);
let left_condition = ScalarExpr::BoundColumnRef(BoundColumnRef {
column: ColumnBinding {
database_name: None,
table_name: None,
column_name,
index,
data_type: subquery.data_type.clone(),
visibility: Visibility::Visible,
},
});
let output_column = subquery.output_column.clone();
let column_name = format!("subquery_{}", output_column.index);
let left_condition = wrap_cast(
&ScalarExpr::BoundColumnRef(BoundColumnRef {
column: ColumnBinding {
database_name: None,
table_name: None,
column_name,
index: output_column.index,
data_type: output_column.data_type,
visibility: Visibility::Visible,
},
}),
&subquery.data_type,
);
let child_expr = *subquery.child_expr.as_ref().unwrap().clone();
let op = subquery.compare_op.as_ref().unwrap().clone();
let (right_condition, is_non_equi_condition) =
Expand Down
3 changes: 1 addition & 2 deletions src/query/sql/src/planner/plans/scalar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,9 @@ pub struct SubqueryExpr {
// Comparison operator for Any/All, such as t1.a = Any (...), `compare_op` is `=`.
pub compare_op: Option<ComparisonOp>,
// Output column of Any/All and scalar subqueries.
pub output_column: IndexType,
pub output_column: ColumnBinding,
pub projection_index: Option<IndexType>,
pub(crate) data_type: Box<DataType>,
pub allow_multi_rows: bool,
pub outer_columns: ColumnSet,
}

Expand Down
9 changes: 2 additions & 7 deletions src/query/sql/src/planner/semantic/type_check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,6 @@ impl<'a> TypeChecker<'a> {
self.resolve_subquery(
SubqueryType::Any,
subquery,
true,
Some(*left.clone()),
Some(comparison_op),
None,
Expand Down Expand Up @@ -770,7 +769,6 @@ impl<'a> TypeChecker<'a> {
SubqueryType::NotExists
},
subquery,
true,
None,
None,
None,
Expand All @@ -779,7 +777,7 @@ impl<'a> TypeChecker<'a> {
}

Expr::Subquery { subquery, .. } => {
self.resolve_subquery(SubqueryType::Scalar, subquery, false, None, None, None)
self.resolve_subquery(SubqueryType::Scalar, subquery, None, None, None)
.await?
}

Expand Down Expand Up @@ -809,7 +807,6 @@ impl<'a> TypeChecker<'a> {
self.resolve_subquery(
SubqueryType::Any,
subquery,
true,
Some(*expr.clone()),
Some(ComparisonOp::Equal),
None,
Expand Down Expand Up @@ -1350,7 +1347,6 @@ impl<'a> TypeChecker<'a> {
&mut self,
typ: SubqueryType,
subquery: &Query,
allow_multi_rows: bool,
child_expr: Option<Expr>,
compare_op: Option<ComparisonOp>,
_required_type: Option<DataType>,
Expand Down Expand Up @@ -1402,10 +1398,9 @@ impl<'a> TypeChecker<'a> {
subquery: Box::new(s_expr),
child_expr: child_scalar,
compare_op,
output_column: output_context.columns[0].index,
output_column: output_context.columns[0].clone(),
projection_index: None,
data_type: data_type.clone(),
allow_multi_rows,
typ,
outer_columns: rel_prop.outer_columns,
};
Expand Down
38 changes: 38 additions & 0 deletions tests/sqllogictests/suites/query/subquery.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,3 +610,41 @@ query I
select (select count() from numbers(10)) + (select count() from numbers(10))
----
20

query T
select 1 < ANY(SELECT NULL)
----
NULL

query I
select 1 < ANY(SELECT 1.0)
----
0

statement ok
drop table if exists t1

statement ok
create table t1 (a int, b int);

statement ok
insert into t1 values(1, 2);

statement ok
drop table if exists t2

statement ok
create table t2 (a int, b int);

statement ok
insert into t2 values(1, 1);

query T
select * from t2 where t2.b < ANY(select NULL from t1 where t1.a = t1.a)
----

statement ok
drop table t1

statement ok
drop table t2