Skip to content

Commit

Permalink
refactor(frontend): better cast error message for UPDATE setting to…
Browse files Browse the repository at this point in the history
… subquery (#19555)

Signed-off-by: Bugen Zhao <[email protected]>
  • Loading branch information
BugenZhao authored Nov 26, 2024
1 parent 6cae4d3 commit b34f4fd
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 25 deletions.
17 changes: 15 additions & 2 deletions e2e_test/batch/basic/dml_update.slt.part
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,21 @@ update t set (v1, v2) = (select '888.88', 999);
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: cannot cast type "record" to "record"
2: cannot cast type "character varying" to "integer" in Assign context
1: cannot cast type "record" to "struct<v1 integer, v2 integer>"
2: cannot cast to struct field "v1"
3: cannot cast type "character varying" to "integer" in Assign context


# Multiple assignments, to subquery (with column name) with cast failure.
statement error
update t set (v1, v2) = (select '888.88' s1, 999 s2);
----
db error: ERROR: Failed to run the query

Caused by these errors (recent errors listed first):
1: cannot cast type "struct<s1 character varying, s2 integer>" to "struct<v1 integer, v2 integer>"
2: cannot cast struct field "s1" to struct field "v1"
3: cannot cast type "character varying" to "integer" in Assign context


# Multiple assignments, to subquery with mismatched column count.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@
batch_plan: |-
BatchExchange { order: [], dist: Single }
└─BatchUpdate { table: t, exprs: [Field($4, 0:Int32), Field($4, 1:Int32), $2] }
└─BatchProject { exprs: [t.v1, t.v2, t._row_id, t._rw_timestamp, $expr10011::Struct(StructType { field_names: [], field_types: [Int32, Int32] }) as $expr1] }
└─BatchProject { exprs: [t.v1, t.v2, t._row_id, t._rw_timestamp, $expr10011::Struct(StructType { field_names: ["v1", "v2"], field_types: [Int32, Int32] }) as $expr1] }
└─BatchNestedLoopJoin { type: LeftOuter, predicate: true, output: all }
├─BatchExchange { order: [], dist: Single }
│ └─BatchScan { table: t, columns: [t.v1, t.v2, t._row_id, t._rw_timestamp], distribution: UpstreamHashShard(t._row_id) }
Expand Down
7 changes: 4 additions & 3 deletions src/frontend/src/binder/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,8 @@ impl Binder {

for Assignment { id, value } in assignments {
let ids: Vec<_> = id
.into_iter()
.map(|id| self.bind_expr(Expr::Identifier(id)))
.iter()
.map(|id| self.bind_expr(Expr::Identifier(id.clone())))
.try_collect()?;

match (ids.as_slice(), value) {
Expand Down Expand Up @@ -206,8 +206,9 @@ impl Binder {
bail_bind_error!("number of columns does not match number of values");
}

let target_type = DataType::new_unnamed_struct(
let target_type = DataType::new_struct(
ids.iter().map(|id| id.return_type()).collect(),
id.iter().map(|id| id.real_value()).collect(),
);
let expr = expr.cast_assign(target_type)?;

Expand Down
18 changes: 15 additions & 3 deletions src/frontend/src/expr/subquery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

use std::hash::Hash;

use risingwave_common::types::DataType;
use risingwave_common::types::{DataType, StructType};

use super::{Expr, ExprImpl, ExprType};
use crate::binder::BoundQuery;
use crate::binder::{BoundQuery, UNNAMED_COLUMN};
use crate::expr::{CorrelatedId, Depth};

#[derive(Clone, Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -91,7 +91,19 @@ impl Expr for Subquery {
assert_eq!(types.len(), 1, "Subquery with more than one column");
types[0].clone()
}
SubqueryKind::UpdateSet => DataType::new_unnamed_struct(self.query.data_types()),
SubqueryKind::UpdateSet => {
let schema = self.query.schema();
let struct_type = if schema.fields().iter().any(|f| f.name == UNNAMED_COLUMN) {
StructType::unnamed(self.query.data_types())
} else {
StructType::new(
(schema.fields().iter().cloned())
.map(|f| (f.name, f.data_type))
.collect(),
)
};
DataType::Struct(struct_type)
}
SubqueryKind::Array => {
let types = self.query.data_types();
assert_eq!(types.len(), 1, "Subquery with more than one column");
Expand Down
23 changes: 7 additions & 16 deletions src/frontend/src/expr/type_inference/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,22 +206,13 @@ fn cast_struct(source: &DataType, target: &DataType, allows: CastContext) -> Cas
Ok(())
} else {
cast(src_ty, dst_ty, allows).map_err(|inner| {
if src_name.is_empty() {
inner
} else if dst_name.is_empty() {
cast_error!(
source = inner,
"cannot cast struct field \"{}\"",
src_name
)
} else {
cast_error!(
source = inner,
"cannot cast struct field \"{}\" to struct field \"{}\"",
src_name,
dst_name
)
}
let cast_from = (!src_name.is_empty())
.then(|| format!(" struct field \"{}\"", src_name))
.unwrap_or_default();
let cast_to = (!dst_name.is_empty())
.then(|| format!(" to struct field \"{}\"", dst_name))
.unwrap_or_default();
cast_error!(source = inner, "cannot cast{}{}", cast_from, cast_to)
})
}
},
Expand Down

0 comments on commit b34f4fd

Please sign in to comment.