Skip to content

Commit 4a8169f

Browse files
committed
fix(ci): fix native prewhere when there is subquery
1 parent a624863 commit 4a8169f

File tree

4 files changed

+40
-30
lines changed

4 files changed

+40
-30
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

scripts/ci/ci-run-sqllogic-tests-native.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,4 @@ fi
1919
echo "Run suites using argument: $RUN_DIR"
2020

2121
echo "Starting databend-sqllogic tests"
22-
target/${BUILD_PROFILE}/databend-sqllogictests --handlers ${TEST_HANDLERS} ${RUN_DIR} --skip_dir management,mode,explain --enable_sandbox --parallel 8 --debug
22+
target/${BUILD_PROFILE}/databend-sqllogictests --handlers ${TEST_HANDLERS} ${RUN_DIR} --skip_dir management,mode,explain,tpch --enable_sandbox --parallel 8 --debug

src/query/sql/src/planner/optimizer/rule/rewrite/rule_push_down_prewhere.rs

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -52,47 +52,51 @@ impl RulePushDownPrewhere {
5252
}
5353
}
5454

55-
fn collect_columns_impl(expr: &ScalarExpr, columns: &mut ColumnSet) {
55+
/// will throw error if the bound column ref is not in the table, such as subquery
56+
fn collect_columns_impl(expr: &ScalarExpr, columns: &mut ColumnSet) -> Option<()> {
5657
match expr {
5758
ScalarExpr::BoundColumnRef(column) => {
59+
if column.column.table_name.is_none() {
60+
return None;
61+
}
5862
columns.insert(column.column.index);
63+
Some(())
5964
}
6065
ScalarExpr::AndExpr(and) => {
61-
Self::collect_columns_impl(and.left.as_ref(), columns);
62-
Self::collect_columns_impl(and.right.as_ref(), columns);
66+
Self::collect_columns_impl(and.left.as_ref(), columns)?;
67+
Self::collect_columns_impl(and.right.as_ref(), columns)
6368
}
6469
ScalarExpr::OrExpr(or) => {
65-
Self::collect_columns_impl(or.left.as_ref(), columns);
66-
Self::collect_columns_impl(or.right.as_ref(), columns);
67-
}
68-
ScalarExpr::NotExpr(not) => {
69-
Self::collect_columns_impl(not.argument.as_ref(), columns);
70+
Self::collect_columns_impl(or.left.as_ref(), columns)?;
71+
Self::collect_columns_impl(or.right.as_ref(), columns)
7072
}
73+
ScalarExpr::NotExpr(not) => Self::collect_columns_impl(not.argument.as_ref(), columns),
7174
ScalarExpr::ComparisonExpr(cmp) => {
72-
Self::collect_columns_impl(cmp.left.as_ref(), columns);
73-
Self::collect_columns_impl(cmp.right.as_ref(), columns);
75+
Self::collect_columns_impl(cmp.left.as_ref(), columns)?;
76+
Self::collect_columns_impl(cmp.right.as_ref(), columns)
7477
}
7578
ScalarExpr::FunctionCall(func) => {
7679
for arg in func.arguments.iter() {
77-
Self::collect_columns_impl(arg, columns);
80+
Self::collect_columns_impl(arg, columns)?;
7881
}
82+
Some(())
7983
}
8084
ScalarExpr::CastExpr(cast) => {
81-
Self::collect_columns_impl(cast.argument.as_ref(), columns);
85+
Self::collect_columns_impl(cast.argument.as_ref(), columns)
8286
}
8387
// 1. ConstantExpr is not collected.
8488
// 2. SubqueryExpr and AggregateFunction will not appear in Filter-LogicalGet
85-
_ => {}
89+
_ => None,
8690
}
8791
}
8892

8993
// analyze if the expression can be moved to prewhere
90-
fn collect_columns(expr: &ScalarExpr) -> ColumnSet {
94+
fn collect_columns(expr: &ScalarExpr) -> Option<ColumnSet> {
9195
let mut columns = ColumnSet::new();
9296
// columns in subqueries are not considered
93-
Self::collect_columns_impl(expr, &mut columns);
97+
Self::collect_columns_impl(expr, &mut columns)?;
9498

95-
columns
99+
Some(columns)
96100
}
97101

98102
pub fn prewhere_optimize(&self, s_expr: &SExpr) -> Result<SExpr> {
@@ -111,21 +115,27 @@ impl RulePushDownPrewhere {
111115

112116
// filter.predicates are already splited by AND
113117
for pred in filter.predicates.iter() {
114-
let columns = Self::collect_columns(pred);
115-
prewhere_pred.push(pred.clone());
116-
prewhere_columns.extend(&columns);
118+
match Self::collect_columns(pred) {
119+
Some(columns) => {
120+
prewhere_pred.push(pred.clone());
121+
prewhere_columns.extend(&columns);
122+
}
123+
None => return Ok(s_expr.clone()),
124+
}
117125
}
118126

119-
get.prewhere = if prewhere_pred.is_empty() {
120-
None
121-
} else {
122-
Some(Prewhere {
127+
if !prewhere_pred.is_empty() {
128+
if let Some(prewhere) = get.prewhere.as_ref() {
129+
prewhere_pred.extend(prewhere.predicates.clone());
130+
prewhere_columns.extend(&prewhere.prewhere_columns);
131+
}
132+
133+
get.prewhere = Some(Prewhere {
123134
output_columns: get.columns.clone(),
124135
prewhere_columns,
125136
predicates: prewhere_pred,
126-
})
127-
};
128-
137+
});
138+
}
129139
Ok(SExpr::create_leaf(get.into()))
130140
}
131141
}

tests/sqllogictests/suites/base/01_system/01_0005_system_tables_data_size

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ statement ok
55
insert into temp values(1)
66

77
query II
8-
select data_size, data_compressed_size from system.tables where name = 'temp'
8+
select data_size, data_compressed_size > 0 from system.tables where name = 'temp'
99
----
10-
1 207
10+
1 1
1111

1212
statement ok
1313
drop table temp

0 commit comments

Comments
 (0)