diff --git a/src/query/ast/src/error.rs b/src/query/ast/src/error.rs index 7a34d666d87ce..1e35906cdb42c 100644 --- a/src/query/ast/src/error.rs +++ b/src/query/ast/src/error.rs @@ -186,9 +186,14 @@ pub fn display_parser_error(error: Error, source: &str) -> String { let mut labels = vec![]; // Plain text error has the highest priority. Only display it if exists. - for kind in &inner.errors { + for (span, kind) in error + .errors + .iter() + .map(|err| (error.span, err)) + .chain(inner.errors.iter().map(|err| (inner.span, err))) + { if let ErrorKind::Other(msg) = kind { - labels = vec![(inner.span, msg.to_string())]; + labels = vec![(span, msg.to_string())]; break; } } diff --git a/src/query/ast/src/parser/expr.rs b/src/query/ast/src/parser/expr.rs index d227aa610c03e..2978541297095 100644 --- a/src/query/ast/src/parser/expr.rs +++ b/src/query/ast/src/parser/expr.rs @@ -1070,7 +1070,7 @@ pub fn expr_element(i: Input) -> IResult> { // python style list comprehensions // python: [i for i in range(10) if i%2==0 ] // sql: [i for i in range(10) if i%2 = 0 ] - let list_comprehensions = check_experimental_chain_function( + let list_comprehensions = check_experimental_list_comprehension( true, map( rule! { @@ -1244,9 +1244,13 @@ pub fn column_id(i: Input) -> IResult { alt(( map_res(rule! { ColumnPosition }, |token| { let name = token.text().to_string(); - let pos = name[1..].parse::()?; + let pos = name[1..] + .parse::() + .map_err(|e| nom::Err::Failure(e.into()))?; if pos == 0 { - return Err(ErrorKind::Other("column position must be greater than 0")); + return Err(nom::Err::Failure(ErrorKind::Other( + "column position must be greater than 0", + ))); } Ok(ColumnID::Position(crate::ast::ColumnPosition { pos, @@ -1384,9 +1388,11 @@ pub fn literal_u64(i: Input) -> IResult { rule! { LiteralInteger }, - |token| Ok(u64::from_str_radix(token.text(), 10)?), + |token| u64::from_str_radix(token.text(), 10).map_err(|e| nom::Err::Failure(e.into())), ); - let hex = map_res(literal_hex_str, |lit| Ok(u64::from_str_radix(lit, 16)?)); + let hex = map_res(literal_hex_str, |lit| { + u64::from_str_radix(lit, 16).map_err(|e| nom::Err::Failure(e.into())) + }); rule!( #decimal @@ -1399,16 +1405,18 @@ pub fn literal_number(i: Input) -> IResult { rule! { LiteralInteger }, - |token| parse_uint(token.text(), 10), + |token| parse_uint(token.text(), 10).map_err(nom::Err::Failure), ); - let hex_uint = map_res(literal_hex_str, |str| parse_uint(str, 16)); + let hex_uint = map_res(literal_hex_str, |str| { + parse_uint(str, 16).map_err(nom::Err::Failure) + }); let decimal_float = map_res( rule! { LiteralFloat }, - |token| parse_float(token.text()), + |token| parse_float(token.text()).map_err(nom::Err::Failure), ); rule!( @@ -1436,11 +1444,12 @@ pub fn literal_string(i: Input) -> IResult { .is_some() { let str = &token.text()[1..token.text().len() - 1]; - let unescaped = unescape_string(str, '\'') - .ok_or(ErrorKind::Other("invalid escape or unicode"))?; + let unescaped = unescape_string(str, '\'').ok_or(nom::Err::Failure( + ErrorKind::Other("invalid escape or unicode"), + ))?; Ok(unescaped) } else { - Err(ErrorKind::ExpectToken(QuotedString)) + Err(nom::Err::Error(ErrorKind::ExpectToken(QuotedString))) } }, )(i) @@ -1452,7 +1461,7 @@ pub fn literal_string_eq_ignore_case(s: &str) -> impl FnMut(Input) -> IResult<() if token.text()[1..token.text().len() - 1].eq_ignore_ascii_case(s) { Ok(()) } else { - Err(ErrorKind::ExpectToken(QuotedString)) + Err(nom::Err::Error(ErrorKind::ExpectToken(QuotedString))) } })(i) } @@ -1510,11 +1519,11 @@ pub fn type_name(i: Input) -> IResult { Ok(TypeName::Decimal { precision: precision .try_into() - .map_err(|_| ErrorKind::Other("precision is too large"))?, + .map_err(|_| nom::Err::Failure(ErrorKind::Other("precision is too large")))?, scale: if let Some((_, scale)) = opt_scale { scale .try_into() - .map_err(|_| ErrorKind::Other("scale is too large"))? + .map_err(|_| nom::Err::Failure(ErrorKind::Other("scale is too large")))? } else { 0 }, @@ -1677,7 +1686,7 @@ pub fn map_access(i: Input) -> IResult { return Ok(MapAccessor::DotNumber { key }); } } - Err(ErrorKind::ExpectText(".")) + Err(nom::Err::Error(ErrorKind::ExpectText("."))) }, ); let colon = map( diff --git a/src/query/ast/src/parser/query.rs b/src/query/ast/src/parser/query.rs index dbeb1f1b131a5..725b24f0af7f8 100644 --- a/src/query/ast/src/parser/query.rs +++ b/src/query/ast/src/parser/query.rs @@ -32,6 +32,7 @@ use crate::parser::statement::hint; use crate::parser::token::*; use crate::rule; use crate::util::*; +use crate::ErrorKind; pub fn query(i: Input) -> IResult { context( @@ -41,7 +42,7 @@ pub fn query(i: Input) -> IResult { } pub fn set_operation(i: Input) -> IResult { - let (rest, set_operation_elements) = rule!(#set_operation_element+)(i)?; + let (rest, set_operation_elements) = rule! { #set_operation_element+ }(i)?; let iter = &mut set_operation_elements.into_iter(); run_pratt_parser(SetOperationParser, iter, rest, i) } @@ -52,13 +53,13 @@ pub enum SetOperationElement { SelectStmt { hints: Option, distinct: bool, - select_list: Box>, - from: Box>, - selection: Box>, + select_list: Vec, + from: Vec, + selection: Option, group_by: Option, - having: Box>, + having: Option, window_list: Option>, - qualify: Box>, + qualify: Option, }, SetOperation { op: SetOperator, @@ -97,84 +98,50 @@ pub fn set_operation_element(i: Input) -> IResult> } }, ); - let select_stmt = map( + let select_stmt = map_res( rule! { - SELECT ~ #hint? ~ DISTINCT? ~ ^#comma_separated_list1(select_target) - ~ ( FROM ~ ^#comma_separated_list1(table_reference) )? - ~ ( WHERE ~ ^#expr )? - ~ ( GROUP ~ ^BY ~ ^#group_by_items )? - ~ ( HAVING ~ ^#expr )? - ~ ( WINDOW ~ ^#comma_separated_list1(window_clause) )? - ~ ( QUALIFY ~ ^#expr )? + ( FROM ~ ^#comma_separated_list1(table_reference) )? + ~ SELECT ~ #hint? ~ DISTINCT? ~ ^#comma_separated_list1(select_target) + ~ ( FROM ~ ^#comma_separated_list1(table_reference) )? + ~ ( WHERE ~ ^#expr )? + ~ ( GROUP ~ ^BY ~ ^#group_by_items )? + ~ ( HAVING ~ ^#expr )? + ~ ( WINDOW ~ ^#comma_separated_list1(window_clause) )? + ~ ( QUALIFY ~ ^#expr )? }, |( + opt_from_block_first, _select, opt_hints, opt_distinct, select_list, - opt_from_block, + opt_from_block_second, opt_where_block, opt_group_by_block, opt_having_block, opt_window_block, opt_qualify_block, )| { - SetOperationElement::SelectStmt { - hints: opt_hints, - distinct: opt_distinct.is_some(), - select_list: Box::new(select_list), - from: Box::new( - opt_from_block - .map(|(_, table_refs)| table_refs) - .unwrap_or_default(), - ), - selection: Box::new(opt_where_block.map(|(_, selection)| selection)), - group_by: opt_group_by_block.map(|(_, _, group_by)| group_by), - having: Box::new(opt_having_block.map(|(_, having)| having)), - window_list: opt_window_block.map(|(_, windows)| windows), - qualify: Box::new(opt_qualify_block.map(|(_, qualify)| qualify)), + if opt_from_block_first.is_some() && opt_from_block_second.is_some() { + return Err(nom::Err::Failure(ErrorKind::Other( + "duplicated FROM clause", + ))); } - }, - ); - // From ... Select - let select_stmt_from_first = map( - rule! { - ( FROM ~ ^#comma_separated_list1(table_reference) )? - ~ SELECT ~ #hint? ~ DISTINCT? ~ ^#comma_separated_list1(select_target) - ~ ( WHERE ~ ^#expr )? - ~ ( GROUP ~ ^BY ~ ^#group_by_items )? - ~ ( HAVING ~ ^#expr )? - ~ ( WINDOW ~ ^#comma_separated_list1(window_clause) )? - ~ ( QUALIFY ~ ^#expr )? - }, - |( - opt_from_block, - _select, - opt_hints, - opt_distinct, - select_list, - opt_where_block, - opt_group_by_block, - opt_having_block, - opt_window_block, - opt_qualify_block, - )| { - SetOperationElement::SelectStmt { + Ok(SetOperationElement::SelectStmt { hints: opt_hints, distinct: opt_distinct.is_some(), - select_list: Box::new(select_list), - from: Box::new( - opt_from_block - .map(|(_, table_refs)| table_refs) - .unwrap_or_default(), - ), - selection: Box::new(opt_where_block.map(|(_, selection)| selection)), + select_list, + from: opt_from_block_first + .or(opt_from_block_second) + .map(|(_, table_refs)| table_refs) + .unwrap_or_default(), + selection: opt_where_block.map(|(_, selection)| selection), group_by: opt_group_by_block.map(|(_, _, group_by)| group_by), - having: Box::new(opt_having_block.map(|(_, having)| having)), + having: opt_having_block.map(|(_, having)| having), window_list: opt_window_block.map(|(_, windows)| windows), - qualify: Box::new(opt_qualify_block.map(|(_, qualify)| qualify)), - } + qualify: opt_qualify_block.map(|(_, qualify)| qualify), + }) }, ); @@ -220,7 +187,6 @@ pub fn set_operation_element(i: Input) -> IResult> | #with | #set_operator | #select_stmt - | #select_stmt_from_first | #values | #order_by | #limit @@ -274,13 +240,13 @@ impl<'a, I: Iterator>> PrattParser span: transform_span(input.span.0), hints, distinct, - select_list: *select_list, - from: *from, - selection: *selection, + select_list, + from, + selection, group_by, - having: *having, + having, window_list, - qualify: *qualify, + qualify, })), SetOperationElement::Values(values) => SetExpr::Values { span: transform_span(input.span.0), @@ -626,7 +592,7 @@ pub fn order_by_expr(i: Input) -> IResult { } pub fn table_reference(i: Input) -> IResult { - let (rest, table_reference_elements) = rule!(#table_reference_element+)(i)?; + let (rest, table_reference_elements) = rule! { #table_reference_element+ }(i)?; let iter = &mut table_reference_elements.into_iter(); run_pratt_parser(TableReferenceParser, iter, rest, i) } diff --git a/src/query/ast/src/parser/share.rs b/src/query/ast/src/parser/share.rs index 01aa1506f2fe6..fa9348af19458 100644 --- a/src/query/ast/src/parser/share.rs +++ b/src/query/ast/src/parser/share.rs @@ -28,7 +28,7 @@ pub fn share_endpoint_uri_location(i: Input) -> IResult { }, |location| { UriLocation::from_uri(location, "".to_string(), BTreeMap::new()) - .map_err(|_| ErrorKind::Other("invalid uri")) + .map_err(|_| nom::Err::Failure(ErrorKind::Other("invalid uri"))) }, )(i) } diff --git a/src/query/ast/src/parser/stage.rs b/src/query/ast/src/parser/stage.rs index 7b295f83fd395..a84efa3858169 100644 --- a/src/query/ast/src/parser/stage.rs +++ b/src/query/ast/src/parser/stage.rs @@ -165,13 +165,17 @@ pub fn file_location(i: Input) -> IResult { pub fn stage_location(i: Input) -> IResult { map_res(file_location, |location| match location { FileLocation::Stage(s) => Ok(s), - FileLocation::Uri(_) => Err(ErrorKind::Other("expect stage location, got uri location")), + FileLocation::Uri(_) => Err(nom::Err::Failure(ErrorKind::Other( + "expect stage location, got uri location", + ))), })(i) } pub fn uri_location(i: Input) -> IResult { map_res(string_location, |location| match location { - FileLocation::Stage(_) => Err(ErrorKind::Other("uri location should not start with '@'")), + FileLocation::Stage(_) => Err(nom::Err::Failure(ErrorKind::Other( + "uri location should not start with '@'", + ))), FileLocation::Uri(u) => Ok(u), })(i) } @@ -192,7 +196,9 @@ pub fn string_location(i: Input) -> IResult { { Ok(FileLocation::Stage(stripped.to_string())) } else { - Err(ErrorKind::Other("uri location should not start with '@'")) + Err(nom::Err::Failure(ErrorKind::Other( + "uri location should not start with '@'", + ))) } } else { let part_prefix = if let Some((_, _, p, _)) = location_prefix { @@ -207,7 +213,7 @@ pub fn string_location(i: Input) -> IResult { conns.extend(credentials_opts.map(|v| v.2).unwrap_or_default()); let uri = UriLocation::from_uri(location, part_prefix, conns) - .map_err(|_| ErrorKind::Other("invalid uri"))?; + .map_err(|_| nom::Err::Failure(ErrorKind::Other("invalid uri")))?; Ok(FileLocation::Uri(uri)) } }, diff --git a/src/query/ast/src/parser/statement.rs b/src/query/ast/src/parser/statement.rs index c58c22cf2e7f7..8cb842ae11ebb 100644 --- a/src/query/ast/src/parser/statement.rs +++ b/src/query/ast/src/parser/statement.rs @@ -68,13 +68,17 @@ pub fn statement(i: Input) -> IResult { Ok(Statement::Explain { kind: match opt_kind.map(|token| token.kind) { Some(TokenKind::AST) => { - let formatted_stmt = format_statement(statement.stmt.clone()) - .map_err(|_| ErrorKind::Other("invalid statement"))?; + let formatted_stmt = + format_statement(statement.stmt.clone()).map_err(|_| { + nom::Err::Failure(ErrorKind::Other("invalid statement")) + })?; ExplainKind::Ast(formatted_stmt) } Some(TokenKind::SYNTAX) => { - let pretty_stmt = pretty_statement(statement.stmt.clone(), 10) - .map_err(|_| ErrorKind::Other("invalid statement"))?; + let pretty_stmt = + pretty_statement(statement.stmt.clone(), 10).map_err(|_| { + nom::Err::Failure(ErrorKind::Other("invalid statement")) + })?; ExplainKind::Syntax(pretty_stmt) } Some(TokenKind::PIPELINE) => ExplainKind::Pipeline, diff --git a/src/query/ast/src/util.rs b/src/query/ast/src/util.rs index 860ac23094223..bfabe628735d5 100644 --- a/src/query/ast/src/util.rs +++ b/src/query/ast/src/util.rs @@ -318,14 +318,16 @@ pub fn map_res<'a, O1, O2, F, G>( ) -> impl FnMut(Input<'a>) -> IResult<'a, O2> where F: nom::Parser, O1, Error<'a>>, - G: FnMut(O1) -> Result, + G: FnMut(O1) -> Result>, { move |input: Input| { let i = input; let (input, o1) = parser.parse(input)?; match f(o1) { Ok(o2) => Ok((input, o2)), - Err(e) => Err(nom::Err::Error(Error::from_error_kind(i, e))), + Err(nom::Err::Error(e)) => Err(nom::Err::Error(Error::from_error_kind(i, e))), + Err(nom::Err::Failure(e)) => Err(nom::Err::Failure(Error::from_error_kind(i, e))), + Err(nom::Err::Incomplete(_)) => unimplemented!(), } } } @@ -443,3 +445,4 @@ macro_rules! declare_experimental_feature { } declare_experimental_feature!(check_experimental_chain_function, "chain function"); +declare_experimental_feature!(check_experimental_list_comprehension, "list comprehension"); diff --git a/src/query/ast/tests/it/parser.rs b/src/query/ast/tests/it/parser.rs index 30bd93b17e4b3..d5cc254802f27 100644 --- a/src/query/ast/tests/it/parser.rs +++ b/src/query/ast/tests/it/parser.rs @@ -623,6 +623,7 @@ fn test_statement_error() { error_on_column_count_mismatch = 1 )"#, r#"CREATE CONNECTION IF NOT EXISTS my_conn"#, + r#"select $0 from t1"#, ]; for case in cases { @@ -704,6 +705,8 @@ fn test_query_error() { let file = &mut mint.new_goldenfile("query-error.txt").unwrap(); let cases = &[ r#"select * from customer join where a = b"#, + r#"from t1 select * from t2"#, + r#"from t1 select * from t2 where a = b"#, r#"select * from join customer"#, r#"select * from customer natural inner join orders on a = b"#, r#"select * order a"#, @@ -815,7 +818,7 @@ fn test_expr() { #[test] fn test_experimental_expr() { let mut mint = Mint::new("tests/it/testdata"); - let file = &mut mint.new_goldenfile("experimental_expr.txt").unwrap(); + let file = &mut mint.new_goldenfile("experimental-expr.txt").unwrap(); let cases = &[ r#"a"#, @@ -825,6 +828,7 @@ fn test_experimental_expr() { r#"1 + {'k1': 4}.k1"#, r#"'3'.plus(4)"#, r#"(3).add({'k1': 4 }.k1)"#, + r#"[ x * 100 FOR x in [1,2,3] if x % 2 = 0 ]"#, ]; for case in cases { @@ -844,6 +848,7 @@ fn test_expr_error() { r#"1 a"#, r#"CAST(col1)"#, r#"a.add(b)"#, + r#"[ x * 100 FOR x in [1,2,3] if x % 2 = 0 ]"#, r#"G.E.B IS NOT NULL AND col1 NOT BETWEEN col2 AND AND 1 + col3 DIV sum(col4)"#, diff --git a/src/query/ast/tests/it/testdata/experimental_expr.txt b/src/query/ast/tests/it/testdata/experimental-expr.txt similarity index 64% rename from src/query/ast/tests/it/testdata/experimental_expr.txt rename to src/query/ast/tests/it/testdata/experimental-expr.txt index 49a8b15a0b246..7583dee4a69ed 100644 --- a/src/query/ast/tests/it/testdata/experimental_expr.txt +++ b/src/query/ast/tests/it/testdata/experimental-expr.txt @@ -423,3 +423,171 @@ FunctionCall { } +---------- Input ---------- +[ x * 100 FOR x in [1,2,3] if x % 2 = 0 ] +---------- Output --------- +array_map(array_filter([1, 2, 3], x -> ((x % 2) = 0)), x -> (x * 100)) +---------- AST ------------ +FunctionCall { + span: Some( + 0..41, + ), + distinct: false, + name: Identifier { + name: "array_map", + quote: None, + span: None, + }, + args: [ + FunctionCall { + span: Some( + 0..41, + ), + distinct: false, + name: Identifier { + name: "array_filter", + quote: None, + span: None, + }, + args: [ + Array { + span: Some( + 19..26, + ), + exprs: [ + Literal { + span: Some( + 20..21, + ), + lit: UInt64( + 1, + ), + }, + Literal { + span: Some( + 22..23, + ), + lit: UInt64( + 2, + ), + }, + Literal { + span: Some( + 24..25, + ), + lit: UInt64( + 3, + ), + }, + ], + }, + ], + params: [], + window: None, + lambda: Some( + Lambda { + params: [ + Identifier { + name: "x", + quote: None, + span: Some( + 14..15, + ), + }, + ], + expr: BinaryOp { + span: Some( + 36..37, + ), + op: Eq, + left: BinaryOp { + span: Some( + 32..33, + ), + op: Modulo, + left: ColumnRef { + span: Some( + 30..31, + ), + database: None, + table: None, + column: Name( + Identifier { + name: "x", + quote: None, + span: Some( + 30..31, + ), + }, + ), + }, + right: Literal { + span: Some( + 34..35, + ), + lit: UInt64( + 2, + ), + }, + }, + right: Literal { + span: Some( + 38..39, + ), + lit: UInt64( + 0, + ), + }, + }, + }, + ), + }, + ], + params: [], + window: None, + lambda: Some( + Lambda { + params: [ + Identifier { + name: "x", + quote: None, + span: Some( + 14..15, + ), + }, + ], + expr: BinaryOp { + span: Some( + 4..5, + ), + op: Multiply, + left: ColumnRef { + span: Some( + 2..3, + ), + database: None, + table: None, + column: Name( + Identifier { + name: "x", + quote: None, + span: Some( + 2..3, + ), + }, + ), + }, + right: Literal { + span: Some( + 6..9, + ), + lit: UInt64( + 100, + ), + }, + }, + }, + ), +} + + diff --git a/src/query/ast/tests/it/testdata/expr-error.txt b/src/query/ast/tests/it/testdata/expr-error.txt index 0d40357b074e5..622e361d3d24c 100644 --- a/src/query/ast/tests/it/testdata/expr-error.txt +++ b/src/query/ast/tests/it/testdata/expr-error.txt @@ -73,6 +73,20 @@ error: | while parsing expression +---------- Input ---------- +[ x * 100 FOR x in [1,2,3] if x % 2 = 0 ] +---------- Output --------- +error: + --> SQL:1:1 + | +1 | [ x * 100 FOR x in [1,2,3] if x % 2 = 0 ] + | ^ + | | + | list comprehension only works in experimental dialect, try `set sql_dialect = experimental` + | while parsing expression + | while parsing [expr for x in ... [if ...]] + + ---------- Input ---------- G.E.B IS NOT NULL AND col1 NOT BETWEEN col2 AND diff --git a/src/query/ast/tests/it/testdata/query-error.txt b/src/query/ast/tests/it/testdata/query-error.txt index bd09b7c29256c..a952f70c48765 100644 --- a/src/query/ast/tests/it/testdata/query-error.txt +++ b/src/query/ast/tests/it/testdata/query-error.txt @@ -10,6 +10,32 @@ error: | while parsing `SELECT ...` +---------- Input ---------- +from t1 select * from t2 +---------- Output --------- +error: + --> SQL:1:1 + | +1 | from t1 select * from t2 + | ^^^^ + | | + | duplicated FROM clause + | while parsing `SELECT ...` + + +---------- Input ---------- +from t1 select * from t2 where a = b +---------- Output --------- +error: + --> SQL:1:1 + | +1 | from t1 select * from t2 where a = b + | ^^^^ + | | + | duplicated FROM clause + | while parsing `SELECT ...` + + ---------- Input ---------- select * from join customer ---------- Output --------- diff --git a/src/query/ast/tests/it/testdata/statement-error.txt b/src/query/ast/tests/it/testdata/statement-error.txt index 78abfdf75bea4..4807af73684fd 100644 --- a/src/query/ast/tests/it/testdata/statement-error.txt +++ b/src/query/ast/tests/it/testdata/statement-error.txt @@ -743,3 +743,18 @@ error: | while parsing `CREATE CONNECTION [IF NOT EXISTS] STORAGE_TYPE = ` +---------- Input ---------- +select $0 from t1 +---------- Output --------- +error: + --> SQL:1:8 + | +1 | select $0 from t1 + | ------ ^^ + | | | + | | column position must be greater than 0 + | | while parsing expression + | | while parsing + | while parsing `SELECT ...` + + diff --git a/tests/sqllogictests/suites/query/column_position.test b/tests/sqllogictests/suites/query/column_position.test index fc87fc04a69ca..899452715c054 100644 --- a/tests/sqllogictests/suites/query/column_position.test +++ b/tests/sqllogictests/suites/query/column_position.test @@ -30,7 +30,7 @@ select * from t1 where $1 = 1; ---- 1 a -statement error 1065 +statement error 1005 select $0 from t1 statement error 1065