From 6dde14cb3e7fe55f066858a57c48c3708077f157 Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Sat, 28 Sep 2024 11:29:57 -0700 Subject: [PATCH 1/2] Expand handling of `LIMIT 1, 2` handling to include sqlite --- src/dialect/mod.rs | 2 +- src/parser/mod.rs | 2 +- src/test_utils.rs | 35 ++++++++++++++++++++--------------- tests/sqlparser_common.rs | 20 +++++++++++++++----- 4 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index 619b54713..094bce4df 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -60,7 +60,7 @@ use alloc::boxed::Box; /// Convenience check if a [`Parser`] uses a certain dialect. /// -/// `dialect_of!(parser Is SQLiteDialect | GenericDialect)` evaluates +/// `dialect_of!(parser is SQLiteDialect | GenericDialect)` evaluates /// to `true` if `parser.dialect` is one of the [`Dialect`]s specified. macro_rules! dialect_of { ( $parsed_dialect: ident is $($dialect_type: ty)|+ ) => { diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 4c3f8788d..909315d0e 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -8715,7 +8715,7 @@ impl<'a> Parser<'a> { offset = Some(self.parse_offset()?) } - if dialect_of!(self is GenericDialect | MySqlDialect | ClickHouseDialect) + if dialect_of!(self is GenericDialect | MySqlDialect | ClickHouseDialect | SQLiteDialect) && limit.is_some() && offset.is_none() && self.consume_token(&Token::Comma) diff --git a/src/test_utils.rs b/src/test_utils.rs index eb0352353..28eb1730d 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -211,22 +211,27 @@ impl TestedDialects { /// Returns all available dialects. pub fn all_dialects() -> TestedDialects { - let all_dialects = vec![ - Box::new(GenericDialect {}) as Box, - Box::new(PostgreSqlDialect {}) as Box, - Box::new(MsSqlDialect {}) as Box, - Box::new(AnsiDialect {}) as Box, - Box::new(SnowflakeDialect {}) as Box, - Box::new(HiveDialect {}) as Box, - Box::new(RedshiftSqlDialect {}) as Box, - Box::new(MySqlDialect {}) as Box, - Box::new(BigQueryDialect {}) as Box, - Box::new(SQLiteDialect {}) as Box, - Box::new(DuckDbDialect {}) as Box, - Box::new(DatabricksDialect {}) as Box, - ]; + specific_dialects(vec![ + Box::new(GenericDialect {}), + Box::new(PostgreSqlDialect {}), + Box::new(MsSqlDialect {}), + Box::new(AnsiDialect {}), + Box::new(SnowflakeDialect {}), + Box::new(HiveDialect {}), + Box::new(RedshiftSqlDialect {}), + Box::new(MySqlDialect {}), + Box::new(BigQueryDialect {}), + Box::new(SQLiteDialect {}), + Box::new(DuckDbDialect {}), + Box::new(DatabricksDialect {}), + Box::new(ClickHouseDialect {}), + ]) +} + +/// Returns a specific set of dialects. +pub fn specific_dialects(dialects: Vec>) -> TestedDialects { TestedDialects { - dialects: all_dialects, + dialects, options: None, } } diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 711070034..480b99543 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -39,7 +39,7 @@ use sqlparser::parser::{Parser, ParserError, ParserOptions}; use sqlparser::tokenizer::Tokenizer; use test_utils::{ all_dialects, all_dialects_where, alter_table_op, assert_eq_vec, call, expr_from_projection, - join, number, only, table, table_alias, TestedDialects, + join, number, only, specific_dialects, table, table_alias, TestedDialects, }; #[macro_use] @@ -6795,7 +6795,8 @@ fn parse_create_view_with_options() { #[test] fn parse_create_view_with_columns() { let sql = "CREATE VIEW v (has, cols) AS SELECT 1, 2"; - match verified_stmt(sql) { + // TODO: why does this fail for ClickHouseDialect? + match all_dialects_except(|d| d.is::()).verified_stmt(sql) { Statement::CreateView { name, columns, @@ -8587,17 +8588,26 @@ fn verified_expr(query: &str) -> Expr { #[test] fn parse_offset_and_limit() { - let sql = "SELECT foo FROM bar LIMIT 2 OFFSET 2"; + let sql = "SELECT foo FROM bar LIMIT 1 OFFSET 2"; let expect = Some(Offset { value: Expr::Value(number("2")), rows: OffsetRows::None, }); let ast = verified_query(sql); assert_eq!(ast.offset, expect); - assert_eq!(ast.limit, Some(Expr::Value(number("2")))); + assert_eq!(ast.limit, Some(Expr::Value(number("1")))); // different order is OK - one_statement_parses_to("SELECT foo FROM bar OFFSET 2 LIMIT 2", sql); + one_statement_parses_to("SELECT foo FROM bar OFFSET 2 LIMIT 1", sql); + + // mysql syntax is ok for some dialects + specific_dialects(vec![ + Box::new(GenericDialect {}), + Box::new(MySqlDialect {}), + Box::new(SQLiteDialect {}), + Box::new(ClickHouseDialect {}), + ]) + .one_statement_parses_to("SELECT foo FROM bar LIMIT 2, 1", sql); // expressions are allowed let sql = "SELECT foo FROM bar LIMIT 1 + 2 OFFSET 3 * 4"; From 983bbfc0894eae997bebefbf04e30a14c7fbdf3d Mon Sep 17 00:00:00 2001 From: Joshua Warner Date: Sun, 29 Sep 2024 14:07:21 -0700 Subject: [PATCH 2/2] Feedback: specific_dialects->TestedDialects::new, add Dialect::supports_limit_comma, add reference to newly discovered ClickHouseDialect issue --- src/dialect/clickhouse.rs | 4 ++++ src/dialect/generic.rs | 4 ++++ src/dialect/mod.rs | 5 +++++ src/dialect/mysql.rs | 4 ++++ src/dialect/sqlite.rs | 4 ++++ src/parser/mod.rs | 2 +- src/test_utils.rs | 18 +++++++++--------- tests/sqlparser_common.rs | 7 ++++--- 8 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/dialect/clickhouse.rs b/src/dialect/clickhouse.rs index ee8ee94d2..0c8f08040 100644 --- a/src/dialect/clickhouse.rs +++ b/src/dialect/clickhouse.rs @@ -46,4 +46,8 @@ impl Dialect for ClickHouseDialect { fn require_interval_qualifier(&self) -> bool { true } + + fn supports_limit_comma(&self) -> bool { + true + } } diff --git a/src/dialect/generic.rs b/src/dialect/generic.rs index bea56d0ef..8bec45b28 100644 --- a/src/dialect/generic.rs +++ b/src/dialect/generic.rs @@ -99,4 +99,8 @@ impl Dialect for GenericDialect { fn supports_explain_with_utility_options(&self) -> bool { true } + + fn supports_limit_comma(&self) -> bool { + true + } } diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index 094bce4df..51274d6ae 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -316,6 +316,11 @@ pub trait Dialect: Debug + Any { false } + /// Does the dialect support parsing `LIMIT 1, 2` as `LIMIT 2 OFFSET 1`? + fn supports_limit_comma(&self) -> bool { + false + } + /// Does the dialect support trailing commas in the projection list? fn supports_projection_trailing_commas(&self) -> bool { self.supports_trailing_commas() diff --git a/src/dialect/mysql.rs b/src/dialect/mysql.rs index 2b5e42e59..d1bf33345 100644 --- a/src/dialect/mysql.rs +++ b/src/dialect/mysql.rs @@ -93,6 +93,10 @@ impl Dialect for MySqlDialect { fn require_interval_qualifier(&self) -> bool { true } + + fn supports_limit_comma(&self) -> bool { + true + } } /// `LOCK TABLES` diff --git a/src/dialect/sqlite.rs b/src/dialect/sqlite.rs index d3813d91c..5c563bf4a 100644 --- a/src/dialect/sqlite.rs +++ b/src/dialect/sqlite.rs @@ -73,4 +73,8 @@ impl Dialect for SQLiteDialect { fn supports_in_empty_list(&self) -> bool { true } + + fn supports_limit_comma(&self) -> bool { + true + } } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 909315d0e..8e9721468 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -8715,7 +8715,7 @@ impl<'a> Parser<'a> { offset = Some(self.parse_offset()?) } - if dialect_of!(self is GenericDialect | MySqlDialect | ClickHouseDialect | SQLiteDialect) + if self.dialect.supports_limit_comma() && limit.is_some() && offset.is_none() && self.consume_token(&Token::Comma) diff --git a/src/test_utils.rs b/src/test_utils.rs index 28eb1730d..e588b3506 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -47,6 +47,14 @@ pub struct TestedDialects { } impl TestedDialects { + /// Create a TestedDialects with default options and the given dialects. + pub fn new(dialects: Vec>) -> Self { + Self { + dialects, + options: None, + } + } + fn new_parser<'a>(&self, dialect: &'a dyn Dialect) -> Parser<'a> { let parser = Parser::new(dialect); if let Some(options) = &self.options { @@ -211,7 +219,7 @@ impl TestedDialects { /// Returns all available dialects. pub fn all_dialects() -> TestedDialects { - specific_dialects(vec![ + TestedDialects::new(vec![ Box::new(GenericDialect {}), Box::new(PostgreSqlDialect {}), Box::new(MsSqlDialect {}), @@ -228,14 +236,6 @@ pub fn all_dialects() -> TestedDialects { ]) } -/// Returns a specific set of dialects. -pub fn specific_dialects(dialects: Vec>) -> TestedDialects { - TestedDialects { - dialects, - options: None, - } -} - /// Returns all dialects matching the given predicate. pub fn all_dialects_where(predicate: F) -> TestedDialects where diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 480b99543..87b09e8f3 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -39,7 +39,7 @@ use sqlparser::parser::{Parser, ParserError, ParserOptions}; use sqlparser::tokenizer::Tokenizer; use test_utils::{ all_dialects, all_dialects_where, alter_table_op, assert_eq_vec, call, expr_from_projection, - join, number, only, specific_dialects, table, table_alias, TestedDialects, + join, number, only, table, table_alias, TestedDialects, }; #[macro_use] @@ -6795,7 +6795,8 @@ fn parse_create_view_with_options() { #[test] fn parse_create_view_with_columns() { let sql = "CREATE VIEW v (has, cols) AS SELECT 1, 2"; - // TODO: why does this fail for ClickHouseDialect? + // TODO: why does this fail for ClickHouseDialect? (#1449) + // match all_dialects().verified_stmt(sql) { match all_dialects_except(|d| d.is::()).verified_stmt(sql) { Statement::CreateView { name, @@ -8601,7 +8602,7 @@ fn parse_offset_and_limit() { one_statement_parses_to("SELECT foo FROM bar OFFSET 2 LIMIT 1", sql); // mysql syntax is ok for some dialects - specific_dialects(vec![ + TestedDialects::new(vec![ Box::new(GenericDialect {}), Box::new(MySqlDialect {}), Box::new(SQLiteDialect {}),