Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dialect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

/// to `true` if `parser.dialect` is one of the [`Dialect`]s specified.
macro_rules! dialect_of {
( $parsed_dialect: ident is $($dialect_type: ty)|+ ) => {
Expand Down
2 changes: 1 addition & 1 deletion src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something else we have been discussing is moving from this dialect_of macro to a method on the Dialect trait instead (the current code uses both patterns)

So it would look something like

if self.dialect.supports_limit_comma() ...

The benefits are that user defined Dialects can control the behavior and that there is a better place for documenting the behavior

Maybe something to think about for a future PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me! Super easy, so will include this in the next commit to this PR.

&& limit.is_some()
&& offset.is_none()
&& self.consume_token(&Token::Comma)
Expand Down
35 changes: 20 additions & 15 deletions src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,22 +211,27 @@ impl TestedDialects {

/// Returns all available dialects.
pub fn all_dialects() -> TestedDialects {
let all_dialects = vec![
Box::new(GenericDialect {}) as Box<dyn Dialect>,
Box::new(PostgreSqlDialect {}) as Box<dyn Dialect>,
Box::new(MsSqlDialect {}) as Box<dyn Dialect>,
Box::new(AnsiDialect {}) as Box<dyn Dialect>,
Box::new(SnowflakeDialect {}) as Box<dyn Dialect>,
Box::new(HiveDialect {}) as Box<dyn Dialect>,
Box::new(RedshiftSqlDialect {}) as Box<dyn Dialect>,
Box::new(MySqlDialect {}) as Box<dyn Dialect>,
Box::new(BigQueryDialect {}) as Box<dyn Dialect>,
Box::new(SQLiteDialect {}) as Box<dyn Dialect>,
Box::new(DuckDbDialect {}) as Box<dyn Dialect>,
Box::new(DatabricksDialect {}) as Box<dyn Dialect>,
];
specific_dialects(vec![
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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<Box<dyn Dialect>>) -> TestedDialects {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also potentially be a constructor on TestedDialects

So like

impl TestedDialects {
  pub fn new(dialects: Vec<Box<dyn Dialect>>) -> Self {
    Self { 
     dialects, 
     options: None,
    }
  }
}

Perhaps

TestedDialects {
dialects: all_dialects,
dialects,
options: None,
}
}
Expand Down
20 changes: 15 additions & 5 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you perhaps file a ticket to track this isse?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do!

match all_dialects_except(|d| d.is::<ClickHouseDialect>()).verified_stmt(sql) {
Statement::CreateView {
name,
columns,
Expand Down Expand Up @@ -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";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 that is a good change

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";
Expand Down
Loading