Skip to content

Commit

Permalink
chore: improve error message for function argument mismatch (#14435)
Browse files Browse the repository at this point in the history
Signed-off-by: Runji Wang <wangrunji0408@163.com>
  • Loading branch information
wangrunji0408 committed Jan 9, 2024
1 parent 0799621 commit a30583f
Show file tree
Hide file tree
Showing 8 changed files with 50 additions and 39 deletions.
22 changes: 20 additions & 2 deletions src/common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,24 @@ pub struct NotImplemented {
pub issue: TrackingIssue,
}

#[derive(Error, Debug, Macro)]
#[thiserror_ext(macro(path = "crate::error"))]
pub struct NoFunction {
#[message]
pub sig: String,
pub candidates: Option<String>,
}

impl Display for NoFunction {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(f, "function {} does not exist", self.sig)?;
if let Some(candidates) = &self.candidates {
write!(f, ", do you mean {}", candidates)?;
}
Ok(())
}
}

#[derive(Error, Debug, Box)]
#[thiserror_ext(newtype(name = RwError, backtrace, report_debug))]
pub enum ErrorCode {
Expand All @@ -107,8 +125,8 @@ pub enum ErrorCode {
// Tips: Use this only if it's intended to reject the query
#[error("Not supported: {0}\nHINT: {1}")]
NotSupported(String, String),
#[error("function {0} does not exist")]
NoFunction(String),
#[error(transparent)]
NoFunction(#[from] NoFunction),
#[error(transparent)]
IoError(#[from] IoError),
#[error("Storage error: {0}")]
Expand Down
6 changes: 2 additions & 4 deletions src/frontend/planner_test/tests/testdata/output/expr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@
Failed to bind expression: must_be_unimplemented_func(1)
Caused by:
Feature is not yet implemented: unsupported function: "must_be_unimplemented_func"
Tracking issue: https://github.com/risingwavelabs/risingwave/issues/112
function must_be_unimplemented_func(integer) does not exist
- sql: |
values(cast(1 as bigint));
batch_plan: 'BatchValues { rows: [[1:Int64]] }'
Expand Down Expand Up @@ -573,8 +572,7 @@
Failed to bind expression: pg_teminate_backend(1)
Caused by:
Feature is not yet implemented: unsupported function "pg_teminate_backend", do you mean "pg_terminate_backend"?
Tracking issue: https://github.com/risingwavelabs/risingwave/issues/112
function pg_teminate_backend(integer) does not exist, do you mean pg_terminate_backend
- name: regression (#7571) - literal debug display for array with NULL values
sql: |
select ARRAY[1, null] t;
Expand Down
3 changes: 1 addition & 2 deletions src/frontend/planner_test/tests/testdata/output/nexmark.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1071,8 +1071,7 @@
Failed to bind expression: SESSION_START(B.date_time, INTERVAL '10' SECOND)
Caused by:
Feature is not yet implemented: unsupported function: "session_start"
Tracking issue: https://github.com/risingwavelabs/risingwave/issues/112
function session_start(timestamp without time zone, interval) does not exist
- id: nexmark_q12
before:
- create_tables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -954,8 +954,7 @@
Failed to bind expression: SESSION_START(B.date_time, INTERVAL '10' SECOND)
Caused by:
Feature is not yet implemented: unsupported function: "session_start"
Tracking issue: https://github.com/risingwavelabs/risingwave/issues/112
function session_start(timestamp without time zone, interval) does not exist
- id: nexmark_q12
before:
- create_sources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -913,8 +913,7 @@
Failed to bind expression: SESSION_START(B.date_time, INTERVAL '10' SECOND)
Caused by:
Feature is not yet implemented: unsupported function: "session_start"
Tracking issue: https://github.com/risingwavelabs/risingwave/issues/112
function session_start(timestamp without time zone, interval) does not exist
- id: nexmark_q12
before:
- create_sources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1171,8 +1171,7 @@
Failed to bind expression: SESSION_START(B.date_time, INTERVAL '10' SECOND)
Caused by:
Feature is not yet implemented: unsupported function: "session_start"
Tracking issue: https://github.com/risingwavelabs/risingwave/issues/112
function session_start(timestamp without time zone, interval) does not exist
- id: nexmark_q12
before:
- create_sources
Expand Down
31 changes: 13 additions & 18 deletions src/frontend/src/binder/expr/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use risingwave_common::catalog::{INFORMATION_SCHEMA_SCHEMA_NAME, PG_CATALOG_SCHE
use risingwave_common::error::{ErrorCode, Result, RwError};
use risingwave_common::session_config::USER_NAME_WILD_CARD;
use risingwave_common::types::{DataType, ScalarImpl, Timestamptz};
use risingwave_common::{bail_not_implemented, current_cluster_version, not_implemented};
use risingwave_common::{bail_not_implemented, current_cluster_version, no_function};
use risingwave_expr::aggregate::{agg_kinds, AggKind};
use risingwave_expr::window_function::{
Frame, FrameBound, FrameBounds, FrameExclusion, WindowFuncKind,
Expand Down Expand Up @@ -1369,27 +1369,22 @@ impl Binder {

match HANDLES.get(function_name) {
Some(handle) => handle(self, inputs),
None => Err({
None => {
let allowed_distance = if function_name.len() > 3 { 2 } else { 1 };

let candidates = FUNCTIONS_BKTREE
.find(function_name, allowed_distance)
.map(|(_idx, c)| c);

let mut candidates = candidates.peekable();

let err_msg = if candidates.peek().is_none() {
format!("unsupported function: \"{}\"", function_name)
} else {
format!(
"unsupported function \"{}\", do you mean \"{}\"?",
function_name,
candidates.join(" or ")
)
};

not_implemented!(issue = 112, "{}", err_msg).into()
}),
.map(|(_idx, c)| c)
.join(" or ");

Err(no_function!(
candidates = (!candidates.is_empty()).then_some(candidates),
"{}({})",
function_name,
inputs.iter().map(|e| e.return_type()).join(", ")
)
.into())
}
}
}

Expand Down
18 changes: 11 additions & 7 deletions src/frontend/src/expr/type_inference/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use itertools::Itertools as _;
use num_integer::Integer as _;
use risingwave_common::bail_no_function;
use risingwave_common::error::{ErrorCode, Result};
use risingwave_common::types::{DataType, StructType};
use risingwave_common::util::iter_util::ZipEqFast;
Expand Down Expand Up @@ -643,13 +644,17 @@ pub fn infer_type_name<'a>(

let mut candidates = top_matches(&candidates, inputs);

if candidates.is_empty() {
return Err(ErrorCode::NoFunction(format!(
// show function in error message
let sig = || {
format!(
"{}({})",
func_name,
inputs.iter().map(TypeDisplay).format(", ")
))
.into());
)
};

if candidates.is_empty() {
bail_no_function!("{}", sig());
}

// After this line `candidates` will never be empty, as the narrow rules will retain original
Expand All @@ -663,9 +668,8 @@ pub fn infer_type_name<'a>(
[] => unreachable!(),
[sig] => Ok(*sig),
_ => Err(ErrorCode::BindError(format!(
"function {}({}) is not unique\nHINT: Could not choose a best candidate function. You might need to add explicit type casts.",
func_name,
inputs.iter().map(TypeDisplay).format(", "),
"function {} is not unique\nHINT: Could not choose a best candidate function. You might need to add explicit type casts.",
sig(),
))
.into()),
}
Expand Down

0 comments on commit a30583f

Please sign in to comment.