From ebaf213441007b8ca09df971d6eee6ac50ce88e8 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Tue, 19 Nov 2024 16:28:08 +0800 Subject: [PATCH 01/10] refactor(optimizer): record error contexts when casting structs Signed-off-by: Bugen Zhao --- src/frontend/src/expr/function_call.rs | 53 ++++++------- src/frontend/src/expr/mod.rs | 2 +- src/frontend/src/expr/type_inference/cast.rs | 79 +++++++++++++++----- src/frontend/src/expr/type_inference/mod.rs | 2 +- 4 files changed, 89 insertions(+), 47 deletions(-) diff --git a/src/frontend/src/expr/function_call.rs b/src/frontend/src/expr/function_call.rs index af1f84b321eb..101bdea17c00 100644 --- a/src/frontend/src/expr/function_call.rs +++ b/src/frontend/src/expr/function_call.rs @@ -14,12 +14,13 @@ use itertools::Itertools; use risingwave_common::catalog::Schema; +use risingwave_common::error::{bail, def_anyhow_newtype}; use risingwave_common::types::{DataType, ScalarImpl}; use risingwave_common::util::iter_util::ZipEqFast; -use thiserror::Error; use thiserror_ext::AsReport; -use super::{cast_ok, infer_some_all, infer_type, CastContext, Expr, ExprImpl, Literal}; +use super::type_inference::cast; +use super::{infer_some_all, infer_type, CastContext, Expr, ExprImpl, Literal}; use crate::error::{ErrorCode, Result as RwResult}; use crate::expr::{ExprDisplay, ExprType, ExprVisitor, ImpureAnalyzer}; @@ -144,22 +145,23 @@ impl FunctionCall { // else when eager parsing fails, just proceed as normal. // Some callers are not ready to handle `'a'::int` error here. } + let source = child.return_type(); if source == target { - Ok(()) - // Casting from unknown is allowed in all context. And PostgreSQL actually does the parsing - // in frontend. - } else if child.is_untyped() || cast_ok(&source, &target, allows) { - // Always Ok below. Safe to mutate `child`. - let owned = std::mem::replace(child, ExprImpl::literal_bool(false)); - *child = Self::new_unchecked(ExprType::Cast, vec![owned], target).into(); - Ok(()) + return Ok(()); + } + + if child.is_untyped() { + // Casting from unknown is allowed in all context. And PostgreSQL actually does the parsing + // in frontend. } else { - Err(CastError(format!( - "cannot cast type \"{}\" to \"{}\" in {:?} context", - source, target, allows - ))) + cast(&source, &target, allows)?; } + + // Always Ok below. Safe to mutate `child`. + let owned = std::mem::replace(child, ExprImpl::literal_bool(false)); + *child = Self::new_unchecked(ExprType::Cast, vec![owned], target).into(); + Ok(()) } /// Cast a `ROW` expression to the target type. We intentionally disallow casting arbitrary @@ -170,13 +172,13 @@ impl FunctionCall { target_type: DataType, allows: CastContext, ) -> Result<(), CastError> { + // Can only cast to a struct type. let DataType::Struct(t) = &target_type else { - return Err(CastError(format!( - "cannot cast type \"{}\" to \"{}\" in {:?} context", - func.return_type(), + bail!( + "cannot cast type \"{}\" to \"{}\"", + func.return_type(), // typically "record" target_type, - allows - ))); + ); }; match t.len().cmp(&func.inputs.len()) { std::cmp::Ordering::Equal => { @@ -189,10 +191,8 @@ impl FunctionCall { func.return_type = target_type; Ok(()) } - std::cmp::Ordering::Less => Err(CastError("Input has too few columns.".to_string())), - std::cmp::Ordering::Greater => { - Err(CastError("Input has too many columns.".to_string())) - } + std::cmp::Ordering::Less => bail!("input has too few columns"), + std::cmp::Ordering::Greater => bail!("input has too many columns"), } } @@ -423,9 +423,10 @@ pub fn is_row_function(expr: &ExprImpl) -> bool { false } -#[derive(Debug, Error)] -#[error("{0}")] -pub struct CastError(pub(super) String); +def_anyhow_newtype! { + pub CastError, +} +pub type CastResult = Result; impl From for ErrorCode { fn from(value: CastError) -> Self { diff --git a/src/frontend/src/expr/mod.rs b/src/frontend/src/expr/mod.rs index c7acdfa5c4a3..bc36679b3547 100644 --- a/src/frontend/src/expr/mod.rs +++ b/src/frontend/src/expr/mod.rs @@ -300,7 +300,7 @@ impl ExprImpl { ))), DataType::Int32 => Ok(self), dt if dt.is_int() => Ok(self.cast_explicit(DataType::Int32)?), - _ => Err(CastError("Unsupported input type".to_string())), + _ => bail!("unsupported input type"), } } diff --git a/src/frontend/src/expr/type_inference/cast.rs b/src/frontend/src/expr/type_inference/cast.rs index 51441c3f70c5..66ad6969ae31 100644 --- a/src/frontend/src/expr/type_inference/cast.rs +++ b/src/frontend/src/expr/type_inference/cast.rs @@ -15,12 +15,15 @@ use std::collections::BTreeMap; use std::sync::LazyLock; +use anyhow::Context; use itertools::Itertools as _; use parse_display::Display; +use risingwave_common::error::bail; use risingwave_common::types::{DataType, DataTypeName}; use risingwave_common::util::iter_util::ZipEqFast; use crate::error::ErrorCode; +use crate::expr::function_call::{CastError, CastResult}; use crate::expr::{Expr as _, ExprImpl, InputRef, Literal}; /// Find the least restrictive type. Used by `VALUES`, `CASE`, `UNION`, etc. @@ -114,12 +117,45 @@ pub fn align_array_and_element( Ok(array_type) } +fn canmeh(ok: bool) -> CastResult { + if ok { + Ok(()) + } else { + bail!("") + } +} +fn cannot() -> CastResult { + canmeh(false) +} + +pub fn cast(source: &DataType, target: &DataType, allows: CastContext) -> Result<(), CastError> { + macro_rules! any { + ($f:ident) => { + source.$f() || target.$f() + }; + } + + if any!(is_struct) { + cast_ok_struct(source, target, allows) + } else if any!(is_array) { + cast_ok_array(source, target, allows) + } else if any!(is_map) { + cast_ok_map(source, target, allows) + } else { + canmeh(cast_ok_base(source, target, allows)) + } + .with_context(|| { + format!( + "cannot cast type \"{}\" to \"{}\" in {:?} context", + source, target, allows + ) + }) + .map_err(Into::into) +} + /// Checks whether casting from `source` to `target` is ok in `allows` context. pub fn cast_ok(source: &DataType, target: &DataType, allows: CastContext) -> bool { - cast_ok_struct(source, target, allows) - || cast_ok_array(source, target, allows) - || cast_ok_map(source, target, allows) - || cast_ok_base(source, target, allows) + cast(source, target, allows).is_ok() } /// Checks whether casting from `source` to `target` is ok in `allows` context. @@ -128,52 +164,57 @@ pub fn cast_ok_base(source: &DataType, target: &DataType, allows: CastContext) - matches!(CAST_MAP.get(&(source.into(), target.into())), Some(context) if *context <= allows) } -fn cast_ok_struct(source: &DataType, target: &DataType, allows: CastContext) -> bool { +fn cast_ok_struct(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { match (source, target) { (DataType::Struct(lty), DataType::Struct(rty)) => { if lty.is_empty() || rty.is_empty() { unreachable!("record type should be already processed at this point"); } if lty.len() != rty.len() { - // only cast structs of the same length - return false; + bail!("cannot cast structs of different lengths"); } // ... and all fields are castable lty.types() .zip_eq_fast(rty.types()) - .all(|(src, dst)| src == dst || cast_ok(src, dst, allows)) + .try_for_each(|(src, dst)| { + if src == dst { + Ok(()) + } else { + cast(src, dst, allows) + } + }) } // The automatic casts to string types are treated as assignment casts, while the automatic // casts from string types are explicit-only. // https://www.postgresql.org/docs/14/sql-createcast.html#id-1.9.3.58.7.4 - (DataType::Varchar, DataType::Struct(_)) => CastContext::Explicit <= allows, - (DataType::Struct(_), DataType::Varchar) => CastContext::Assign <= allows, - _ => false, + (DataType::Varchar, DataType::Struct(_)) => canmeh(CastContext::Explicit <= allows), + (DataType::Struct(_), DataType::Varchar) => canmeh(CastContext::Assign <= allows), + _ => cannot(), } } -fn cast_ok_array(source: &DataType, target: &DataType, allows: CastContext) -> bool { +fn cast_ok_array(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { match (source, target) { (DataType::List(source_elem), DataType::List(target_elem)) => { - cast_ok(source_elem, target_elem, allows) + cast(source_elem, target_elem, allows) } // The automatic casts to string types are treated as assignment casts, while the automatic // casts from string types are explicit-only. // https://www.postgresql.org/docs/14/sql-createcast.html#id-1.9.3.58.7.4 - (DataType::Varchar, DataType::List(_)) => CastContext::Explicit <= allows, - (DataType::List(_), DataType::Varchar) => CastContext::Assign <= allows, - _ => false, + (DataType::Varchar, DataType::List(_)) => canmeh(CastContext::Explicit <= allows), + (DataType::List(_), DataType::Varchar) => canmeh(CastContext::Assign <= allows), + _ => cannot(), } } -fn cast_ok_map(source: &DataType, target: &DataType, allows: CastContext) -> bool { +fn cast_ok_map(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { match (source, target) { - (DataType::Map(source_elem), DataType::Map(target_elem)) => cast_ok( + (DataType::Map(source_elem), DataType::Map(target_elem)) => cast( &source_elem.clone().into_list(), &target_elem.clone().into_list(), allows, ), - _ => false, + _ => cannot(), } } diff --git a/src/frontend/src/expr/type_inference/mod.rs b/src/frontend/src/expr/type_inference/mod.rs index 2845f05ec0da..7f10d3aec3f6 100644 --- a/src/frontend/src/expr/type_inference/mod.rs +++ b/src/frontend/src/expr/type_inference/mod.rs @@ -18,6 +18,6 @@ mod cast; mod func; pub use cast::{ - align_types, cast_map_array, cast_ok, cast_ok_base, cast_sigs, CastContext, CastSig, + align_types, cast_map_array, cast_ok, cast, cast_ok_base, cast_sigs, CastContext, CastSig, }; pub use func::{infer_some_all, infer_type, infer_type_name, infer_type_with_sigmap, FuncSign}; From 23c21964f59600394b34918b4d06ae48b5c8b2ca Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Wed, 20 Nov 2024 17:34:09 +0800 Subject: [PATCH 02/10] switch to lightweight error Signed-off-by: Bugen Zhao --- Cargo.lock | 8 ++++---- Cargo.toml | 2 +- src/frontend/src/expr/function_call.rs | 19 ++++++++++++------- src/frontend/src/expr/mod.rs | 3 ++- src/frontend/src/expr/type_inference/cast.rs | 18 +++++++++--------- 5 files changed, 28 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c8be3e154ddd..c08e776947cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -14299,9 +14299,9 @@ dependencies = [ [[package]] name = "thiserror-ext" -version = "0.1.2" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a7c19760dc47062bca5c1b3699b032111c93802d51ac47660db11b08afc6bad2" +checksum = "ef4323942237f7cc071061f2c5f0db919e6053c2cdf58c6bc974883073429737" dependencies = [ "thiserror 1.0.63", "thiserror-ext-derive", @@ -14309,9 +14309,9 @@ dependencies = [ [[package]] name = "thiserror-ext-derive" -version = "0.1.2" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "667c8c48f68021098038115926c64d9950b0582062ae63f7d30638b1168daf03" +checksum = "96541747c50e6c73e094737938f4f5dfaf50c48a31adff4197a3e2a481371674" dependencies = [ "either", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index c260bf8c5293..6f9b85aa5c1a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -163,7 +163,7 @@ parquet = { version = "53.2", features = ["async"] } mysql_async = { version = "0.34", default-features = false, features = [ "default", ] } -thiserror-ext = "0.1.2" +thiserror-ext = { version = "0.2.1", features = ["backtrace"] } tikv-jemalloc-ctl = { git = "https://github.com/risingwavelabs/jemallocator.git", rev = "64a2d9" } tikv-jemallocator = { git = "https://github.com/risingwavelabs/jemallocator.git", features = [ "profiling", diff --git a/src/frontend/src/expr/function_call.rs b/src/frontend/src/expr/function_call.rs index 101bdea17c00..a8a5598b7349 100644 --- a/src/frontend/src/expr/function_call.rs +++ b/src/frontend/src/expr/function_call.rs @@ -14,10 +14,10 @@ use itertools::Itertools; use risingwave_common::catalog::Schema; -use risingwave_common::error::{bail, def_anyhow_newtype}; use risingwave_common::types::{DataType, ScalarImpl}; use risingwave_common::util::iter_util::ZipEqFast; -use thiserror_ext::AsReport; +use thiserror::Error; +use thiserror_ext::{AsReport, Box, Macro}; use super::type_inference::cast; use super::{infer_some_all, infer_type, CastContext, Expr, ExprImpl, Literal}; @@ -174,7 +174,7 @@ impl FunctionCall { ) -> Result<(), CastError> { // Can only cast to a struct type. let DataType::Struct(t) = &target_type else { - bail!( + bail_cast_error!( "cannot cast type \"{}\" to \"{}\"", func.return_type(), // typically "record" target_type, @@ -191,8 +191,8 @@ impl FunctionCall { func.return_type = target_type; Ok(()) } - std::cmp::Ordering::Less => bail!("input has too few columns"), - std::cmp::Ordering::Greater => bail!("input has too many columns"), + std::cmp::Ordering::Less => bail_cast_error!("input has too few columns"), + std::cmp::Ordering::Greater => bail_cast_error!("input has too many columns"), } } @@ -423,9 +423,14 @@ pub fn is_row_function(expr: &ExprImpl) -> bool { false } -def_anyhow_newtype! { - pub CastError, +#[derive(Error, Debug, Box, Macro)] +#[thiserror_ext(newtype(name = CastError), macro(path = "crate::expr::function_call"))] +#[error("{message}")] +pub struct CastErrorInner { + pub source: Option, + pub message: Box, } + pub type CastResult = Result; impl From for ErrorCode { diff --git a/src/frontend/src/expr/mod.rs b/src/frontend/src/expr/mod.rs index bc36679b3547..cf1d0cc21879 100644 --- a/src/frontend/src/expr/mod.rs +++ b/src/frontend/src/expr/mod.rs @@ -14,6 +14,7 @@ use enum_as_inner::EnumAsInner; use fixedbitset::FixedBitSet; +use function_call::bail_cast_error; use futures::FutureExt; use paste::paste; use risingwave_common::array::ListValue; @@ -300,7 +301,7 @@ impl ExprImpl { ))), DataType::Int32 => Ok(self), dt if dt.is_int() => Ok(self.cast_explicit(DataType::Int32)?), - _ => bail!("unsupported input type"), + _ => bail_cast_error!("unsupported input type"), } } diff --git a/src/frontend/src/expr/type_inference/cast.rs b/src/frontend/src/expr/type_inference/cast.rs index 66ad6969ae31..806d13f9fb30 100644 --- a/src/frontend/src/expr/type_inference/cast.rs +++ b/src/frontend/src/expr/type_inference/cast.rs @@ -15,15 +15,13 @@ use std::collections::BTreeMap; use std::sync::LazyLock; -use anyhow::Context; use itertools::Itertools as _; use parse_display::Display; -use risingwave_common::error::bail; use risingwave_common::types::{DataType, DataTypeName}; use risingwave_common::util::iter_util::ZipEqFast; use crate::error::ErrorCode; -use crate::expr::function_call::{CastError, CastResult}; +use crate::expr::function_call::{bail_cast_error, cast_error, CastError, CastResult}; use crate::expr::{Expr as _, ExprImpl, InputRef, Literal}; /// Find the least restrictive type. Used by `VALUES`, `CASE`, `UNION`, etc. @@ -121,7 +119,7 @@ fn canmeh(ok: bool) -> CastResult { if ok { Ok(()) } else { - bail!("") + bail_cast_error!() } } fn cannot() -> CastResult { @@ -144,13 +142,15 @@ pub fn cast(source: &DataType, target: &DataType, allows: CastContext) -> Result } else { canmeh(cast_ok_base(source, target, allows)) } - .with_context(|| { - format!( + .map_err(|inner| { + cast_error!( + source = inner, "cannot cast type \"{}\" to \"{}\" in {:?} context", - source, target, allows + source, + target, + allows ) }) - .map_err(Into::into) } /// Checks whether casting from `source` to `target` is ok in `allows` context. @@ -171,7 +171,7 @@ fn cast_ok_struct(source: &DataType, target: &DataType, allows: CastContext) -> unreachable!("record type should be already processed at this point"); } if lty.len() != rty.len() { - bail!("cannot cast structs of different lengths"); + bail_cast_error!("cannot cast structs of different lengths"); } // ... and all fields are castable lty.types() From 969a58c7f56a2512bbea165f75ce0c12be721aae Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 21 Nov 2024 15:23:49 +0800 Subject: [PATCH 03/10] rename and add docs Signed-off-by: Bugen Zhao --- .../system_catalog/pg_catalog/pg_cast.rs | 10 +++--- src/frontend/src/expr/function_call.rs | 2 ++ src/frontend/src/expr/mod.rs | 4 +-- src/frontend/src/expr/type_inference/cast.rs | 36 +++++++++---------- src/frontend/src/expr/type_inference/mod.rs | 2 +- 5 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/frontend/src/catalog/system_catalog/pg_catalog/pg_cast.rs b/src/frontend/src/catalog/system_catalog/pg_catalog/pg_cast.rs index d5b1332c25b3..291743ea4ba2 100644 --- a/src/frontend/src/catalog/system_catalog/pg_catalog/pg_cast.rs +++ b/src/frontend/src/catalog/system_catalog/pg_catalog/pg_cast.rs @@ -12,11 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. +use itertools::Itertools; use risingwave_common::types::{DataType, Fields}; use risingwave_frontend_macro::system_catalog; use crate::catalog::system_catalog::SysCatalogReaderImpl; -use crate::expr::cast_map_array; +use crate::expr::CAST_TABLE; /// The catalog `pg_cast` stores data type conversion paths. /// Ref: [`https://www.postgresql.org/docs/current/catalog-pg-cast.html`] @@ -31,12 +32,11 @@ struct PgCast { #[system_catalog(table, "pg_catalog.pg_cast")] fn read_pg_cast(_: &SysCatalogReaderImpl) -> Vec { - let mut cast_array = cast_map_array(); - cast_array.sort(); - cast_array + CAST_TABLE .iter() + .sorted() .enumerate() - .map(|(idx, (src, target, ctx))| PgCast { + .map(|(idx, ((src, target), ctx))| PgCast { oid: idx as i32, castsource: DataType::try_from(*src).unwrap().to_oid(), casttarget: DataType::try_from(*target).unwrap().to_oid(), diff --git a/src/frontend/src/expr/function_call.rs b/src/frontend/src/expr/function_call.rs index a8a5598b7349..e6da13ccbbce 100644 --- a/src/frontend/src/expr/function_call.rs +++ b/src/frontend/src/expr/function_call.rs @@ -423,6 +423,7 @@ pub fn is_row_function(expr: &ExprImpl) -> bool { false } +/// A stack of error messages for the cast operation. #[derive(Error, Debug, Box, Macro)] #[thiserror_ext(newtype(name = CastError), macro(path = "crate::expr::function_call"))] #[error("{message}")] @@ -433,6 +434,7 @@ pub struct CastErrorInner { pub type CastResult = Result; +// TODO(error-handling): do not use report string but directly make it a source of `ErrorCode`. impl From for ErrorCode { fn from(value: CastError) -> Self { ErrorCode::BindError(value.to_report_string()) diff --git a/src/frontend/src/expr/mod.rs b/src/frontend/src/expr/mod.rs index cf1d0cc21879..96a90efd9db7 100644 --- a/src/frontend/src/expr/mod.rs +++ b/src/frontend/src/expr/mod.rs @@ -68,8 +68,8 @@ pub use session_timezone::{SessionTimezone, TimestamptzExprFinder}; pub use subquery::{Subquery, SubqueryKind}; pub use table_function::{TableFunction, TableFunctionType}; pub use type_inference::{ - align_types, cast_map_array, cast_ok, cast_sigs, infer_some_all, infer_type, infer_type_name, - infer_type_with_sigmap, CastContext, CastSig, FuncSign, + align_types, cast_ok, cast_sigs, infer_some_all, infer_type, infer_type_name, + infer_type_with_sigmap, CastContext, CastSig, FuncSign, CAST_TABLE, }; pub use user_defined_function::UserDefinedFunction; pub use utils::*; diff --git a/src/frontend/src/expr/type_inference/cast.rs b/src/frontend/src/expr/type_inference/cast.rs index 806d13f9fb30..bbc36f218542 100644 --- a/src/frontend/src/expr/type_inference/cast.rs +++ b/src/frontend/src/expr/type_inference/cast.rs @@ -115,6 +115,8 @@ pub fn align_array_and_element( Ok(array_type) } +/// Returns `Ok` if `ok` is true, otherwise returns a placeholder [`CastError`] to be further +/// wrapped with a more informative context in [`cast`]. fn canmeh(ok: bool) -> CastResult { if ok { Ok(()) @@ -122,10 +124,13 @@ fn canmeh(ok: bool) -> CastResult { bail_cast_error!() } } +/// Equivalent to `canmeh(false)`. fn cannot() -> CastResult { canmeh(false) } +/// Checks whether casting from `source` to `target` is ok in `allows` context. +/// Returns an error if the cast is not possible. pub fn cast(source: &DataType, target: &DataType, allows: CastContext) -> Result<(), CastError> { macro_rules! any { ($f:ident) => { @@ -134,11 +139,11 @@ pub fn cast(source: &DataType, target: &DataType, allows: CastContext) -> Result } if any!(is_struct) { - cast_ok_struct(source, target, allows) + cast_struct(source, target, allows) } else if any!(is_array) { - cast_ok_array(source, target, allows) + cast_array(source, target, allows) } else if any!(is_map) { - cast_ok_map(source, target, allows) + cast_map(source, target, allows) } else { canmeh(cast_ok_base(source, target, allows)) } @@ -154,6 +159,8 @@ pub fn cast(source: &DataType, target: &DataType, allows: CastContext) -> Result } /// Checks whether casting from `source` to `target` is ok in `allows` context. +/// +/// Equivalent to `cast(..).is_ok()`, but [`cast`] may be preferred for its error messages. pub fn cast_ok(source: &DataType, target: &DataType, allows: CastContext) -> bool { cast(source, target, allows).is_ok() } @@ -161,10 +168,10 @@ pub fn cast_ok(source: &DataType, target: &DataType, allows: CastContext) -> boo /// Checks whether casting from `source` to `target` is ok in `allows` context. /// Both `source` and `target` must be base types, i.e. not struct or array. pub fn cast_ok_base(source: &DataType, target: &DataType, allows: CastContext) -> bool { - matches!(CAST_MAP.get(&(source.into(), target.into())), Some(context) if *context <= allows) + matches!(CAST_TABLE.get(&(source.into(), target.into())), Some(context) if *context <= allows) } -fn cast_ok_struct(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { +fn cast_struct(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { match (source, target) { (DataType::Struct(lty), DataType::Struct(rty)) => { if lty.is_empty() || rty.is_empty() { @@ -193,7 +200,7 @@ fn cast_ok_struct(source: &DataType, target: &DataType, allows: CastContext) -> } } -fn cast_ok_array(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { +fn cast_array(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { match (source, target) { (DataType::List(source_elem), DataType::List(target_elem)) => { cast(source_elem, target_elem, allows) @@ -207,7 +214,7 @@ fn cast_ok_array(source: &DataType, target: &DataType, allows: CastContext) -> C } } -fn cast_ok_map(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { +fn cast_map(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { match (source, target) { (DataType::Map(source_elem), DataType::Map(target_elem)) => cast( &source_elem.clone().into_list(), @@ -218,13 +225,6 @@ fn cast_ok_map(source: &DataType, target: &DataType, allows: CastContext) -> Cas } } -pub fn cast_map_array() -> Vec<(DataTypeName, DataTypeName, CastContext)> { - CAST_MAP - .iter() - .map(|((src, target), ctx)| (*src, *target, *ctx)) - .collect_vec() -} - #[derive(Clone, Debug)] pub struct CastSig { pub from_type: DataTypeName, @@ -245,10 +245,10 @@ pub enum CastContext { Explicit, } -pub type CastMap = BTreeMap<(DataTypeName, DataTypeName), CastContext>; +pub type CastTable = BTreeMap<(DataTypeName, DataTypeName), CastContext>; pub fn cast_sigs() -> impl Iterator { - CAST_MAP + CAST_TABLE .iter() .map(|((from_type, to_type), context)| CastSig { from_type: *from_type, @@ -257,7 +257,7 @@ pub fn cast_sigs() -> impl Iterator { }) } -pub static CAST_MAP: LazyLock = LazyLock::new(|| { +pub static CAST_TABLE: LazyLock = LazyLock::new(|| { // cast rules: // 1. implicit cast operations in PG are organized in 3 sequences, // with the reverse direction being assign cast operations. @@ -347,7 +347,7 @@ mod tests { fn test_cast_ok() { // With the help of a script we can obtain the 3 expected cast tables from PG. They are // slightly modified on same-type cast and from-string cast for reasons explained above in - // `build_cast_map`. + // `build_cast_table`. let actual = gen_cast_table(CastContext::Implicit); assert_eq!( diff --git a/src/frontend/src/expr/type_inference/mod.rs b/src/frontend/src/expr/type_inference/mod.rs index 7f10d3aec3f6..bc4e6477bf2d 100644 --- a/src/frontend/src/expr/type_inference/mod.rs +++ b/src/frontend/src/expr/type_inference/mod.rs @@ -18,6 +18,6 @@ mod cast; mod func; pub use cast::{ - align_types, cast_map_array, cast_ok, cast, cast_ok_base, cast_sigs, CastContext, CastSig, + align_types, cast, cast_ok, cast_ok_base, cast_sigs, CastContext, CastSig, CAST_TABLE, }; pub use func::{infer_some_all, infer_type, infer_type_name, infer_type_with_sigmap, FuncSign}; From 2473a083da8fdb00973b21bf09c16f3ae5bfd64f Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 21 Nov 2024 16:14:49 +0800 Subject: [PATCH 04/10] preserve source Signed-off-by: Bugen Zhao --- src/frontend/src/expr/function_call.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/frontend/src/expr/function_call.rs b/src/frontend/src/expr/function_call.rs index e6da13ccbbce..d3c9c0dba2fd 100644 --- a/src/frontend/src/expr/function_call.rs +++ b/src/frontend/src/expr/function_call.rs @@ -17,7 +17,7 @@ use risingwave_common::catalog::Schema; use risingwave_common::types::{DataType, ScalarImpl}; use risingwave_common::util::iter_util::ZipEqFast; use thiserror::Error; -use thiserror_ext::{AsReport, Box, Macro}; +use thiserror_ext::{Box, Macro}; use super::type_inference::cast; use super::{infer_some_all, infer_type, CastContext, Expr, ExprImpl, Literal}; @@ -434,9 +434,9 @@ pub struct CastErrorInner { pub type CastResult = Result; -// TODO(error-handling): do not use report string but directly make it a source of `ErrorCode`. +// TODO(error-handling): shall we make it a new variant? impl From for ErrorCode { fn from(value: CastError) -> Self { - ErrorCode::BindError(value.to_report_string()) + ErrorCode::Uncategorized(value.into()).into() } } From 3f12a205bae457ec4b7d27b7e9cc29b75f0fe1da Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 21 Nov 2024 16:15:01 +0800 Subject: [PATCH 05/10] add & update planner tests Signed-off-by: Bugen Zhao --- .../tests/testdata/input/cast.yaml | 17 ++++++++++ .../tests/testdata/output/array.yaml | 2 +- .../tests/testdata/output/cast.yaml | 32 +++++++++++++++++++ .../tests/testdata/output/expr.yaml | 2 +- .../tests/testdata/output/struct_query.yaml | 2 +- .../tests/testdata/output/update.yaml | 2 +- 6 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/frontend/planner_test/tests/testdata/input/cast.yaml b/src/frontend/planner_test/tests/testdata/input/cast.yaml index a21b9e3cb409..f2344b3dd00a 100644 --- a/src/frontend/planner_test/tests/testdata/input/cast.yaml +++ b/src/frontend/planner_test/tests/testdata/input/cast.yaml @@ -64,3 +64,20 @@ select count(*) FILTER(WHERE 'y') from t; expected_outputs: - batch_plan +- name: composite type cast error message (array) + sql: | + select '[]'::int[]::bytea[]; + expected_outputs: + - binder_error +- name: composite type cast error message (struct) + sql: | + create table t (v struct, c bool>); + select v::struct, f bool> from t; + expected_outputs: + - binder_error +- name: composite type cast error message (map) + sql: | + create table t (v map(int, int)); + select v::map(int, bytea) from t; + expected_outputs: + - binder_error diff --git a/src/frontend/planner_test/tests/testdata/output/array.yaml b/src/frontend/planner_test/tests/testdata/output/array.yaml index a2b9486fdb33..259a727d23df 100644 --- a/src/frontend/planner_test/tests/testdata/output/array.yaml +++ b/src/frontend/planner_test/tests/testdata/output/array.yaml @@ -228,7 +228,7 @@ sql: | create table t (v1 varchar[]); insert into t values ('{c,' || 'd}'); - binder_error: 'Bind error: cannot cast type "character varying" to "character varying[]" in Assign context' + binder_error: cannot cast type "character varying" to "character varying[]" in Assign context - name: string to varchar[] in explicit context sql: | select ('{c,' || 'd}')::varchar[]; diff --git a/src/frontend/planner_test/tests/testdata/output/cast.yaml b/src/frontend/planner_test/tests/testdata/output/cast.yaml index 636f25a9b07d..7a774e224f1f 100644 --- a/src/frontend/planner_test/tests/testdata/output/cast.yaml +++ b/src/frontend/planner_test/tests/testdata/output/cast.yaml @@ -80,3 +80,35 @@ └─BatchExchange { order: [], dist: Single } └─BatchSimpleAgg { aggs: [count] } └─BatchScan { table: t, columns: [], distribution: SomeShard } +- name: composite type cast error message (array) + sql: | + select '[]'::int[]::bytea[]; + binder_error: | + Failed to bind expression: CAST(CAST('[]' AS INT[]) AS BYTEA[]) + + Caused by these errors (recent errors listed first): + 1: cannot cast type "integer[]" to "bytea[]" in Explicit context + 2: cannot cast type "integer" to "bytea" in Explicit context +- name: composite type cast error message (struct) + sql: | + create table t (v struct, c bool>); + select v::struct, f bool> from t; + binder_error: | + Failed to bind expression: CAST(v AS STRUCT, f BOOLEAN>) + + Caused by these errors (recent errors listed first): + 1: cannot cast type "struct, c boolean>" to "struct, f boolean>" in Explicit context + 2: cannot cast type "struct" to "struct" in Explicit context + 3: cannot cast type "integer" to "bytea" in Explicit context +- name: composite type cast error message (map) + sql: | + create table t (v map(int, int)); + select v::map(int, bytea) from t; + binder_error: | + Failed to bind expression: CAST(v AS MAP(INT,BYTEA)) + + Caused by these errors (recent errors listed first): + 1: cannot cast type "map(integer,integer)" to "map(integer,bytea)" in Explicit context + 2: cannot cast type "struct[]" to "struct[]" in Explicit context + 3: cannot cast type "struct" to "struct" in Explicit context + 4: cannot cast type "integer" to "bytea" in Explicit context diff --git a/src/frontend/planner_test/tests/testdata/output/expr.yaml b/src/frontend/planner_test/tests/testdata/output/expr.yaml index eacab421069a..ce2f86ed2e68 100644 --- a/src/frontend/planner_test/tests/testdata/output/expr.yaml +++ b/src/frontend/planner_test/tests/testdata/output/expr.yaml @@ -304,7 +304,7 @@ Failed to bind expression: concat_ws(v1, 1.2) Caused by: - Bind error: cannot cast type "integer" to "character varying" in Implicit context + cannot cast type "integer" to "character varying" in Implicit context - sql: | create table t (v1 int); select concat_ws() from t; diff --git a/src/frontend/planner_test/tests/testdata/output/struct_query.yaml b/src/frontend/planner_test/tests/testdata/output/struct_query.yaml index 907aa209c6d2..3f6b4579e44c 100644 --- a/src/frontend/planner_test/tests/testdata/output/struct_query.yaml +++ b/src/frontend/planner_test/tests/testdata/output/struct_query.yaml @@ -421,7 +421,7 @@ - sql: | CREATE TABLE a (c STRUCT, j INTEGER>); INSERT INTO a VALUES (1); - binder_error: 'Bind error: cannot cast type "integer" to "struct, j integer>" in Assign context' + binder_error: cannot cast type "integer" to "struct, j integer>" in Assign context - name: test struct type alignment in CASE expression sql: | select CASE WHEN false THEN ROW(0, INTERVAL '1') WHEN true THEN ROW(1.1, INTERVAL '1') ELSE ROW(1, INTERVAL '1') END; diff --git a/src/frontend/planner_test/tests/testdata/output/update.yaml b/src/frontend/planner_test/tests/testdata/output/update.yaml index 4a12b492660a..26c6d52dc5e0 100644 --- a/src/frontend/planner_test/tests/testdata/output/update.yaml +++ b/src/frontend/planner_test/tests/testdata/output/update.yaml @@ -11,7 +11,7 @@ - sql: | create table t (v1 int, v2 int); update t set v1 = true; - binder_error: 'Bind error: cannot cast type "boolean" to "integer" in Assign context' + binder_error: cannot cast type "boolean" to "integer" in Assign context - sql: | create table t (v1 int, v2 int); update t set v1 = v2 + 1; From 3a818ae480e9414de86c860c57ec2dc780060be0 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 21 Nov 2024 16:23:15 +0800 Subject: [PATCH 06/10] update `UPDATE` test Signed-off-by: Bugen Zhao --- e2e_test/batch/basic/dml_update.slt.part | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/e2e_test/batch/basic/dml_update.slt.part b/e2e_test/batch/basic/dml_update.slt.part index fcc3bbdfce9a..69eb15b800fe 100644 --- a/e2e_test/batch/basic/dml_update.slt.part +++ b/e2e_test/batch/basic/dml_update.slt.part @@ -93,10 +93,15 @@ select * from t; 889 999 # Multiple assignments, to subquery with cast failure. -# TODO: this currently shows `cannot cast type "record" to "record"` because we wrap the subquery result -# into a struct, which is not quite clear. -statement error cannot cast type +statement error 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" in Assign context + 2: cannot cast type "character varying" to "integer" in Assign context + # Multiple assignments, to subquery with mismatched column count. statement error number of columns does not match number of values From fc4223348295f029a4d88d0d9f64b4de8a760ac8 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 21 Nov 2024 16:54:18 +0800 Subject: [PATCH 07/10] refine message: - only show "context" in the bottom level - show struct field name - do not expose internal impl of map type, instead, show "key" or "value" Signed-off-by: Bugen Zhao --- e2e_test/batch/basic/dml_update.slt.part | 2 +- .../tests/testdata/output/cast.yaml | 17 +++--- src/frontend/src/expr/type_inference/cast.rs | 58 ++++++++++++++----- 3 files changed, 54 insertions(+), 23 deletions(-) diff --git a/e2e_test/batch/basic/dml_update.slt.part b/e2e_test/batch/basic/dml_update.slt.part index 69eb15b800fe..fc2647cea147 100644 --- a/e2e_test/batch/basic/dml_update.slt.part +++ b/e2e_test/batch/basic/dml_update.slt.part @@ -99,7 +99,7 @@ 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" in Assign context + 1: cannot cast type "record" to "record" 2: cannot cast type "character varying" to "integer" in Assign context diff --git a/src/frontend/planner_test/tests/testdata/output/cast.yaml b/src/frontend/planner_test/tests/testdata/output/cast.yaml index 7a774e224f1f..28cc002eae56 100644 --- a/src/frontend/planner_test/tests/testdata/output/cast.yaml +++ b/src/frontend/planner_test/tests/testdata/output/cast.yaml @@ -87,7 +87,7 @@ Failed to bind expression: CAST(CAST('[]' AS INT[]) AS BYTEA[]) Caused by these errors (recent errors listed first): - 1: cannot cast type "integer[]" to "bytea[]" in Explicit context + 1: cannot cast type "integer[]" to "bytea[]" 2: cannot cast type "integer" to "bytea" in Explicit context - name: composite type cast error message (struct) sql: | @@ -97,9 +97,11 @@ Failed to bind expression: CAST(v AS STRUCT, f BOOLEAN>) Caused by these errors (recent errors listed first): - 1: cannot cast type "struct, c boolean>" to "struct, f boolean>" in Explicit context - 2: cannot cast type "struct" to "struct" in Explicit context - 3: cannot cast type "integer" to "bytea" in Explicit context + 1: cannot cast type "struct, c boolean>" to "struct, f boolean>" + 2: cannot cast struct field "a" to struct field "d" + 3: cannot cast type "struct" to "struct" + 4: cannot cast struct field "b" to struct field "e" + 5: cannot cast type "integer" to "bytea" in Explicit context - name: composite type cast error message (map) sql: | create table t (v map(int, int)); @@ -108,7 +110,6 @@ Failed to bind expression: CAST(v AS MAP(INT,BYTEA)) Caused by these errors (recent errors listed first): - 1: cannot cast type "map(integer,integer)" to "map(integer,bytea)" in Explicit context - 2: cannot cast type "struct[]" to "struct[]" in Explicit context - 3: cannot cast type "struct" to "struct" in Explicit context - 4: cannot cast type "integer" to "bytea" in Explicit context + 1: cannot cast type "map(integer,integer)" to "map(integer,bytea)" + 2: cannot cast map value + 3: cannot cast type "integer" to "bytea" in Explicit context diff --git a/src/frontend/src/expr/type_inference/cast.rs b/src/frontend/src/expr/type_inference/cast.rs index bbc36f218542..8e4408e6dadd 100644 --- a/src/frontend/src/expr/type_inference/cast.rs +++ b/src/frontend/src/expr/type_inference/cast.rs @@ -13,12 +13,13 @@ // limitations under the License. use std::collections::BTreeMap; +use std::error::Error; use std::sync::LazyLock; use itertools::Itertools as _; use parse_display::Display; use risingwave_common::types::{DataType, DataTypeName}; -use risingwave_common::util::iter_util::ZipEqFast; +use risingwave_common::util::iter_util::{ZipEqDebug, ZipEqFast}; use crate::error::ErrorCode; use crate::expr::function_call::{bail_cast_error, cast_error, CastError, CastResult}; @@ -148,12 +149,18 @@ pub fn cast(source: &DataType, target: &DataType, allows: CastContext) -> Result canmeh(cast_ok_base(source, target, allows)) } .map_err(|inner| { + // Only show "in .. context" once in the error source chain. + let in_context = if inner.source().is_none() { + &format!(" in {:?} context", allows) + } else { + "" + }; cast_error!( source = inner, - "cannot cast type \"{}\" to \"{}\" in {:?} context", + "cannot cast type \"{}\" to \"{}\"{}", source, target, - allows + in_context, ) }) } @@ -181,15 +188,32 @@ fn cast_struct(source: &DataType, target: &DataType, allows: CastContext) -> Cas bail_cast_error!("cannot cast structs of different lengths"); } // ... and all fields are castable - lty.types() - .zip_eq_fast(rty.types()) - .try_for_each(|(src, dst)| { - if src == dst { + lty.iter().zip_eq_debug(rty.iter()).try_for_each( + |((src_name, src_ty), (dst_name, dst_ty))| { + if src_ty == dst_ty { Ok(()) } else { - cast(src, dst, allows) + 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 + ) + } + }) } - }) + }, + ) } // The automatic casts to string types are treated as assignment casts, while the automatic // casts from string types are explicit-only. @@ -216,11 +240,17 @@ fn cast_array(source: &DataType, target: &DataType, allows: CastContext) -> Cast fn cast_map(source: &DataType, target: &DataType, allows: CastContext) -> CastResult { match (source, target) { - (DataType::Map(source_elem), DataType::Map(target_elem)) => cast( - &source_elem.clone().into_list(), - &target_elem.clone().into_list(), - allows, - ), + (DataType::Map(source_elem), DataType::Map(target_elem)) => { + if source_elem.key() != target_elem.key() { + cast(source_elem.key(), target_elem.key(), allows) + .map_err(|inner| cast_error!(source = inner, "cannot cast map key"))?; + } + if source_elem.value() != target_elem.value() { + cast(source_elem.value(), target_elem.value(), allows) + .map_err(|inner| cast_error!(source = inner, "cannot cast map value"))?; + } + Ok(()) + } _ => cannot(), } } From d0ea0ef0e0a8c62cf4cf148828df9c014d0ad32b Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Thu, 21 Nov 2024 17:19:21 +0800 Subject: [PATCH 08/10] fix check Signed-off-by: Bugen Zhao --- src/frontend/src/expr/function_call.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/frontend/src/expr/function_call.rs b/src/frontend/src/expr/function_call.rs index d3c9c0dba2fd..0f4c36ce3a00 100644 --- a/src/frontend/src/expr/function_call.rs +++ b/src/frontend/src/expr/function_call.rs @@ -437,6 +437,6 @@ pub type CastResult = Result; // TODO(error-handling): shall we make it a new variant? impl From for ErrorCode { fn from(value: CastError) -> Self { - ErrorCode::Uncategorized(value.into()).into() + ErrorCode::Uncategorized(value.into()) } } From 9a0246c907eb652ff45a9308f7082ad998cad255 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Fri, 22 Nov 2024 13:48:39 +0800 Subject: [PATCH 09/10] apply xxchan's linguistic suggestion Signed-off-by: Bugen Zhao --- src/frontend/src/expr/type_inference/cast.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/frontend/src/expr/type_inference/cast.rs b/src/frontend/src/expr/type_inference/cast.rs index 8e4408e6dadd..13e5740c937a 100644 --- a/src/frontend/src/expr/type_inference/cast.rs +++ b/src/frontend/src/expr/type_inference/cast.rs @@ -118,16 +118,16 @@ pub fn align_array_and_element( /// Returns `Ok` if `ok` is true, otherwise returns a placeholder [`CastError`] to be further /// wrapped with a more informative context in [`cast`]. -fn canmeh(ok: bool) -> CastResult { +fn canbo(ok: bool) -> CastResult { if ok { Ok(()) } else { bail_cast_error!() } } -/// Equivalent to `canmeh(false)`. +/// Equivalent to `canbo(false)`. fn cannot() -> CastResult { - canmeh(false) + canbo(false) } /// Checks whether casting from `source` to `target` is ok in `allows` context. @@ -146,7 +146,7 @@ pub fn cast(source: &DataType, target: &DataType, allows: CastContext) -> Result } else if any!(is_map) { cast_map(source, target, allows) } else { - canmeh(cast_ok_base(source, target, allows)) + canbo(cast_ok_base(source, target, allows)) } .map_err(|inner| { // Only show "in .. context" once in the error source chain. @@ -218,8 +218,8 @@ fn cast_struct(source: &DataType, target: &DataType, allows: CastContext) -> Cas // The automatic casts to string types are treated as assignment casts, while the automatic // casts from string types are explicit-only. // https://www.postgresql.org/docs/14/sql-createcast.html#id-1.9.3.58.7.4 - (DataType::Varchar, DataType::Struct(_)) => canmeh(CastContext::Explicit <= allows), - (DataType::Struct(_), DataType::Varchar) => canmeh(CastContext::Assign <= allows), + (DataType::Varchar, DataType::Struct(_)) => canbo(CastContext::Explicit <= allows), + (DataType::Struct(_), DataType::Varchar) => canbo(CastContext::Assign <= allows), _ => cannot(), } } @@ -232,8 +232,8 @@ fn cast_array(source: &DataType, target: &DataType, allows: CastContext) -> Cast // The automatic casts to string types are treated as assignment casts, while the automatic // casts from string types are explicit-only. // https://www.postgresql.org/docs/14/sql-createcast.html#id-1.9.3.58.7.4 - (DataType::Varchar, DataType::List(_)) => canmeh(CastContext::Explicit <= allows), - (DataType::List(_), DataType::Varchar) => canmeh(CastContext::Assign <= allows), + (DataType::Varchar, DataType::List(_)) => canbo(CastContext::Explicit <= allows), + (DataType::List(_), DataType::Varchar) => canbo(CastContext::Assign <= allows), _ => cannot(), } } From 5edd505294b843c0d86a233c0b291c3e8e55ed12 Mon Sep 17 00:00:00 2001 From: Bugen Zhao Date: Fri, 22 Nov 2024 15:11:41 +0800 Subject: [PATCH 10/10] move it to `type_inference` and make it a variant of `RwError` Signed-off-by: Bugen Zhao --- src/frontend/src/error.rs | 8 ++++++ src/frontend/src/expr/function_call.rs | 26 +++----------------- src/frontend/src/expr/mod.rs | 7 +----- src/frontend/src/expr/type_inference/cast.rs | 14 ++++++++++- src/frontend/src/expr/type_inference/mod.rs | 3 ++- 5 files changed, 27 insertions(+), 31 deletions(-) diff --git a/src/frontend/src/error.rs b/src/frontend/src/error.rs index f0cf35e85966..d0615b9f0e13 100644 --- a/src/frontend/src/error.rs +++ b/src/frontend/src/error.rs @@ -26,6 +26,8 @@ use risingwave_rpc_client::error::{RpcError, TonicStatusWrapper}; use thiserror::Error; use tokio::task::JoinError; +use crate::expr::CastError; + /// The error type for the frontend crate, acting as the top-level error type for the /// entire RisingWave project. // TODO(error-handling): this is migrated from the `common` crate, and there could @@ -114,6 +116,12 @@ pub enum ErrorCode { #[backtrace] error: BoxedError, }, + #[error(transparent)] + CastError( + #[from] + #[backtrace] + CastError, + ), #[error("Catalog error: {0}")] CatalogError( #[source] diff --git a/src/frontend/src/expr/function_call.rs b/src/frontend/src/expr/function_call.rs index 0f4c36ce3a00..c5ae4ca178bf 100644 --- a/src/frontend/src/expr/function_call.rs +++ b/src/frontend/src/expr/function_call.rs @@ -16,13 +16,11 @@ use itertools::Itertools; use risingwave_common::catalog::Schema; use risingwave_common::types::{DataType, ScalarImpl}; use risingwave_common::util::iter_util::ZipEqFast; -use thiserror::Error; -use thiserror_ext::{Box, Macro}; use super::type_inference::cast; -use super::{infer_some_all, infer_type, CastContext, Expr, ExprImpl, Literal}; -use crate::error::{ErrorCode, Result as RwResult}; -use crate::expr::{ExprDisplay, ExprType, ExprVisitor, ImpureAnalyzer}; +use super::{infer_some_all, infer_type, CastContext, CastError, Expr, ExprImpl, Literal}; +use crate::error::Result as RwResult; +use crate::expr::{bail_cast_error, ExprDisplay, ExprType, ExprVisitor, ImpureAnalyzer}; #[derive(Clone, Eq, PartialEq, Hash)] pub struct FunctionCall { @@ -422,21 +420,3 @@ pub fn is_row_function(expr: &ExprImpl) -> bool { } false } - -/// A stack of error messages for the cast operation. -#[derive(Error, Debug, Box, Macro)] -#[thiserror_ext(newtype(name = CastError), macro(path = "crate::expr::function_call"))] -#[error("{message}")] -pub struct CastErrorInner { - pub source: Option, - pub message: Box, -} - -pub type CastResult = Result; - -// TODO(error-handling): shall we make it a new variant? -impl From for ErrorCode { - fn from(value: CastError) -> Self { - ErrorCode::Uncategorized(value.into()) - } -} diff --git a/src/frontend/src/expr/mod.rs b/src/frontend/src/expr/mod.rs index 96a90efd9db7..4cc3a1831939 100644 --- a/src/frontend/src/expr/mod.rs +++ b/src/frontend/src/expr/mod.rs @@ -14,7 +14,6 @@ use enum_as_inner::EnumAsInner; use fixedbitset::FixedBitSet; -use function_call::bail_cast_error; use futures::FutureExt; use paste::paste; use risingwave_common::array::ListValue; @@ -67,10 +66,7 @@ pub use risingwave_pb::expr::expr_node::Type as ExprType; pub use session_timezone::{SessionTimezone, TimestamptzExprFinder}; pub use subquery::{Subquery, SubqueryKind}; pub use table_function::{TableFunction, TableFunctionType}; -pub use type_inference::{ - align_types, cast_ok, cast_sigs, infer_some_all, infer_type, infer_type_name, - infer_type_with_sigmap, CastContext, CastSig, FuncSign, CAST_TABLE, -}; +pub use type_inference::*; pub use user_defined_function::UserDefinedFunction; pub use utils::*; pub use window_function::WindowFunction; @@ -1172,7 +1168,6 @@ use risingwave_common::bail; use risingwave_common::catalog::Schema; use risingwave_common::row::OwnedRow; -use self::function_call::CastError; use crate::binder::BoundSetExpr; use crate::utils::Condition; diff --git a/src/frontend/src/expr/type_inference/cast.rs b/src/frontend/src/expr/type_inference/cast.rs index 13e5740c937a..c9b09fe18eca 100644 --- a/src/frontend/src/expr/type_inference/cast.rs +++ b/src/frontend/src/expr/type_inference/cast.rs @@ -20,9 +20,10 @@ use itertools::Itertools as _; use parse_display::Display; use risingwave_common::types::{DataType, DataTypeName}; use risingwave_common::util::iter_util::{ZipEqDebug, ZipEqFast}; +use thiserror::Error; +use thiserror_ext::{Box, Macro}; use crate::error::ErrorCode; -use crate::expr::function_call::{bail_cast_error, cast_error, CastError, CastResult}; use crate::expr::{Expr as _, ExprImpl, InputRef, Literal}; /// Find the least restrictive type. Used by `VALUES`, `CASE`, `UNION`, etc. @@ -116,6 +117,17 @@ pub fn align_array_and_element( Ok(array_type) } +/// A stack of error messages for the cast operation. +#[derive(Error, Debug, Box, Macro)] +#[thiserror_ext(newtype(name = CastError), macro(path = "crate::expr"))] +#[error("{message}")] +pub struct CastErrorInner { + pub source: Option, + pub message: Box, +} + +pub type CastResult = Result; + /// Returns `Ok` if `ok` is true, otherwise returns a placeholder [`CastError`] to be further /// wrapped with a more informative context in [`cast`]. fn canbo(ok: bool) -> CastResult { diff --git a/src/frontend/src/expr/type_inference/mod.rs b/src/frontend/src/expr/type_inference/mod.rs index bc4e6477bf2d..4c507a958619 100644 --- a/src/frontend/src/expr/type_inference/mod.rs +++ b/src/frontend/src/expr/type_inference/mod.rs @@ -18,6 +18,7 @@ mod cast; mod func; pub use cast::{ - align_types, cast, cast_ok, cast_ok_base, cast_sigs, CastContext, CastSig, CAST_TABLE, + align_types, bail_cast_error, cast, cast_error, cast_ok, cast_ok_base, cast_sigs, CastContext, + CastError, CastErrorInner, CastSig, CAST_TABLE, }; pub use func::{infer_some_all, infer_type, infer_type_name, infer_type_with_sigmap, FuncSign};