Skip to content

Commit

Permalink
fix: no-recursive cte reference itself (databendlabs#15941)
Browse files Browse the repository at this point in the history
* fix: no-recursive cte reference itself

* fix lateral join
  • Loading branch information
xudong963 authored Jul 2, 2024
1 parent f75fc61 commit c5e6eed
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 43 deletions.
7 changes: 7 additions & 0 deletions src/query/ast/src/ast/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,13 @@ impl TableReference {
_ => false,
}
}

pub fn is_lateral_subquery(&self) -> bool {
match self {
TableReference::Subquery { lateral, .. } => *lateral,
_ => false,
}
}
}

impl Display for TableReference {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@ impl Binder {
)?;
return Ok((result_expr, bind_context));
}

let (right_child, right_context) =
self.bind_table_reference(&mut left_context, &join.right)?;
let (right_child, right_context) = if join.right.is_lateral_subquery() {
self.bind_table_reference(&mut left_context, &join.right)?
} else {
self.bind_table_reference(bind_context, &join.right)?
};

let right_column_bindings = right_context.columns.clone();

Expand Down
54 changes: 22 additions & 32 deletions src/query/sql/src/planner/binder/bind_table_reference/bind_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,42 +49,32 @@ impl Binder {
let table_alias_name = alias
.as_ref()
.map(|table_alias| self.normalize_identifier(&table_alias.name).name);
let mut bind_cte = true;
if let Some(cte_name) = &bind_context.cte_name {
// If table name equals to cte name, then skip bind cte and find table from catalog
// Or will dead loop and stack overflow
if cte_name == &table_name {
bind_cte = false;
}
}
// Check and bind common table expression
let ctes_map = self.ctes_map.clone();
if let Some(cte_info) = ctes_map.get(&table_name) {
if bind_cte {
if self
.metadata
.read()
.get_table_index(Some(&database), &table_name)
.is_some()
{
return Err(ErrorCode::SyntaxException(format!(
"Table name `{}` is misleading, please distinguish it.",
table_name
))
.set_span(*span));
}
return if cte_info.materialized {
self.bind_m_cte(bind_context, cte_info, &table_name, alias, span)
} else if cte_info.recursive {
if self.bind_recursive_cte {
self.bind_r_cte_scan(bind_context, cte_info, &table_name, alias)
} else {
self.bind_r_cte(bind_context, cte_info, &table_name, alias)
}
} else {
self.bind_cte(*span, bind_context, &table_name, alias, cte_info)
};
if self
.metadata
.read()
.get_table_index(Some(&database), &table_name)
.is_some()
{
return Err(ErrorCode::SyntaxException(format!(
"Table name `{}` is misleading, please distinguish it.",
table_name
))
.set_span(*span));
}
return if cte_info.materialized {
self.bind_m_cte(bind_context, cte_info, &table_name, alias, span)
} else if cte_info.recursive {
if self.bind_recursive_cte {
self.bind_r_cte_scan(bind_context, cte_info, &table_name, alias)
} else {
self.bind_r_cte(bind_context, cte_info, &table_name, alias)
}
} else {
self.bind_cte(*span, bind_context, &table_name, alias, cte_info)
};
}

let tenant = self.ctx.get_tenant();
Expand Down
10 changes: 10 additions & 0 deletions src/query/sql/src/planner/binder/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,16 @@ impl Binder {
alias: &Option<TableAlias>,
cte_info: &CteInfo,
) -> Result<(SExpr, BindContext)> {
if let Some(cte_name) = &bind_context.cte_name {
// `cte_name` exists, which means the current cte is a nested cte
// If the `cte_name` is the same as the current cte's name, it means the cte is recursive
if cte_name == table_name {
return Err(ErrorCode::SemanticError(
"The cte is not recursive, but it references itself.".to_string(),
)
.set_span(span));
}
}
let mut new_bind_context = BindContext {
parent: Some(Box::new(bind_context.clone())),
bound_internal_columns: BTreeMap::new(),
Expand Down
11 changes: 3 additions & 8 deletions tests/sqllogictests/suites/query/cte/cte.test
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,12 @@ WITH test1 AS (SELECT * FROM numbers(5)) SELECT * FROM test1
3
4

query II
query error
WITH test1 AS (SELECT i + 1, j + 1 FROM test1) SELECT * FROM test1
----
2 3
4 5

query II
query error
WITH test1 AS (SELECT i + 1, j + 1 FROM test1) SELECT * FROM (SELECT * FROM test1)
----
2 3
4 5


query III
SELECT * FROM (WITH t1 AS (SELECT i FROM test1) SELECT * FROM t1) l INNER JOIN test1 r on l.i = r.i order by l.i, r.j
Expand Down

0 comments on commit c5e6eed

Please sign in to comment.