From f40baa6e20ca25f866e40fba39b1d4f9f0047a08 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 14:04:48 +0000 Subject: [PATCH 1/4] fix(syntax): point a dangling operator before GROUP BY or ORDER BY at the clause GROUP or ORDER before BY read as a column name, so a condition left open before the clause, as in the playground's syntax error example, failed one token late, at BY. GROUP or ORDER before BY now always starts the clause, and the error points at it. --- .../src/parser/_helpers/statements/analyze.rs | 6 +- .../tests/parser/test_fixes.rs | 59 ++++++++++++++++++- .../tests/parser/test_types.rs | 6 ++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/analyze.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/analyze.rs index f7a3176dc..e9e6195cb 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/analyze.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/statements/analyze.rs @@ -122,7 +122,7 @@ impl Parser { false } - /// Whether a keyword may be an identifier: structural ones (FROM, WHERE, ...) may not. + /// Whether a keyword may be an identifier: structural ones (FROM, WHERE, ...) may not, nor GROUP or ORDER before BY, which start a clause. pub(crate) fn is_safe_keyword_as_identifier(&self) -> bool { if self.is_at_end() { return false; @@ -156,8 +156,10 @@ impl Parser { | TokenType::Lateral | TokenType::Natural ); + let starts_clause = matches!(token_type, TokenType::Group | TokenType::Order) + && self.check_next(TokenType::By); - self.peek().token_type.is_keyword() && !is_structural + self.peek().token_type.is_keyword() && !is_structural && !starts_clause } /// Whether the token here ends the expression, so IS or KEEP is an alias, not an operator. diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs index b2559da73..dfcbadab0 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs @@ -1,10 +1,12 @@ use serde_json::json; -use sqlquack_syntax::{CallMode, Dialect}; +use sqlquack_syntax::{CallMode, Dialect, ParseOptions}; +use sqlquack_syntax_snowflake::parse_statements_with; use crate::helpers::{accepted, cast_targets, projection_at, statement_at, statements}; use crate::test_types::{ - AcceptanceTestCase, CastTargetTestCase, ProjectionTestCase, StatementTestCase, + AcceptanceTestCase, CastTargetTestCase, ParseErrorTestCase, ProjectionTestCase, + StatementTestCase, }; #[test] @@ -1950,3 +1952,56 @@ fn given_a_keyword_lambda_parameter_when_parsing_duckdb_then_it_names_the_parame ); } } + +#[test] +fn given_group_or_order_by_after_an_operator_when_parsing_snowflake_then_the_error_is_at_the_clause( +) { + let test_cases = [ + ParseErrorTestCase { + description: "AND before GROUP BY on the next line fails at GROUP, not at BY", + sql: "SELECT customer_id, SUM(total) AS revenue\nFROM orders\nWHERE status = 'shipped' AND\nGROUP BY customer_id", + expected_error_at: Some("GROUP"), + }, + ParseErrorTestCase { + description: "AND before ORDER BY on the same line fails at ORDER", + sql: "SELECT order_id FROM orders WHERE total > 1 AND ORDER BY order_id", + expected_error_at: Some("ORDER"), + }, + ParseErrorTestCase { + description: "NOT before GROUP BY fails at GROUP", + sql: "SELECT status FROM orders WHERE NOT\ngroup by status", + expected_error_at: Some("group"), + }, + ParseErrorTestCase { + description: "GROUP BY and ORDER BY after WHERE still parse", + sql: "SELECT status FROM orders WHERE total > 1\nGROUP BY status\nORDER BY status", + expected_error_at: None, + }, + ParseErrorTestCase { + description: "a quoted GROUP column still parses", + sql: "SELECT \"GROUP\" FROM orders ORDER BY 1", + expected_error_at: None, + }, + ]; + + for test_case in test_cases { + let error_at = parse_statements_with( + test_case.sql, + Dialect::Snowflake, + &ParseOptions::default(), + CallMode::Faithful, + ) + .err() + .map(|error| { + let start = error.start().unwrap_or_default(); + let end = error.end().unwrap_or_default(); + test_case.sql.get(start..end).unwrap_or_default().to_owned() + }); + assert_eq!( + error_at.as_deref(), + test_case.expected_error_at, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs index d7908b555..8b169ab98 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs @@ -67,6 +67,12 @@ pub(crate) struct ScriptingVerdictTestCase { pub(crate) expected_error: Option<&'static str>, } +pub(crate) struct ParseErrorTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_error_at: Option<&'static str>, +} + pub(crate) struct ScriptingDialectTestCase { pub(crate) description: &'static str, pub(crate) sql: &'static str, From 9db9b3088e93e2d238da69b4f7a874c3d36ab678 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 14:34:19 +0000 Subject: [PATCH 2/4] fix(syntax): name what is missing when a statement ends where an expression belongs A statement that ended where an expression belongs, such as an empty WHERE, reported the token before the gap: "Unexpected token: Where". It now says what is missing: a condition after WHERE, HAVING, QUALIFY or ON, and an expression after anything else, such as an operator. --- .../parser/_helpers/expressions/primary.rs | 9 ++++ .../src/parser/constants.rs | 3 ++ .../tests/parser/test_fixes.rs | 53 ++++++++++++++++++- .../tests/parser/test_types.rs | 6 +++ 4 files changed, 69 insertions(+), 2 deletions(-) diff --git a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/primary.rs b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/primary.rs index 24b124043..f7c24d367 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/primary.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/_helpers/expressions/primary.rs @@ -1,6 +1,7 @@ //! Parser expressions: primary. use crate::parser::constants::ARRAY; +use crate::parser::constants::CONDITION_CLAUSES; use crate::parser::constants::MAP; use crate::parser::constants::ODBC_DATE_ESCAPE; use crate::parser::constants::ODBC_TIMESTAMP_ESCAPE; @@ -441,6 +442,14 @@ impl Parser { return self.maybe_parse_subscript(col); } + if self.is_at_end() || self.check(TokenType::Semicolon) { + let previous = self.previous().text.to_ascii_uppercase(); + let expected = match CONDITION_CLAUSES.contains(&previous.as_str()) { + true => "a condition", + false => "an expression", + }; + return Err(self.parse_error(format!("Expected {expected} after {previous}"))); + } Err(self.parse_error(format!("Unexpected token: {:?}", self.peek().token_type))) } diff --git a/crates/sqlquack-syntax-snowflake/src/parser/constants.rs b/crates/sqlquack-syntax-snowflake/src/parser/constants.rs index ddadcf326..b86e98f10 100644 --- a/crates/sqlquack-syntax-snowflake/src/parser/constants.rs +++ b/crates/sqlquack-syntax-snowflake/src/parser/constants.rs @@ -191,6 +191,9 @@ pub(crate) const SET: &str = "SET"; /// The text "SET MASKING POLICY", as comparisons test it. pub(crate) const SET_MASKING_POLICY: &str = "SET MASKING POLICY"; +/// The clauses whose operand is a condition, as an error names what is missing after one. +pub(crate) const CONDITION_CLAUSES: &[&str] = &["HAVING", "ON", "QUALIFY", "WHERE"]; + /// The text "SET TAG", as comparisons test it. pub(crate) const SET_TAG: &str = "SET TAG"; diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs index dfcbadab0..713223b1e 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs @@ -5,8 +5,8 @@ use sqlquack_syntax_snowflake::parse_statements_with; use crate::helpers::{accepted, cast_targets, projection_at, statement_at, statements}; use crate::test_types::{ - AcceptanceTestCase, CastTargetTestCase, ParseErrorTestCase, ProjectionTestCase, - StatementTestCase, + AcceptanceTestCase, CastTargetTestCase, ParseErrorTestCase, ParseMessageTestCase, + ProjectionTestCase, StatementTestCase, }; #[test] @@ -2005,3 +2005,52 @@ fn given_group_or_order_by_after_an_operator_when_parsing_snowflake_then_the_err ); } } + +#[test] +fn given_a_statement_ending_where_an_expression_belongs_when_parsing_snowflake_then_the_error_names_what_is_missing( +) { + let test_cases = [ + ParseMessageTestCase { + description: "an empty WHERE asks for a condition", + sql: "SELECT * FROM orders WHERE", + expected_message: "Expected a condition after WHERE", + }, + ParseMessageTestCase { + description: "an empty WHERE before a semicolon asks for a condition", + sql: "SELECT * FROM orders WHERE ;", + expected_message: "Expected a condition after WHERE", + }, + ParseMessageTestCase { + description: "an empty HAVING asks for a condition", + sql: "SELECT status FROM orders GROUP BY status HAVING", + expected_message: "Expected a condition after HAVING", + }, + ParseMessageTestCase { + description: "a join's empty ON asks for a condition", + sql: "SELECT 1 FROM orders o JOIN customers c ON", + expected_message: "Expected a condition after ON", + }, + ParseMessageTestCase { + description: "a comparison with no right side asks for an expression", + sql: "SELECT 1 FROM orders WHERE total >", + expected_message: "Expected an expression after >", + }, + ]; + + for test_case in test_cases { + let message = parse_statements_with( + test_case.sql, + Dialect::Snowflake, + &ParseOptions::default(), + CallMode::Faithful, + ) + .err() + .map(|error| error.to_string()) + .unwrap_or_default(); + assert!( + message.contains(test_case.expected_message), + "{}: {message}", + test_case.description + ); + } +} diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs index 8b169ab98..a81b8a118 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_types.rs @@ -73,6 +73,12 @@ pub(crate) struct ParseErrorTestCase { pub(crate) expected_error_at: Option<&'static str>, } +pub(crate) struct ParseMessageTestCase { + pub(crate) description: &'static str, + pub(crate) sql: &'static str, + pub(crate) expected_message: &'static str, +} + pub(crate) struct ScriptingDialectTestCase { pub(crate) description: &'static str, pub(crate) sql: &'static str, From 3eea2bc12cde1681a47dc7715aaa2ffc8769a4a3 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 14:34:19 +0000 Subject: [PATCH 3/4] fix(diagnostics): keep node names and guessed aliases out of the unaliased column message The SQ0009 label named the internal node kind, such as `j_s_o_n_extract`, and its help suggested an alias guessed from the text under the span. For a path that was only its first name, and for a literal a quoted copy of the literal. The label now reads plainly, and a literal or path gets the help to add an alias without a guessed name. --- .../src/snowflake/conversion/_helpers/queries/query.rs | 9 +++++++-- .../src/snowflake/conversion/constants.rs | 3 +++ crates/sqlquack-diagnostics/src/diagnostics/constants.rs | 7 +++++++ .../src/diagnostics/main/diagnose.rs | 8 +++++--- crates/sqlquack/tests/cli/test_convert.rs | 2 +- crates/sqlquack/tests/cli/test_output_names.rs | 4 ++-- 6 files changed, 25 insertions(+), 8 deletions(-) diff --git a/crates/sqlquack-convert/src/snowflake/conversion/_helpers/queries/query.rs b/crates/sqlquack-convert/src/snowflake/conversion/_helpers/queries/query.rs index ba93276db..14985124d 100644 --- a/crates/sqlquack-convert/src/snowflake/conversion/_helpers/queries/query.rs +++ b/crates/sqlquack-convert/src/snowflake/conversion/_helpers/queries/query.rs @@ -4,7 +4,8 @@ use serde_json::{Map, Value, json}; use sqlquack_blocks::blocks::main::quoted_identifier::quoted_identifier; use sqlquack_diagnostics::classification::models::{Construct, ConstructFamily}; use sqlquack_diagnostics::diagnostics::constants::{ - REPEATED_RESULT_NAME, UNALIASED_OUTPUT_NAME_CONSTRUCT, + REPEATED_RESULT_NAME, UNALIASED_OUTPUT_NAME_CONSTRUCT, UNALIASED_QUOTED_NAME, + UNALIASED_TEXT_NAME, }; use sqlquack_diagnostics::provenance::constants::QUERY_CONSTRUCT; use sqlquack_diagnostics::provenance::models::ShapeArgument; @@ -34,6 +35,7 @@ use crate::snowflake::conversion::_helpers::support::tree::{ function_construct, identifier_name, list, node_construct, node_kind, payload_object, require_understood, }; +use crate::snowflake::conversion::constants::UNALIASED_QUOTED_KINDS; use crate::snowflake::conversion::constants::WINDOW_MARKER; use crate::snowflake::conversion::constants::{ ALIAS, COMMENT, COMMON_TABLE, DISTINCT, GENERATOR_SEQUENCE_COLUMN, GROUP_BY, HAVING, @@ -694,7 +696,10 @@ fn convert_projection( location: site.context.locator.locate_written(item), ..missing( Construct::new(ConstructFamily::Syntax, UNALIASED_OUTPUT_NAME_CONSTRUCT), - format!("Snowflake names this `{kind}` column after its text"), + match UNALIASED_QUOTED_KINDS.contains(&kind) { + true => UNALIASED_QUOTED_NAME.to_owned(), + false => UNALIASED_TEXT_NAME.to_owned(), + }, ) }), } diff --git a/crates/sqlquack-convert/src/snowflake/conversion/constants.rs b/crates/sqlquack-convert/src/snowflake/conversion/constants.rs index 2210b477b..1fdb55c7c 100644 --- a/crates/sqlquack-convert/src/snowflake/conversion/constants.rs +++ b/crates/sqlquack-convert/src/snowflake/conversion/constants.rs @@ -1384,3 +1384,6 @@ pub(crate) const CORRELATED_DERIVED: &str = pub(crate) const AMBIGUOUS_SUFFIX: &str = "is ambiguous"; /// A JSON-held VARIANT as a sort key, in ARRAY_SORT's recorded order of kinds. pub const VARIANT_ORDER_KEY: &str = "rule.variant_order_key"; + +/// The select item kinds whose Snowflake name keeps quotes or a path: literals and paths into semi-structured values. +pub(crate) const UNALIASED_QUOTED_KINDS: &[&str] = &["j_s_o_n_extract", "literal", "subscript"]; diff --git a/crates/sqlquack-diagnostics/src/diagnostics/constants.rs b/crates/sqlquack-diagnostics/src/diagnostics/constants.rs index bdbdaf0c2..7706cf72d 100644 --- a/crates/sqlquack-diagnostics/src/diagnostics/constants.rs +++ b/crates/sqlquack-diagnostics/src/diagnostics/constants.rs @@ -12,6 +12,13 @@ pub const CASE_ONLY_MATCH_HELP: &str = "Snowflake folds unquoted names to upper quote the name in the SQL to keep its case"; /// The words of an unresolved name's reason when two result columns share a name. pub const REPEATED_RESULT_NAME: &str = "repeats an earlier result column's name"; +/// The label of an unaliased item Snowflake names after its text. +pub const UNALIASED_TEXT_NAME: &str = "Snowflake names this column after its text"; +/// The label of an unaliased literal or path, whose Snowflake name keeps its quotes or path, so no alias is suggested. +pub const UNALIASED_QUOTED_NAME: &str = + "Snowflake names this column after its full text, quotes and path included"; +/// The help for an unaliased literal or path. +pub const UNALIASED_QUOTED_NAME_HELP: &str = "add an alias, which names the column in both engines"; /// The help for such a name, replacing SQ0004's own. pub const REPEATED_RESULT_NAME_HELP: &str = "give each result column its own alias, such as `SELECT 1 AS first, 1 AS second`"; diff --git a/crates/sqlquack-diagnostics/src/diagnostics/main/diagnose.rs b/crates/sqlquack-diagnostics/src/diagnostics/main/diagnose.rs index 8f3083a64..bbb682f2f 100644 --- a/crates/sqlquack-diagnostics/src/diagnostics/main/diagnose.rs +++ b/crates/sqlquack-diagnostics/src/diagnostics/main/diagnose.rs @@ -10,8 +10,8 @@ use crate::diagnostics::constants::{ CASE_ONLY_MATCH, CASE_ONLY_MATCH_HELP, CONVERSION_BUDGET, DIVERGENCE_REFUSED, FUNCTION_OPERAND_HELP, GUARD_PREFIX, JINJA_HELP, MISSING_RULE, NOT_EXPRESSIBLE, PARSE_FIDELITY, REPEATED_RESULT_NAME, REPEATED_RESULT_NAME_HELP, SUGGESTED_NAME_MAX_CHARS, TYPE_CONDITION, - UNALIASED_OUTPUT_NAME, UNPARSED_PREFIX, UNRESOLVED_NAME, ZONED_LOCAL_TIME, - ZONED_LOCAL_TIME_HELP, ZONED_LOCAL_TIME_NOTE, + UNALIASED_OUTPUT_NAME, UNALIASED_QUOTED_NAME, UNALIASED_QUOTED_NAME_HELP, UNPARSED_PREFIX, + UNRESOLVED_NAME, ZONED_LOCAL_TIME, ZONED_LOCAL_TIME_HELP, ZONED_LOCAL_TIME_NOTE, }; use crate::diagnostics::models::Diagnostic; use crate::refusals::models::{Refusal, RefusalCategory}; @@ -52,7 +52,9 @@ pub fn diagnose(refusals: &[Refusal], source: &str) -> Vec { { diagnostic.help = Some(FUNCTION_OPERAND_HELP.to_owned()); } - if code == UNALIASED_OUTPUT_NAME + if code == UNALIASED_OUTPUT_NAME && refusal.reason.contains(UNALIASED_QUOTED_NAME) { + diagnostic.help = Some(UNALIASED_QUOTED_NAME_HELP.to_owned()); + } else if code == UNALIASED_OUTPUT_NAME && let Some(name) = diagnostic .span .and_then(|span| suggested_name(source, span.start, span.end)) diff --git a/crates/sqlquack/tests/cli/test_convert.rs b/crates/sqlquack/tests/cli/test_convert.rs index c2839610a..a0b9e3582 100644 --- a/crates/sqlquack/tests/cli/test_convert.rs +++ b/crates/sqlquack/tests/cli/test_convert.rs @@ -1566,7 +1566,7 @@ fn given_refused_sql_when_converting_then_text_and_json_carry_the_same_coded_dia expected_text: concat!( "error[SQ0009]: This column needs an alias, because SQLQuack can't be sure of the name ", "Snowflake gives it\n --> :1:18\n |\n1 | SELECT 'a' AS a, UPPER('é')\n", - " | ^^^^^^^^^^ Snowflake names this `function` column after its text\n", + " | ^^^^^^^^^^ Snowflake names this column after its text\n", " |\n = help: add an alias, such as `AS total`, which names the column in both engines\n", "sqlquack convert: refused, 1 refusal\n", ), diff --git a/crates/sqlquack/tests/cli/test_output_names.rs b/crates/sqlquack/tests/cli/test_output_names.rs index c34eb74f3..d207b7da3 100644 --- a/crates/sqlquack/tests/cli/test_output_names.rs +++ b/crates/sqlquack/tests/cli/test_output_names.rs @@ -489,8 +489,8 @@ fn given_a_refused_result_name_when_converting_then_the_diagnostic_underlines_th expected_text: concat!( "error[SQ0009]: This column needs an alias, because SQLQuack can't be sure of the name ", "Snowflake gives it\n --> :1:18\n |\n1 | SELECT order_id, 'it''s' FROM orders\n", - " | ^^^^^^^ Snowflake names this `literal` column after its text\n", - " |\n = help: add an alias, such as `AS \"'IT''S'\"`, which names the column in both engines\n", + " | ^^^^^^^ Snowflake names this column after its full text, quotes and ", + "path included\n |\n = help: add an alias, which names the column in both engines\n", "sqlquack convert: refused, 1 refusal\n", ), expected_span: ("SQ0009", 17, 24), From cd99cfd1b5404c3975c3548d1c7c02e3e2df1bac Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 11 Nov 2026 19:36:04 +0000 Subject: [PATCH 4/4] test(syntax): keep the parse error position tests in their own file test_fixes.rs grew past the 2,000-line limit once main's tests joined it; the tests that check where a parse error points and what it says move to test_parse_errors.rs. --- .../sqlquack-syntax-snowflake/tests/parser.rs | 2 + .../tests/parser/test_fixes.rs | 108 +----------------- .../tests/parser/test_parse_errors.rs | 106 +++++++++++++++++ 3 files changed, 110 insertions(+), 106 deletions(-) create mode 100644 crates/sqlquack-syntax-snowflake/tests/parser/test_parse_errors.rs diff --git a/crates/sqlquack-syntax-snowflake/tests/parser.rs b/crates/sqlquack-syntax-snowflake/tests/parser.rs index 8bdf79284..a118c77b0 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser.rs @@ -8,6 +8,8 @@ mod test_fixes; mod test_located; #[path = "parser/test_lossless.rs"] mod test_lossless; +#[path = "parser/test_parse_errors.rs"] +mod test_parse_errors; #[path = "parser/test_scripting.rs"] mod test_scripting; #[path = "parser/test_snowflake_alter.rs"] diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs index 713223b1e..b2559da73 100644 --- a/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_fixes.rs @@ -1,12 +1,10 @@ use serde_json::json; -use sqlquack_syntax::{CallMode, Dialect, ParseOptions}; -use sqlquack_syntax_snowflake::parse_statements_with; +use sqlquack_syntax::{CallMode, Dialect}; use crate::helpers::{accepted, cast_targets, projection_at, statement_at, statements}; use crate::test_types::{ - AcceptanceTestCase, CastTargetTestCase, ParseErrorTestCase, ParseMessageTestCase, - ProjectionTestCase, StatementTestCase, + AcceptanceTestCase, CastTargetTestCase, ProjectionTestCase, StatementTestCase, }; #[test] @@ -1952,105 +1950,3 @@ fn given_a_keyword_lambda_parameter_when_parsing_duckdb_then_it_names_the_parame ); } } - -#[test] -fn given_group_or_order_by_after_an_operator_when_parsing_snowflake_then_the_error_is_at_the_clause( -) { - let test_cases = [ - ParseErrorTestCase { - description: "AND before GROUP BY on the next line fails at GROUP, not at BY", - sql: "SELECT customer_id, SUM(total) AS revenue\nFROM orders\nWHERE status = 'shipped' AND\nGROUP BY customer_id", - expected_error_at: Some("GROUP"), - }, - ParseErrorTestCase { - description: "AND before ORDER BY on the same line fails at ORDER", - sql: "SELECT order_id FROM orders WHERE total > 1 AND ORDER BY order_id", - expected_error_at: Some("ORDER"), - }, - ParseErrorTestCase { - description: "NOT before GROUP BY fails at GROUP", - sql: "SELECT status FROM orders WHERE NOT\ngroup by status", - expected_error_at: Some("group"), - }, - ParseErrorTestCase { - description: "GROUP BY and ORDER BY after WHERE still parse", - sql: "SELECT status FROM orders WHERE total > 1\nGROUP BY status\nORDER BY status", - expected_error_at: None, - }, - ParseErrorTestCase { - description: "a quoted GROUP column still parses", - sql: "SELECT \"GROUP\" FROM orders ORDER BY 1", - expected_error_at: None, - }, - ]; - - for test_case in test_cases { - let error_at = parse_statements_with( - test_case.sql, - Dialect::Snowflake, - &ParseOptions::default(), - CallMode::Faithful, - ) - .err() - .map(|error| { - let start = error.start().unwrap_or_default(); - let end = error.end().unwrap_or_default(); - test_case.sql.get(start..end).unwrap_or_default().to_owned() - }); - assert_eq!( - error_at.as_deref(), - test_case.expected_error_at, - "{}", - test_case.description - ); - } -} - -#[test] -fn given_a_statement_ending_where_an_expression_belongs_when_parsing_snowflake_then_the_error_names_what_is_missing( -) { - let test_cases = [ - ParseMessageTestCase { - description: "an empty WHERE asks for a condition", - sql: "SELECT * FROM orders WHERE", - expected_message: "Expected a condition after WHERE", - }, - ParseMessageTestCase { - description: "an empty WHERE before a semicolon asks for a condition", - sql: "SELECT * FROM orders WHERE ;", - expected_message: "Expected a condition after WHERE", - }, - ParseMessageTestCase { - description: "an empty HAVING asks for a condition", - sql: "SELECT status FROM orders GROUP BY status HAVING", - expected_message: "Expected a condition after HAVING", - }, - ParseMessageTestCase { - description: "a join's empty ON asks for a condition", - sql: "SELECT 1 FROM orders o JOIN customers c ON", - expected_message: "Expected a condition after ON", - }, - ParseMessageTestCase { - description: "a comparison with no right side asks for an expression", - sql: "SELECT 1 FROM orders WHERE total >", - expected_message: "Expected an expression after >", - }, - ]; - - for test_case in test_cases { - let message = parse_statements_with( - test_case.sql, - Dialect::Snowflake, - &ParseOptions::default(), - CallMode::Faithful, - ) - .err() - .map(|error| error.to_string()) - .unwrap_or_default(); - assert!( - message.contains(test_case.expected_message), - "{}: {message}", - test_case.description - ); - } -} diff --git a/crates/sqlquack-syntax-snowflake/tests/parser/test_parse_errors.rs b/crates/sqlquack-syntax-snowflake/tests/parser/test_parse_errors.rs new file mode 100644 index 000000000..c36132424 --- /dev/null +++ b/crates/sqlquack-syntax-snowflake/tests/parser/test_parse_errors.rs @@ -0,0 +1,106 @@ +use sqlquack_syntax::{CallMode, Dialect, ParseOptions}; +use sqlquack_syntax_snowflake::parse_statements_with; + +use crate::test_types::{ParseErrorTestCase, ParseMessageTestCase}; + +#[test] +fn given_group_or_order_by_after_an_operator_when_parsing_snowflake_then_the_error_is_at_the_clause( +) { + let test_cases = [ + ParseErrorTestCase { + description: "AND before GROUP BY on the next line fails at GROUP, not at BY", + sql: "SELECT customer_id, SUM(total) AS revenue\nFROM orders\nWHERE status = 'shipped' AND\nGROUP BY customer_id", + expected_error_at: Some("GROUP"), + }, + ParseErrorTestCase { + description: "AND before ORDER BY on the same line fails at ORDER", + sql: "SELECT order_id FROM orders WHERE total > 1 AND ORDER BY order_id", + expected_error_at: Some("ORDER"), + }, + ParseErrorTestCase { + description: "NOT before GROUP BY fails at GROUP", + sql: "SELECT status FROM orders WHERE NOT\ngroup by status", + expected_error_at: Some("group"), + }, + ParseErrorTestCase { + description: "GROUP BY and ORDER BY after WHERE still parse", + sql: "SELECT status FROM orders WHERE total > 1\nGROUP BY status\nORDER BY status", + expected_error_at: None, + }, + ParseErrorTestCase { + description: "a quoted GROUP column still parses", + sql: "SELECT \"GROUP\" FROM orders ORDER BY 1", + expected_error_at: None, + }, + ]; + + for test_case in test_cases { + let error_at = parse_statements_with( + test_case.sql, + Dialect::Snowflake, + &ParseOptions::default(), + CallMode::Faithful, + ) + .err() + .map(|error| { + let start = error.start().unwrap_or_default(); + let end = error.end().unwrap_or_default(); + test_case.sql.get(start..end).unwrap_or_default().to_owned() + }); + assert_eq!( + error_at.as_deref(), + test_case.expected_error_at, + "{}", + test_case.description + ); + } +} + +#[test] +fn given_a_statement_ending_where_an_expression_belongs_when_parsing_snowflake_then_the_error_names_what_is_missing( +) { + let test_cases = [ + ParseMessageTestCase { + description: "an empty WHERE asks for a condition", + sql: "SELECT * FROM orders WHERE", + expected_message: "Expected a condition after WHERE", + }, + ParseMessageTestCase { + description: "an empty WHERE before a semicolon asks for a condition", + sql: "SELECT * FROM orders WHERE ;", + expected_message: "Expected a condition after WHERE", + }, + ParseMessageTestCase { + description: "an empty HAVING asks for a condition", + sql: "SELECT status FROM orders GROUP BY status HAVING", + expected_message: "Expected a condition after HAVING", + }, + ParseMessageTestCase { + description: "a join's empty ON asks for a condition", + sql: "SELECT 1 FROM orders o JOIN customers c ON", + expected_message: "Expected a condition after ON", + }, + ParseMessageTestCase { + description: "a comparison with no right side asks for an expression", + sql: "SELECT 1 FROM orders WHERE total >", + expected_message: "Expected an expression after >", + }, + ]; + + for test_case in test_cases { + let message = parse_statements_with( + test_case.sql, + Dialect::Snowflake, + &ParseOptions::default(), + CallMode::Faithful, + ) + .err() + .map(|error| error.to_string()) + .unwrap_or_default(); + assert!( + message.contains(test_case.expected_message), + "{}: {message}", + test_case.description + ); + } +}