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 stack overflow when applying RuleFilterPushDownJoin #9645

Merged
merged 9 commits into from
Jan 17, 2023
3 changes: 2 additions & 1 deletion src/query/sql/src/planner/optimizer/heuristic/heuristic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ impl HeuristicOptimizer {
Ok(result)
}

// Return `None` if no rules matched
/// Try to apply the rules to the expression.
/// Return the final result that no rule can be applied.
fn apply_transform_rules(&self, s_expr: &SExpr, rule_list: &RuleList) -> Result<SExpr> {
let mut s_expr = s_expr.clone();
for rule in rule_list.iter() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use common_exception::Result;
use common_expression::type_check::common_super_type;
use itertools::Itertools;

use crate::binder::JoinPredicate;
use crate::optimizer::rule::Rule;
Expand Down Expand Up @@ -336,6 +337,9 @@ fn rewrite_predicates(s_expr: &SExpr) -> Result<Vec<Scalar>> {
}
}
origin_predicates.extend(new_predicates);
// Deduplicate predicates here to prevent handled by `EliminateFilter` rule later,
// which may cause infinite loop.
origin_predicates = origin_predicates.into_iter().unique().collect();
Ok(origin_predicates)
}

Expand Down
46 changes: 44 additions & 2 deletions src/query/sql/src/planner/plans/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::fmt::Debug;
use std::hash::Hash;

use common_ast::ast::BinaryOperator;
Expand Down Expand Up @@ -39,7 +40,7 @@ pub trait ScalarExpr {
// fn contains_subquery(&self) -> bool;
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Clone, PartialEq, Eq, Hash)]
pub enum Scalar {
BoundColumnRef(BoundColumnRef),
ConstantExpr(ConstantExpr),
Expand All @@ -55,6 +56,34 @@ pub enum Scalar {
SubqueryExpr(SubqueryExpr),
}

impl Debug for Scalar {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Scalar::BoundColumnRef(expr) => write!(f, "{}", expr.column.column_name),
Scalar::ConstantExpr(expr) => write!(f, "{}", expr.value),
Scalar::AndExpr(expr) => write!(f, "{:?} AND {:?}", expr.left, expr.right),
Scalar::OrExpr(expr) => write!(f, "{:?} OR {:?}", expr.left, expr.right),
Scalar::NotExpr(expr) => write!(f, "NOT {:?}", expr.argument),
Scalar::ComparisonExpr(expr) => write!(
f,
"{:?} {} {:?}",
expr.left,
expr.op.to_func_name(),
expr.right
),
Scalar::AggregateFunction(expr) => write!(f, "{}", expr.display_name),
Scalar::FunctionCall(expr) => write!(f, "{:?}", expr),
Scalar::CastExpr(expr) => write!(
f,
"CAST({:?} AS {})",
expr.argument,
expr.target_type.sql_name()
),
Scalar::SubqueryExpr(expr) => write!(f, "(subquery #{})", expr.output_column),
}
}
}

impl ScalarExpr for Scalar {
fn data_type(&self) -> DataType {
match self {
Expand Down Expand Up @@ -449,7 +478,7 @@ impl ScalarExpr for AggregateFunction {
}
}

#[derive(Clone, PartialEq, Eq, Hash, Debug)]
#[derive(Clone, PartialEq, Eq, Hash)]
pub struct FunctionCall {
pub params: Vec<usize>,
pub arguments: Vec<Scalar>,
Expand All @@ -458,6 +487,19 @@ pub struct FunctionCall {
pub return_type: Box<DataType>,
}

impl Debug for FunctionCall {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}(", self.func_name)?;
for (i, arg) in self.arguments.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}
write!(f, "{:?}", arg)?;
}
write!(f, ")")
}
}

impl ScalarExpr for FunctionCall {
fn data_type(&self) -> DataType {
*self.return_type.clone()
Expand Down
27 changes: 27 additions & 0 deletions tests/sqllogictests/suites/query/issues/issue_9236.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Github issue: https://github.com/datafuselabs/databend/issues/9236

statement ok
DROP DATABASE IF EXISTS databend0

statement ok
CREATE DATABASE databend0

statement ok
USE databend0

statement ok
CREATE TABLE t0(c0INT BIGINT NULL, c1FLOAT DOUBLE NULL DEFAULT(0.410796195268631))

statement ok
CREATE TABLE t1(c0INT INT8 NULL DEFAULT(-1949367821))

statement ok
CREATE TABLE t2(c0FLOAT DOUBLE NULL)

query TITI
SELECT t2.c0float, t0.c0int, t0.c1float, t1.c0int FROM t1, t2 LEFT JOIN t0 ON (((- ((-116704857)+(t0.c0int))))<((+ ((t0.c0int)*(t0.c0int))))) WHERE (((((((0.11264920979738235 NOT BETWEEN 0.9119921326637268 AND 0.8280299305915833))AND((true IN (true, ((false)AND(false)), (t0.c1float NOT BETWEEN t0.c1float AND 0.8534555435180664))))))and((((('')LIKE('hYAnCXx')))AND(((((((false)or(false)))or(false)))or(true)))))))OR((NOT ((NULL)=(NULL)))))
leiysky marked this conversation as resolved.
Show resolved Hide resolved
----


statement ok
DROP DATABASE databend0