Skip to content

Commit

Permalink
fix delete contain internal columns panic (databendlabs#12833)
Browse files Browse the repository at this point in the history
  • Loading branch information
zhyass authored Sep 13, 2023
1 parent 891ae00 commit 69be928
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 0 deletions.
14 changes: 14 additions & 0 deletions src/query/sql/src/planner/binder/bind_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ pub struct BindContext {

pub expr_context: ExprContext,

pub allow_internal_columns: bool,
/// If true, the query is planning for aggregate index.
/// It's used to avoid infinite loop.
pub planning_agg_index: bool,
Expand Down Expand Up @@ -171,6 +172,7 @@ impl BindContext {
lambda_info: LambdaInfo::default(),
cte_name: None,
cte_map_ref: Box::default(),
allow_internal_columns: true,
in_grouping: false,
view_info: None,
srfs: DashMap::new(),
Expand All @@ -190,6 +192,7 @@ impl BindContext {
lambda_info: LambdaInfo::default(),
cte_name: parent.cte_name,
cte_map_ref: parent.cte_map_ref.clone(),
allow_internal_columns: parent.allow_internal_columns,
in_grouping: false,
view_info: None,
srfs: DashMap::new(),
Expand Down Expand Up @@ -222,6 +225,10 @@ impl BindContext {
self.columns.push(column_binding);
}

pub fn allow_internal_columns(&mut self, allow: bool) {
self.allow_internal_columns = allow;
}

/// Apply table alias like `SELECT * FROM t AS t1(a, b, c)`.
/// This method will rename column bindings according to table alias.
pub fn apply_table_alias(
Expand Down Expand Up @@ -507,6 +514,13 @@ impl BindContext {
column_binding: &InternalColumnBinding,
metadata: MetadataRef,
) -> Result<ColumnBinding> {
if !self.allow_internal_columns {
return Err(ErrorCode::SemanticError(format!(
"Internal column `{}` is not allowed in current statement",
column_binding.internal_column.column_name()
)));
}

let column_id = column_binding.internal_column.column_id();
let (table_index, column_index, new) = match self.bound_internal_columns.entry(column_id) {
btree_map::Entry::Vacant(e) => {
Expand Down
1 change: 1 addition & 0 deletions src/query/sql/src/planner/binder/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ impl<'a> Binder {
.bind_table_reference(bind_context, table_reference)
.await?;

context.allow_internal_columns(false);
let mut scalar_binder = ScalarBinder::new(
&mut context,
self.ctx.clone(),
Expand Down
1 change: 1 addition & 0 deletions src/query/sql/src/planner/binder/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ impl Binder {
srfs: Default::default(),
expr_context: ExprContext::default(),
planning_agg_index: false,
allow_internal_columns: true,
window_definitions: DashMap::new(),
};

Expand Down
1 change: 1 addition & 0 deletions src/query/sql/src/planner/binder/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ impl Binder {
.get_table(&catalog_name, &database_name, &table_name)
.await?;

context.allow_internal_columns(false);
let mut scalar_binder = ScalarBinder::new(
&mut context,
self.ctx.clone(),
Expand Down
3 changes: 3 additions & 0 deletions tests/sqllogictests/suites/base/03_common/03_0025_delete_from
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,9 @@ insert into t select c + 1000000 from t_number;
statement ok
delete from t where c >= 0 and c < 1500000;

statement error 1065
delete from t where _row_id = 18446735277616529408;

query I
select count(*) from t;
----
Expand Down

0 comments on commit 69be928

Please sign in to comment.