Skip to content

Commit

Permalink
fix(binder): pick 15597 (#15607)
Browse files Browse the repository at this point in the history
  • Loading branch information
chenzl25 authored Mar 11, 2024
1 parent bfaf07f commit c154bab
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 3 deletions.
14 changes: 14 additions & 0 deletions src/frontend/planner_test/tests/testdata/input/insert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,17 @@
insert into t select * from t;
expected_outputs:
- batch_distributed_plan
- name: test binding for insert select issue 15594
sql: |
create table t (a int);
create table t2 (b int);
insert into t select 1 as a from t2 group by a;
expected_outputs:
- batch_plan
- name: test binding for insert select issue 15594
sql: |
create table t (a int);
create table t2 (b int);
insert into t select 1 as a from t2 group by a returning a;
expected_outputs:
- batch_plan
34 changes: 33 additions & 1 deletion src/frontend/planner_test/tests/testdata/output/insert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,11 @@
sql: |
create table t (a int, b int);
insert into t values (0,1), (1,2) returning sum(a);
binder_error: 'Bind error: should not have agg/window in the `RETURNING` list'
binder_error: |
Failed to bind expression: sum(a)
Caused by:
Invalid input syntax: aggregate functions are not allowed in INSERT
- name: insert and specify all columns with values
sql: |
create table t (a int, b int);
Expand Down Expand Up @@ -343,3 +347,31 @@
└─BatchInsert { table: t, mapping: [0:0, 1:1] }
└─BatchExchange { order: [], dist: HashShard(t.a, t.b) }
└─BatchScan { table: t, columns: [t.a, t.b], distribution: SomeShard }
- name: test binding for insert select issue 15594
sql: |
create table t (a int);
create table t2 (b int);
insert into t select 1 as a from t2 group by a;
batch_plan: |-
BatchExchange { order: [], dist: Single }
└─BatchInsert { table: t, mapping: [0:0] }
└─BatchExchange { order: [], dist: Single }
└─BatchProject { exprs: [1:Int32] }
└─BatchHashAgg { group_key: [1:Int32], aggs: [] }
└─BatchExchange { order: [], dist: HashShard(1:Int32) }
└─BatchProject { exprs: [1:Int32] }
└─BatchScan { table: t2, columns: [], distribution: SomeShard }
- name: test binding for insert select issue 15594
sql: |
create table t (a int);
create table t2 (b int);
insert into t select 1 as a from t2 group by a returning a;
batch_plan: |-
BatchExchange { order: [], dist: Single }
└─BatchInsert { table: t, returning: true, mapping: [0:0] }
└─BatchExchange { order: [], dist: Single }
└─BatchProject { exprs: [1:Int32] }
└─BatchHashAgg { group_key: [1:Int32], aggs: [] }
└─BatchExchange { order: [], dist: HashShard(1:Int32) }
└─BatchProject { exprs: [1:Int32] }
└─BatchScan { table: t2, columns: [], distribution: SomeShard }
1 change: 1 addition & 0 deletions src/frontend/src/binder/bind_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub enum Clause {
Filter,
From,
GeneratedColumn,
Insert,
}

/// A `BindContext` that is only visible if the `LATERAL` keyword
Expand Down
11 changes: 10 additions & 1 deletion src/frontend/src/binder/expr/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use risingwave_common::error::{ErrorCode, Result};
use risingwave_common::types::DataType;
use risingwave_sqlparser::ast::Ident;

use crate::binder::Binder;
use crate::binder::{Binder, Clause};
use crate::expr::{CorrelatedInputRef, ExprImpl, ExprType, FunctionCall, InputRef, Literal};

impl Binder {
Expand Down Expand Up @@ -103,6 +103,9 @@ impl Binder {
for (i, lateral_context) in self.lateral_contexts.iter().rev().enumerate() {
if lateral_context.is_visible {
let context = &lateral_context.context;
if matches!(context.clause, Some(Clause::Insert)) {
continue;
}
// input ref from lateral context `depth` starts from 1.
let depth = i + 1;
match context.get_column_binding_index(&table_name, &column_name) {
Expand All @@ -125,6 +128,9 @@ impl Binder {
for (i, (context, lateral_contexts)) in
self.upper_subquery_contexts.iter().rev().enumerate()
{
if matches!(context.clause, Some(Clause::Insert)) {
continue;
}
// `depth` starts from 1.
let depth = i + 1;
match context.get_column_binding_index(&table_name, &column_name) {
Expand All @@ -145,6 +151,9 @@ impl Binder {
for (i, lateral_context) in lateral_contexts.iter().rev().enumerate() {
if lateral_context.is_visible {
let context = &lateral_context.context;
if matches!(context.clause, Some(Clause::Insert)) {
continue;
}
// correlated input ref from lateral context `depth` starts from 1.
let depth = i + 1;
match context.get_column_binding_index(&table_name, &column_name) {
Expand Down
3 changes: 3 additions & 0 deletions src/frontend/src/binder/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,7 @@ impl Binder {
| Clause::Filter
| Clause::GeneratedColumn
| Clause::From
| Clause::Insert
| Clause::JoinOn => {
return Err(ErrorCode::InvalidInputSyntax(format!(
"window functions are not allowed in {}",
Expand Down Expand Up @@ -1455,6 +1456,7 @@ impl Binder {
| Clause::Values
| Clause::From
| Clause::GeneratedColumn
| Clause::Insert
| Clause::JoinOn => {
return Err(ErrorCode::InvalidInputSyntax(format!(
"aggregate functions are not allowed in {}",
Expand All @@ -1476,6 +1478,7 @@ impl Binder {
| Clause::Having
| Clause::Filter
| Clause::Values
| Clause::Insert
| Clause::GeneratedColumn => {
return Err(ErrorCode::InvalidInputSyntax(format!(
"table functions are not allowed in {}",
Expand Down
4 changes: 3 additions & 1 deletion src/frontend/src/binder/insert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use risingwave_sqlparser::ast::{Ident, ObjectName, Query, SelectItem};

use super::statement::RewriteExprsRecursive;
use super::BoundQuery;
use crate::binder::Binder;
use crate::binder::{Binder, Clause};
use crate::catalog::TableId;
use crate::expr::{ExprImpl, InputRef};
use crate::user::UserId;
Expand Down Expand Up @@ -102,6 +102,8 @@ impl Binder {
returning_items: Vec<SelectItem>,
) -> Result<BoundInsert> {
let (schema_name, table_name) = Self::resolve_schema_qualified_name(&self.db_name, name)?;
// bind insert table
self.context.clause = Some(Clause::Insert);
self.bind_table(schema_name.as_deref(), &table_name, None)?;

let table_catalog = self.resolve_dml_table(schema_name.as_deref(), &table_name, true)?;
Expand Down

0 comments on commit c154bab

Please sign in to comment.