From f70d90a6410d66540b1acab54df08eede6fe3596 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 23 Aug 2024 14:55:58 +0100 Subject: [PATCH 01/14] refactor "INTERVAL" parsing --- src/dialect/duckdb.rs | 4 + src/dialect/mod.rs | 17 +++++ src/dialect/postgresql.rs | 4 + src/dialect/redshift.rs | 4 + src/dialect/snowflake.rs | 4 + src/parser/mod.rs | 140 ++++++++++++++++------------------- tests/sqlparser_bigquery.rs | 18 ++--- tests/sqlparser_common.rs | 141 +++++++++++++++++++++++++----------- tests/sqlparser_postgres.rs | 5 +- 9 files changed, 207 insertions(+), 130 deletions(-) diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs index 1fc211685..e43f407cf 100644 --- a/src/dialect/duckdb.rs +++ b/src/dialect/duckdb.rs @@ -55,4 +55,8 @@ impl Dialect for DuckDbDialect { fn support_map_literal_syntax(&self) -> bool { true } + + fn require_interval_units(&self) -> bool { + false + } } diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index df3022adc..fd95f63e4 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -499,6 +499,23 @@ pub trait Dialect: Debug + Any { fn describe_requires_table_keyword(&self) -> bool { false } + + /// Whether or not units are required with interval expressions. + /// + /// When `true`: + /// * `INTERVAL '1' DAY` is VALID + /// * `INTERVAL 1 + 1 DAY` is VALID + /// * `INTERVAL '1' + '1' DAY` is VALID + /// * `INTERVAL '1'` is INVALID + /// + /// When `false`: + /// * `INTERVAL '1' DAY` is VALID + /// * `INTERVAL '1'` is VALID + /// * `INTERVAL '1 second'` is VALID + /// * `INTERVAL 1 + 1 DAY` is INVALID + fn require_interval_units(&self) -> bool { + true + } } /// This represents the operators for which precedence must be defined diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs index c25a80f67..4a3eeaf33 100644 --- a/src/dialect/postgresql.rs +++ b/src/dialect/postgresql.rs @@ -154,6 +154,10 @@ impl Dialect for PostgreSqlDialect { Precedence::Or => OR_PREC, } } + + fn require_interval_units(&self) -> bool { + false + } } pub fn parse_comment(parser: &mut Parser) -> Result { diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs index bd4dc817c..b6cce26e0 100644 --- a/src/dialect/redshift.rs +++ b/src/dialect/redshift.rs @@ -63,4 +63,8 @@ impl Dialect for RedshiftSqlDialect { fn supports_connect_by(&self) -> bool { true } + + fn require_interval_units(&self) -> bool { + false + } } diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs index 89508c3bb..cd70c2116 100644 --- a/src/dialect/snowflake.rs +++ b/src/dialect/snowflake.rs @@ -158,6 +158,10 @@ impl Dialect for SnowflakeDialect { fn describe_requires_table_keyword(&self) -> bool { true } + + fn require_interval_units(&self) -> bool { + false + } } /// Parse snowflake create table statement. diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 1eff8f7d5..e4364b130 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -56,15 +56,6 @@ macro_rules! parser_err { }; } -// Returns a successful result if the optional expression is some -macro_rules! return_ok_if_some { - ($e:expr) => {{ - if let Some(v) = $e { - return Ok(v); - } - }}; -} - #[cfg(feature = "std")] /// Implementation [`RecursionCounter`] if std is available mod recursion { @@ -896,33 +887,61 @@ impl<'a> Parser<'a> { Ok(expr) } - pub fn parse_interval_expr(&mut self) -> Result { - let precedence = self.dialect.prec_unknown(); + fn parse_interval_expr(&mut self) -> Result<(Expr, bool), ParserError> { let mut expr = self.parse_prefix()?; - loop { - let next_precedence = self.get_next_interval_precedence()?; - - if precedence >= next_precedence { - break; - } - - expr = self.parse_infix(expr, next_precedence)?; - } - - Ok(expr) - } - - /// Get the precedence of the next token, with AND, OR, and XOR. - pub fn get_next_interval_precedence(&self) -> Result { - let token = self.peek_token(); - - match token.token { - Token::Word(w) if w.keyword == Keyword::AND => Ok(self.dialect.prec_unknown()), - Token::Word(w) if w.keyword == Keyword::OR => Ok(self.dialect.prec_unknown()), - Token::Word(w) if w.keyword == Keyword::XOR => Ok(self.dialect.prec_unknown()), - _ => self.get_next_precedence(), - } + if self.dialect.require_interval_units() { + // if require_interval_units is true, continue parsing expressions until a unit is foudn + loop { + if self.next_token_is_unit() { + return Ok((expr, true)); + } else { + expr = self.parse_infix(expr, self.dialect.prec_unknown())?; + } + } + } else { + // otherwise, check if the next token is a unit, but don't iterate + Ok((expr, self.next_token_is_unit())) + } + } + + pub fn next_token_is_unit(&mut self) -> bool { + let token_loc = self.peek_token(); + if let Token::Word(word) = token_loc.token { + if matches!( + word.keyword, + Keyword::YEAR + | Keyword::MONTH + | Keyword::WEEK + | Keyword::DAY + | Keyword::HOUR + | Keyword::MINUTE + | Keyword::SECOND + | Keyword::CENTURY + | Keyword::DECADE + | Keyword::DOW + | Keyword::DOY + | Keyword::EPOCH + | Keyword::ISODOW + | Keyword::ISOYEAR + | Keyword::JULIAN + | Keyword::MICROSECOND + | Keyword::MICROSECONDS + | Keyword::MILLENIUM + | Keyword::MILLENNIUM + | Keyword::MILLISECOND + | Keyword::MILLISECONDS + | Keyword::NANOSECOND + | Keyword::NANOSECONDS + | Keyword::QUARTER + | Keyword::TIMEZONE + | Keyword::TIMEZONE_HOUR + | Keyword::TIMEZONE_MINUTE + ) { + return true; + } + } + return false; } pub fn parse_assert(&mut self) -> Result { @@ -972,7 +991,7 @@ impl<'a> Parser<'a> { // name is not followed by a string literal, but in fact in PostgreSQL it is a valid // expression that should parse as the column name "date". let loc = self.peek_token().location; - return_ok_if_some!(self.maybe_parse(|parser| { + let opt_expr = self.maybe_parse(|parser| { match parser.parse_data_type()? { DataType::Interval => parser.parse_interval(), // PostgreSQL allows almost any identifier to be used as custom data type name, @@ -988,7 +1007,11 @@ impl<'a> Parser<'a> { value: parser.parse_literal_string()?, }), } - })); + }); + + if let Some(expr) = opt_expr { + return Ok(expr); + } let next_token = self.next_token(); let expr = match next_token.token { @@ -2079,52 +2102,17 @@ impl<'a> Parser<'a> { // don't currently try to parse it. (The sign can instead be included // inside the value string.) - // The first token in an interval is a string literal which specifies - // the duration of the interval. - let value = self.parse_interval_expr()?; + let (value, has_units) = self.parse_interval_expr()?; // Following the string literal is a qualifier which indicates the units // of the duration specified in the string literal. // // Note that PostgreSQL allows omitting the qualifier, so we provide // this more general implementation. - let leading_field = match self.peek_token().token { - Token::Word(kw) - if [ - Keyword::YEAR, - Keyword::MONTH, - Keyword::WEEK, - Keyword::DAY, - Keyword::HOUR, - Keyword::MINUTE, - Keyword::SECOND, - Keyword::CENTURY, - Keyword::DECADE, - Keyword::DOW, - Keyword::DOY, - Keyword::EPOCH, - Keyword::ISODOW, - Keyword::ISOYEAR, - Keyword::JULIAN, - Keyword::MICROSECOND, - Keyword::MICROSECONDS, - Keyword::MILLENIUM, - Keyword::MILLENNIUM, - Keyword::MILLISECOND, - Keyword::MILLISECONDS, - Keyword::NANOSECOND, - Keyword::NANOSECONDS, - Keyword::QUARTER, - Keyword::TIMEZONE, - Keyword::TIMEZONE_HOUR, - Keyword::TIMEZONE_MINUTE, - ] - .iter() - .any(|d| kw.keyword == *d) => - { - Some(self.parse_date_time_field()?) - } - _ => None, + let leading_field = if has_units { + Some(self.parse_date_time_field()?) + } else { + None }; let (leading_precision, last_field, fsec_precision) = diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index 57cf9d7fd..ac71c482f 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -830,16 +830,14 @@ fn parse_typed_struct_syntax_bigquery() { expr_from_projection(&select.projection[3]) ); - let sql = r#"SELECT STRUCT(INTERVAL '1-2 3 4:5:6.789999'), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; + let sql = r#"SELECT STRUCT(INTERVAL '1' DAY), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; let select = bigquery().verified_only_select(sql); assert_eq!(2, select.projection.len()); assert_eq!( &Expr::Struct { values: vec![Expr::Interval(ast::Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString( - "1-2 3 4:5:6.789999".to_string() - ))), - leading_field: None, + value: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))), + leading_field: Some(DateTimeField::Day), leading_precision: None, last_field: None, fractional_seconds_precision: None @@ -1141,16 +1139,14 @@ fn parse_typed_struct_syntax_bigquery_and_generic() { expr_from_projection(&select.projection[3]) ); - let sql = r#"SELECT STRUCT(INTERVAL '1-2 3 4:5:6.789999'), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; + let sql = r#"SELECT STRUCT(INTERVAL '2' MONTH), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; let select = bigquery_and_generic().verified_only_select(sql); assert_eq!(2, select.projection.len()); assert_eq!( &Expr::Struct { - values: vec![Expr::Interval(ast::Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString( - "1-2 3 4:5:6.789999".to_string() - ))), - leading_field: None, + values: vec![Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString("2".to_string()))), + leading_field: Some(DateTimeField::Month), leading_precision: None, last_field: None, fractional_seconds_precision: None diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 517e978b4..111e79b01 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4464,9 +4464,9 @@ fn parse_window_functions() { avg(bar) OVER (ORDER BY a \ RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING), \ sum(bar) OVER (ORDER BY a \ - RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1 MONTH' FOLLOWING), \ + RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1' MONTH FOLLOWING), \ COUNT(*) OVER (ORDER BY a \ - RANGE BETWEEN INTERVAL '1 DAY' PRECEDING AND INTERVAL '1 DAY' FOLLOWING), \ + RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1' DAY FOLLOWING), \ max(baz) OVER (ORDER BY a \ ROWS UNBOUNDED PRECEDING), \ sum(qux) OVER (ORDER BY a \ @@ -4886,9 +4886,11 @@ fn parse_literal_timestamp_with_time_zone() { } #[test] -fn parse_interval() { +fn parse_interval_unit_required() { + let dialects_unit_required = all_dialects_where(|d| d.require_interval_units()); + let sql = "SELECT INTERVAL '1-1' YEAR TO MONTH"; - let select = verified_only_select(sql); + let select = dialects_unit_required.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1-1")))), @@ -4901,7 +4903,7 @@ fn parse_interval() { ); let sql = "SELECT INTERVAL '01:01.01' MINUTE (5) TO SECOND (5)"; - let select = verified_only_select(sql); + let select = dialects_unit_required.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( @@ -4916,7 +4918,7 @@ fn parse_interval() { ); let sql = "SELECT INTERVAL '1' SECOND (5, 4)"; - let select = verified_only_select(sql); + let select = dialects_unit_required.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1")))), @@ -4929,7 +4931,7 @@ fn parse_interval() { ); let sql = "SELECT INTERVAL '10' HOUR"; - let select = verified_only_select(sql); + let select = dialects_unit_required.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))), @@ -4942,7 +4944,7 @@ fn parse_interval() { ); let sql = "SELECT INTERVAL 5 DAY"; - let select = verified_only_select(sql); + let select = dialects_unit_required.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(number("5"))), @@ -4955,7 +4957,7 @@ fn parse_interval() { ); let sql = "SELECT INTERVAL 1 + 1 DAY"; - let select = verified_only_select(sql); + let select = dialects_unit_required.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::BinaryOp { @@ -4972,7 +4974,7 @@ fn parse_interval() { ); let sql = "SELECT INTERVAL '10' HOUR (1)"; - let select = verified_only_select(sql); + let select = dialects_unit_required.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))), @@ -4984,8 +4986,39 @@ fn parse_interval() { expr_from_projection(only(&select.projection)), ); + let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND"); + assert_eq!( + ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()), + result.unwrap_err(), + ); + + let result = parse_sql_statements("SELECT INTERVAL '10' HOUR (1) TO HOUR (2)"); + assert_eq!( + ParserError::ParserError("Expected: end of statement, found: (".to_string()), + result.unwrap_err(), + ); + + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' YEAR"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MONTH"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MINUTE"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' SECOND"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO HOUR"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO SECOND"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND"); + dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND"); +} + +#[test] +fn parse_interval_unit_not_required() { + let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); + let sql = "SELECT INTERVAL '1 DAY'"; - let select = verified_only_select(sql); + let select = dialect_no_unit.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( @@ -4999,37 +5032,60 @@ fn parse_interval() { expr_from_projection(only(&select.projection)), ); - let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND"); - assert_eq!( - ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()), - result.unwrap_err(), + dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR'"); + dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR' AS one_year"); + dialect_no_unit.one_statement_parses_to( + "SELECT INTERVAL '1 YEAR' one_year", + "SELECT INTERVAL '1 YEAR' AS one_year", ); +} - let result = parse_sql_statements("SELECT INTERVAL '10' HOUR (1) TO HOUR (2)"); +#[test] +fn interval_unit_not_required_gt() { + let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); + let expr = dialect_no_unit.verified_expr("INTERVAL '1 second' > x"); assert_eq!( - ParserError::ParserError("Expected: end of statement, found: (".to_string()), - result.unwrap_err(), - ); + expr, + Expr::BinaryOp { + left: Box::new(Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString( + "1 second".to_string() + ))), + leading_field: None, + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + },)), + op: BinaryOperator::Gt, + right: Box::new(Expr::Identifier(Ident { + value: "x".to_string(), + quote_style: None, + })), + } + ) +} - verified_only_select("SELECT INTERVAL '1' YEAR"); - verified_only_select("SELECT INTERVAL '1' MONTH"); - verified_only_select("SELECT INTERVAL '1' DAY"); - verified_only_select("SELECT INTERVAL '1' HOUR"); - verified_only_select("SELECT INTERVAL '1' MINUTE"); - verified_only_select("SELECT INTERVAL '1' SECOND"); - verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH"); - verified_only_select("SELECT INTERVAL '1' DAY TO HOUR"); - verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE"); - verified_only_select("SELECT INTERVAL '1' DAY TO SECOND"); - verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE"); - verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND"); - verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND"); - verified_only_select("SELECT INTERVAL '1 YEAR'"); - verified_only_select("SELECT INTERVAL '1 YEAR' AS one_year"); - one_statement_parses_to( - "SELECT INTERVAL '1 YEAR' one_year", - "SELECT INTERVAL '1 YEAR' AS one_year", - ); +#[test] +fn interval_unit_not_required_double_colon() { + let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); + let expr = dialect_no_unit.verified_expr("INTERVAL '1 second'::TEXT"); + assert_eq!( + expr, + Expr::Cast { + kind: CastKind::DoubleColon, + expr: Box::new(Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString( + "1 second".to_string() + ))), + leading_field: None, + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + })), + data_type: DataType::Text, + format: None, + } + ) } #[test] @@ -5038,7 +5094,7 @@ fn parse_interval_and_or_xor() { WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'"; - let actual_ast = Parser::parse_sql(&GenericDialect {}, sql).unwrap(); + let actual_ast = Parser::parse_sql(&PostgreSqlDialect {}, sql).unwrap(); let expected_ast = vec![Statement::Query(Box::new(Query { with: None, @@ -5140,19 +5196,20 @@ fn parse_interval_and_or_xor() { assert_eq!(actual_ast, expected_ast); - verified_stmt( + let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); + dialect_no_unit.verified_stmt( "SELECT col FROM test \ WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'", ); - verified_stmt( + dialect_no_unit.verified_stmt( "SELECT col FROM test \ WHERE d3_date > d1_date + INTERVAL '5 days' \ OR d2_date > d1_date + INTERVAL '3 days'", ); - verified_stmt( + dialect_no_unit.verified_stmt( "SELECT col FROM test \ WHERE d3_date > d1_date + INTERVAL '5 days' \ XOR d2_date > d1_date + INTERVAL '3 days'", diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 2f9fe86c9..4ba338dc0 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -4353,7 +4353,10 @@ fn parse_mat_cte() { fn parse_at_time_zone() { pg_and_generic().verified_expr("CURRENT_TIMESTAMP AT TIME ZONE tz"); pg_and_generic().verified_expr("CURRENT_TIMESTAMP AT TIME ZONE ('America/' || 'Los_Angeles')"); +} +#[test] +fn parse_at_time_zone_interval() { // check precedence let expr = Expr::BinaryOp { left: Box::new(Expr::AtTimeZone { @@ -4382,7 +4385,7 @@ fn parse_at_time_zone() { })), }; pretty_assertions::assert_eq!( - pg_and_generic().verified_expr( + pg().verified_expr( "TIMESTAMP '2001-09-28 01:00' AT TIME ZONE 'America/Los_Angeles'::TEXT + INTERVAL '23 hours'", ), expr From f643d44ca7d810f12e3b763184f48ad69e07e636 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 23 Aug 2024 15:06:36 +0100 Subject: [PATCH 02/14] rearrange Parser code, split out some tests --- src/parser/mod.rs | 121 ++++++++++++++++++++------------------ tests/sqlparser_common.rs | 38 +++++++----- 2 files changed, 87 insertions(+), 72 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index e4364b130..dc5f9f19b 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -887,63 +887,6 @@ impl<'a> Parser<'a> { Ok(expr) } - fn parse_interval_expr(&mut self) -> Result<(Expr, bool), ParserError> { - let mut expr = self.parse_prefix()?; - - if self.dialect.require_interval_units() { - // if require_interval_units is true, continue parsing expressions until a unit is foudn - loop { - if self.next_token_is_unit() { - return Ok((expr, true)); - } else { - expr = self.parse_infix(expr, self.dialect.prec_unknown())?; - } - } - } else { - // otherwise, check if the next token is a unit, but don't iterate - Ok((expr, self.next_token_is_unit())) - } - } - - pub fn next_token_is_unit(&mut self) -> bool { - let token_loc = self.peek_token(); - if let Token::Word(word) = token_loc.token { - if matches!( - word.keyword, - Keyword::YEAR - | Keyword::MONTH - | Keyword::WEEK - | Keyword::DAY - | Keyword::HOUR - | Keyword::MINUTE - | Keyword::SECOND - | Keyword::CENTURY - | Keyword::DECADE - | Keyword::DOW - | Keyword::DOY - | Keyword::EPOCH - | Keyword::ISODOW - | Keyword::ISOYEAR - | Keyword::JULIAN - | Keyword::MICROSECOND - | Keyword::MICROSECONDS - | Keyword::MILLENIUM - | Keyword::MILLENNIUM - | Keyword::MILLISECOND - | Keyword::MILLISECONDS - | Keyword::NANOSECOND - | Keyword::NANOSECONDS - | Keyword::QUARTER - | Keyword::TIMEZONE - | Keyword::TIMEZONE_HOUR - | Keyword::TIMEZONE_MINUTE - ) { - return true; - } - } - return false; - } - pub fn parse_assert(&mut self) -> Result { let condition = self.parse_expr()?; let message = if self.parse_keyword(Keyword::AS) { @@ -2102,7 +2045,11 @@ impl<'a> Parser<'a> { // don't currently try to parse it. (The sign can instead be included // inside the value string.) - let (value, has_units) = self.parse_interval_expr()?; + let (value, has_units) = if self.dialect.require_interval_units() { + self.parse_interval_expr_units_required()? + } else { + self.parse_interval_expr_units_not_require()? + }; // Following the string literal is a qualifier which indicates the units // of the duration specified in the string literal. @@ -2149,6 +2096,64 @@ impl<'a> Parser<'a> { })) } + /// if `require_interval_units` is `true`, continue parsing expressions until a unit is found + pub fn parse_interval_expr_units_required(&mut self) -> Result<(Expr, bool), ParserError> { + let mut expr = self.parse_prefix()?; + + loop { + if self.next_token_is_unit() { + return Ok((expr, true)); + } else { + expr = self.parse_infix(expr, self.dialect.prec_unknown())?; + } + } + } + + /// if `require_interval_units` is `false`, just parse the first expression, but check if the next token is a unit + pub fn parse_interval_expr_units_not_require(&mut self) -> Result<(Expr, bool), ParserError> { + self.parse_prefix() + .map(|expr| (expr, self.next_token_is_unit())) + } + + pub fn next_token_is_unit(&mut self) -> bool { + let token_loc = self.peek_token(); + if let Token::Word(word) = token_loc.token { + if matches!( + word.keyword, + Keyword::YEAR + | Keyword::MONTH + | Keyword::WEEK + | Keyword::DAY + | Keyword::HOUR + | Keyword::MINUTE + | Keyword::SECOND + | Keyword::CENTURY + | Keyword::DECADE + | Keyword::DOW + | Keyword::DOY + | Keyword::EPOCH + | Keyword::ISODOW + | Keyword::ISOYEAR + | Keyword::JULIAN + | Keyword::MICROSECOND + | Keyword::MICROSECONDS + | Keyword::MILLENIUM + | Keyword::MILLENNIUM + | Keyword::MILLISECOND + | Keyword::MILLISECONDS + | Keyword::NANOSECOND + | Keyword::NANOSECONDS + | Keyword::QUARTER + | Keyword::TIMEZONE + | Keyword::TIMEZONE_HOUR + | Keyword::TIMEZONE_MINUTE + ) { + return true; + } + } + return false; + } + /// Bigquery specific: Parse a struct literal /// Syntax /// ```sql diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 111e79b01..9852e05f5 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4885,6 +4885,30 @@ fn parse_literal_timestamp_with_time_zone() { one_statement_parses_to("SELECT TIMESTAMPTZ '1999-01-01 01:23:34Z'", sql); } +#[test] +fn parse_interval_all() { + // these intervals expressions should work with both variants of INTERVAL + verified_only_select("SELECT INTERVAL '1' YEAR"); + verified_only_select("SELECT INTERVAL '1' MONTH"); + verified_only_select("SELECT INTERVAL '1' DAY"); + verified_only_select("SELECT INTERVAL '1' HOUR"); + verified_only_select("SELECT INTERVAL '1' MINUTE"); + verified_only_select("SELECT INTERVAL '1' SECOND"); + verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH"); + verified_only_select("SELECT INTERVAL '1' DAY TO HOUR"); + verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE"); + verified_only_select("SELECT INTERVAL '1' DAY TO SECOND"); + verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE"); + verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND"); + verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND"); + verified_only_select("SELECT INTERVAL 1 YEAR"); + verified_only_select("SELECT INTERVAL 1 MONTH"); + verified_only_select("SELECT INTERVAL 1 DAY"); + verified_only_select("SELECT INTERVAL 1 HOUR"); + verified_only_select("SELECT INTERVAL 1 MINUTE"); + verified_only_select("SELECT INTERVAL 1 SECOND"); +} + #[test] fn parse_interval_unit_required() { let dialects_unit_required = all_dialects_where(|d| d.require_interval_units()); @@ -4997,20 +5021,6 @@ fn parse_interval_unit_required() { ParserError::ParserError("Expected: end of statement, found: (".to_string()), result.unwrap_err(), ); - - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' YEAR"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MONTH"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MINUTE"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' SECOND"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO HOUR"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' DAY TO SECOND"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND"); - dialects_unit_required.verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND"); } #[test] From d61d73e97a163868cce0f90ad806dc14f3cd1a5c Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 23 Aug 2024 15:16:48 +0100 Subject: [PATCH 03/14] cleanup next_token_is_unit --- src/parser/mod.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index dc5f9f19b..75866a29e 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2116,9 +2116,8 @@ impl<'a> Parser<'a> { } pub fn next_token_is_unit(&mut self) -> bool { - let token_loc = self.peek_token(); - if let Token::Word(word) = token_loc.token { - if matches!( + if let Token::Word(word) = self.peek_token().token { + matches!( word.keyword, Keyword::YEAR | Keyword::MONTH @@ -2147,11 +2146,10 @@ impl<'a> Parser<'a> { | Keyword::TIMEZONE | Keyword::TIMEZONE_HOUR | Keyword::TIMEZONE_MINUTE - ) { - return true; - } + ) + } else { + false } - return false; } /// Bigquery specific: Parse a struct literal From 83e93982381147a5abc79348480c6c758371da26 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 23 Aug 2024 15:29:45 +0100 Subject: [PATCH 04/14] more comments and improve tests --- src/parser/mod.rs | 8 +++ tests/sqlparser_common.rs | 121 ++++++++++++++++++++++++-------------- 2 files changed, 84 insertions(+), 45 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 75866a29e..0e21eebab 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2097,6 +2097,10 @@ impl<'a> Parser<'a> { } /// if `require_interval_units` is `true`, continue parsing expressions until a unit is found + /// + /// # Returns + /// + /// A tuple of (interval expression, whether a unit is found) pub fn parse_interval_expr_units_required(&mut self) -> Result<(Expr, bool), ParserError> { let mut expr = self.parse_prefix()?; @@ -2110,6 +2114,10 @@ impl<'a> Parser<'a> { } /// if `require_interval_units` is `false`, just parse the first expression, but check if the next token is a unit + /// + /// # Returns + /// + /// A tuple of (interval expression, whether a unit is found) pub fn parse_interval_expr_units_not_require(&mut self) -> Result<(Expr, bool), ParserError> { self.parse_prefix() .map(|expr| (expr, self.next_token_is_unit())) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 9852e05f5..a75f9dcb0 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4887,34 +4887,23 @@ fn parse_literal_timestamp_with_time_zone() { #[test] fn parse_interval_all() { - // these intervals expressions should work with both variants of INTERVAL - verified_only_select("SELECT INTERVAL '1' YEAR"); - verified_only_select("SELECT INTERVAL '1' MONTH"); - verified_only_select("SELECT INTERVAL '1' DAY"); - verified_only_select("SELECT INTERVAL '1' HOUR"); - verified_only_select("SELECT INTERVAL '1' MINUTE"); - verified_only_select("SELECT INTERVAL '1' SECOND"); - verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH"); - verified_only_select("SELECT INTERVAL '1' DAY TO HOUR"); - verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE"); - verified_only_select("SELECT INTERVAL '1' DAY TO SECOND"); - verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE"); - verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND"); - verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND"); - verified_only_select("SELECT INTERVAL 1 YEAR"); - verified_only_select("SELECT INTERVAL 1 MONTH"); - verified_only_select("SELECT INTERVAL 1 DAY"); - verified_only_select("SELECT INTERVAL 1 HOUR"); - verified_only_select("SELECT INTERVAL 1 MINUTE"); - verified_only_select("SELECT INTERVAL 1 SECOND"); -} + // these intervals expressions all work with both variants of INTERVAL -#[test] -fn parse_interval_unit_required() { - let dialects_unit_required = all_dialects_where(|d| d.require_interval_units()); + let sql = "SELECT INTERVAL '1-1' YEAR TO MONTH"; + let select = verified_only_select(sql); + assert_eq!( + &Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1-1")))), + leading_field: Some(DateTimeField::Year), + leading_precision: None, + last_field: Some(DateTimeField::Month), + fractional_seconds_precision: None, + }), + expr_from_projection(only(&select.projection)), + ); let sql = "SELECT INTERVAL '1-1' YEAR TO MONTH"; - let select = dialects_unit_required.verified_only_select(sql); + let select = verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1-1")))), @@ -4927,7 +4916,7 @@ fn parse_interval_unit_required() { ); let sql = "SELECT INTERVAL '01:01.01' MINUTE (5) TO SECOND (5)"; - let select = dialects_unit_required.verified_only_select(sql); + let select = verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( @@ -4942,7 +4931,7 @@ fn parse_interval_unit_required() { ); let sql = "SELECT INTERVAL '1' SECOND (5, 4)"; - let select = dialects_unit_required.verified_only_select(sql); + let select = verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1")))), @@ -4955,7 +4944,7 @@ fn parse_interval_unit_required() { ); let sql = "SELECT INTERVAL '10' HOUR"; - let select = dialects_unit_required.verified_only_select(sql); + let select = verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))), @@ -4968,7 +4957,7 @@ fn parse_interval_unit_required() { ); let sql = "SELECT INTERVAL 5 DAY"; - let select = dialects_unit_required.verified_only_select(sql); + let select = verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { value: Box::new(Expr::Value(number("5"))), @@ -4980,6 +4969,56 @@ fn parse_interval_unit_required() { expr_from_projection(only(&select.projection)), ); + let sql = "SELECT INTERVAL '10' HOUR (1)"; + let select = verified_only_select(sql); + assert_eq!( + &Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))), + leading_field: Some(DateTimeField::Hour), + leading_precision: Some(1), + last_field: None, + fractional_seconds_precision: None, + }), + expr_from_projection(only(&select.projection)), + ); + + let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND"); + assert_eq!( + ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()), + result.unwrap_err(), + ); + + let result = parse_sql_statements("SELECT INTERVAL '10' HOUR (1) TO HOUR (2)"); + assert_eq!( + ParserError::ParserError("Expected: end of statement, found: (".to_string()), + result.unwrap_err(), + ); + + verified_only_select("SELECT INTERVAL '1' YEAR"); + verified_only_select("SELECT INTERVAL '1' MONTH"); + verified_only_select("SELECT INTERVAL '1' DAY"); + verified_only_select("SELECT INTERVAL '1' HOUR"); + verified_only_select("SELECT INTERVAL '1' MINUTE"); + verified_only_select("SELECT INTERVAL '1' SECOND"); + verified_only_select("SELECT INTERVAL '1' YEAR TO MONTH"); + verified_only_select("SELECT INTERVAL '1' DAY TO HOUR"); + verified_only_select("SELECT INTERVAL '1' DAY TO MINUTE"); + verified_only_select("SELECT INTERVAL '1' DAY TO SECOND"); + verified_only_select("SELECT INTERVAL '1' HOUR TO MINUTE"); + verified_only_select("SELECT INTERVAL '1' HOUR TO SECOND"); + verified_only_select("SELECT INTERVAL '1' MINUTE TO SECOND"); + verified_only_select("SELECT INTERVAL 1 YEAR"); + verified_only_select("SELECT INTERVAL 1 MONTH"); + verified_only_select("SELECT INTERVAL 1 DAY"); + verified_only_select("SELECT INTERVAL 1 HOUR"); + verified_only_select("SELECT INTERVAL 1 MINUTE"); + verified_only_select("SELECT INTERVAL 1 SECOND"); +} + +#[test] +fn parse_interval_unit_required() { + let dialects_unit_required = all_dialects_where(|d| d.require_interval_units()); + let sql = "SELECT INTERVAL 1 + 1 DAY"; let select = dialects_unit_required.verified_only_select(sql); assert_eq!( @@ -4997,30 +5036,22 @@ fn parse_interval_unit_required() { expr_from_projection(only(&select.projection)), ); - let sql = "SELECT INTERVAL '10' HOUR (1)"; + let sql = "SELECT INTERVAL '1' + '1' DAY"; let select = dialects_unit_required.verified_only_select(sql); assert_eq!( &Expr::Interval(Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("10")))), - leading_field: Some(DateTimeField::Hour), - leading_precision: Some(1), + value: Box::new(Expr::BinaryOp { + left: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))), + }), + leading_field: Some(DateTimeField::Day), + leading_precision: None, last_field: None, fractional_seconds_precision: None, }), expr_from_projection(only(&select.projection)), ); - - let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND"); - assert_eq!( - ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()), - result.unwrap_err(), - ); - - let result = parse_sql_statements("SELECT INTERVAL '10' HOUR (1) TO HOUR (2)"); - assert_eq!( - ParserError::ParserError("Expected: end of statement, found: (".to_string()), - result.unwrap_err(), - ); } #[test] From fa8e807d6c0ede5dfc892bc81e514012e951f79f Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Fri, 23 Aug 2024 15:34:04 +0100 Subject: [PATCH 05/14] improve tests, more --- tests/sqlparser_common.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index a75f9dcb0..e4d3391a6 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4902,19 +4902,6 @@ fn parse_interval_all() { expr_from_projection(only(&select.projection)), ); - let sql = "SELECT INTERVAL '1-1' YEAR TO MONTH"; - let select = verified_only_select(sql); - assert_eq!( - &Expr::Interval(Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString(String::from("1-1")))), - leading_field: Some(DateTimeField::Year), - leading_precision: None, - last_field: Some(DateTimeField::Month), - fractional_seconds_precision: None, - }), - expr_from_projection(only(&select.projection)), - ); - let sql = "SELECT INTERVAL '01:01.01' MINUTE (5) TO SECOND (5)"; let select = verified_only_select(sql); assert_eq!( @@ -5135,7 +5122,8 @@ fn parse_interval_and_or_xor() { WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'"; - let actual_ast = Parser::parse_sql(&PostgreSqlDialect {}, sql).unwrap(); + let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); + let actual_ast = dialect_no_unit.parse_sql_statements(sql).unwrap(); let expected_ast = vec![Statement::Query(Box::new(Query { with: None, From 2c14b9599d1df45093eaa8b4d7d654490049ac5d Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Wed, 28 Aug 2024 21:23:32 +0100 Subject: [PATCH 06/14] more backwards compatible and fix binary expressions --- src/dialect/duckdb.rs | 2 +- src/dialect/mod.rs | 5 +- src/dialect/postgresql.rs | 2 +- src/dialect/redshift.rs | 2 +- src/dialect/snowflake.rs | 2 +- src/parser/mod.rs | 38 +++--------- tests/sqlparser_bigquery.rs | 16 +++-- tests/sqlparser_common.rs | 115 +++++++++++++++++++++++++++++++----- 8 files changed, 124 insertions(+), 58 deletions(-) diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs index e43f407cf..316068844 100644 --- a/src/dialect/duckdb.rs +++ b/src/dialect/duckdb.rs @@ -56,7 +56,7 @@ impl Dialect for DuckDbDialect { true } - fn require_interval_units(&self) -> bool { + fn prefer_interval_units(&self) -> bool { false } } diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index b5b532b42..0d232eac0 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -517,13 +517,16 @@ pub trait Dialect: Debug + Any { /// * `INTERVAL 1 + 1 DAY` is VALID /// * `INTERVAL '1' + '1' DAY` is VALID /// * `INTERVAL '1'` is INVALID + /// * `INTERVAL '2' SECOND > INTERVAL '1' SECOND` is interpreted correctly, but + /// `INTERVAL '2' > INTERVAL '1'` is not! /// /// When `false`: /// * `INTERVAL '1' DAY` is VALID /// * `INTERVAL '1'` is VALID /// * `INTERVAL '1 second'` is VALID /// * `INTERVAL 1 + 1 DAY` is INVALID - fn require_interval_units(&self) -> bool { + /// * `INTERVAL '2' > INTERVAL '1'` is interpreted correctly + fn prefer_interval_units(&self) -> bool { true } } diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs index c68f6b792..d663570fb 100644 --- a/src/dialect/postgresql.rs +++ b/src/dialect/postgresql.rs @@ -163,7 +163,7 @@ impl Dialect for PostgreSqlDialect { true } - fn require_interval_units(&self) -> bool { + fn prefer_interval_units(&self) -> bool { false } } diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs index b6cce26e0..d554d6285 100644 --- a/src/dialect/redshift.rs +++ b/src/dialect/redshift.rs @@ -64,7 +64,7 @@ impl Dialect for RedshiftSqlDialect { true } - fn require_interval_units(&self) -> bool { + fn prefer_interval_units(&self) -> bool { false } } diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs index cdb972602..a712fd0f8 100644 --- a/src/dialect/snowflake.rs +++ b/src/dialect/snowflake.rs @@ -167,7 +167,7 @@ impl Dialect for SnowflakeDialect { true } - fn require_interval_units(&self) -> bool { + fn prefer_interval_units(&self) -> bool { false } } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 64b875095..a89858547 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2044,10 +2044,13 @@ impl<'a> Parser<'a> { // don't currently try to parse it. (The sign can instead be included // inside the value string.) - let (value, has_units) = if self.dialect.require_interval_units() { - self.parse_interval_expr_units_required()? + let value = if self.dialect.prefer_interval_units() { + // parse a whole expression so `INTERVAL 1 + 1 DAY` is valid + self.parse_expr()? } else { - self.parse_interval_expr_units_not_require()? + // parse a prefix expression so `INTERVAL 1 DAY` is valid, but `INTERVAL 1 + 1 DAY` is not + // this also means that `INTERVAL '5 days' > INTERVAL '1 day'` treated properly + self.parse_prefix()? }; // Following the string literal is a qualifier which indicates the units @@ -2055,7 +2058,7 @@ impl<'a> Parser<'a> { // // Note that PostgreSQL allows omitting the qualifier, so we provide // this more general implementation. - let leading_field = if has_units { + let leading_field = if self.next_token_is_unit() { Some(self.parse_date_time_field()?) } else { None @@ -2095,33 +2098,6 @@ impl<'a> Parser<'a> { })) } - /// if `require_interval_units` is `true`, continue parsing expressions until a unit is found - /// - /// # Returns - /// - /// A tuple of (interval expression, whether a unit is found) - pub fn parse_interval_expr_units_required(&mut self) -> Result<(Expr, bool), ParserError> { - let mut expr = self.parse_prefix()?; - - loop { - if self.next_token_is_unit() { - return Ok((expr, true)); - } else { - expr = self.parse_infix(expr, self.dialect.prec_unknown())?; - } - } - } - - /// if `require_interval_units` is `false`, just parse the first expression, but check if the next token is a unit - /// - /// # Returns - /// - /// A tuple of (interval expression, whether a unit is found) - pub fn parse_interval_expr_units_not_require(&mut self) -> Result<(Expr, bool), ParserError> { - self.parse_prefix() - .map(|expr| (expr, self.next_token_is_unit())) - } - pub fn next_token_is_unit(&mut self) -> bool { if let Token::Word(word) = self.peek_token().token { matches!( diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index ac71c482f..22d086d8a 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -830,14 +830,16 @@ fn parse_typed_struct_syntax_bigquery() { expr_from_projection(&select.projection[3]) ); - let sql = r#"SELECT STRUCT(INTERVAL '1' DAY), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; + let sql = r#"SELECT STRUCT(INTERVAL '1-2 3 4:5:6.789999'), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; let select = bigquery().verified_only_select(sql); assert_eq!(2, select.projection.len()); assert_eq!( &Expr::Struct { values: vec![Expr::Interval(ast::Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))), - leading_field: Some(DateTimeField::Day), + value: Box::new(Expr::Value(Value::SingleQuotedString( + "1-2 3 4:5:6.789999".to_string() + ))), + leading_field: None, leading_precision: None, last_field: None, fractional_seconds_precision: None @@ -1139,14 +1141,16 @@ fn parse_typed_struct_syntax_bigquery_and_generic() { expr_from_projection(&select.projection[3]) ); - let sql = r#"SELECT STRUCT(INTERVAL '2' MONTH), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; + let sql = r#"SELECT STRUCT(INTERVAL '1-2 3 4:5:6.789999'), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; let select = bigquery_and_generic().verified_only_select(sql); assert_eq!(2, select.projection.len()); assert_eq!( &Expr::Struct { values: vec![Expr::Interval(Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString("2".to_string()))), - leading_field: Some(DateTimeField::Month), + value: Box::new(Expr::Value(Value::SingleQuotedString( + "1-2 3 4:5:6.789999".to_string() + ))), + leading_field: None, leading_precision: None, last_field: None, fractional_seconds_precision: None diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 0a7121dbb..17f4bc6ea 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4464,9 +4464,9 @@ fn parse_window_functions() { avg(bar) OVER (ORDER BY a \ RANGE BETWEEN 1 PRECEDING AND 1 FOLLOWING), \ sum(bar) OVER (ORDER BY a \ - RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1' MONTH FOLLOWING), \ + RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1 MONTH' FOLLOWING), \ COUNT(*) OVER (ORDER BY a \ - RANGE BETWEEN INTERVAL '1' DAY PRECEDING AND INTERVAL '1' DAY FOLLOWING), \ + RANGE BETWEEN INTERVAL '1 DAY' PRECEDING AND INTERVAL '1 DAY' FOLLOWING), \ max(baz) OVER (ORDER BY a \ ROWS UNBOUNDED PRECEDING), \ sum(qux) OVER (ORDER BY a \ @@ -5000,15 +5000,23 @@ fn parse_interval_all() { verified_only_select("SELECT INTERVAL 1 HOUR"); verified_only_select("SELECT INTERVAL 1 MINUTE"); verified_only_select("SELECT INTERVAL 1 SECOND"); + // these are allow with both variants for backwards compatibility + verified_only_select("SELECT INTERVAL '1 YEAR'"); + verified_only_select("SELECT INTERVAL '1 MONTH'"); + verified_only_select("SELECT INTERVAL '1 DAY'"); + verified_only_select("SELECT INTERVAL '1 HOUR'"); + verified_only_select("SELECT INTERVAL '1 MINUTE'"); + verified_only_select("SELECT INTERVAL '1 SECOND'"); } #[test] -fn parse_interval_unit_required() { - let dialects_unit_required = all_dialects_where(|d| d.require_interval_units()); +fn parse_interval_unit_preferred() { + let dialects_unit_preferred = all_dialects_where(|d| d.prefer_interval_units()); let sql = "SELECT INTERVAL 1 + 1 DAY"; - let select = dialects_unit_required.verified_only_select(sql); + let select = dialects_unit_preferred.verified_only_select(sql); assert_eq!( + expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { value: Box::new(Expr::BinaryOp { left: Box::new(Expr::Value(number("1"))), @@ -5020,12 +5028,12 @@ fn parse_interval_unit_required() { last_field: None, fractional_seconds_precision: None, }), - expr_from_projection(only(&select.projection)), ); let sql = "SELECT INTERVAL '1' + '1' DAY"; - let select = dialects_unit_required.verified_only_select(sql); + let select = dialects_unit_preferred.verified_only_select(sql); assert_eq!( + expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { value: Box::new(Expr::BinaryOp { left: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))), @@ -5037,17 +5045,66 @@ fn parse_interval_unit_required() { last_field: None, fractional_seconds_precision: None, }), + ); + + let sql = "SELECT INTERVAL '1' + '2' - '3' DAY"; + let select = dialects_unit_preferred.verified_only_select(sql); + assert_eq!( expr_from_projection(only(&select.projection)), + &Expr::Interval(Interval { + value: Box::new(Expr::BinaryOp { + left: Box::new(Expr::BinaryOp { + left: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))), + op: BinaryOperator::Plus, + right: Box::new(Expr::Value(Value::SingleQuotedString("2".to_string()))), + }), + op: BinaryOperator::Minus, + right: Box::new(Expr::Value(Value::SingleQuotedString("3".to_string()))), + }), + leading_field: Some(DateTimeField::Day), + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + }), + ); + + // NOTE: this is invalid SQL, hence why we need prefer_interval_units->false + let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'"; + let select = dialects_unit_preferred.verified_only_select(sql); + assert_eq!( + expr_from_projection(only(&select.projection)), + &Expr::Interval(Interval { + value: Box::new(Expr::BinaryOp { + left: Box::new(Expr::Value(Value::SingleQuotedString(String::from( + "1 DAY" + )))), + op: BinaryOperator::Gt, + right: Box::new(Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( + "1 SECOND" + )))), + leading_field: None, + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + })) + }), + leading_field: None, + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + }), ); } #[test] -fn parse_interval_unit_not_required() { - let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); +fn parse_interval_unit_not_preferred() { + let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); let sql = "SELECT INTERVAL '1 DAY'"; let select = dialect_no_unit.verified_only_select(sql); assert_eq!( + expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( "1 DAY" @@ -5057,7 +5114,6 @@ fn parse_interval_unit_not_required() { last_field: None, fractional_seconds_precision: None, }), - expr_from_projection(only(&select.projection)), ); dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR'"); @@ -5066,11 +5122,38 @@ fn parse_interval_unit_not_required() { "SELECT INTERVAL '1 YEAR' one_year", "SELECT INTERVAL '1 YEAR' AS one_year", ); + + let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'"; + let select = dialect_no_unit.verified_only_select(sql); + assert_eq!( + expr_from_projection(only(&select.projection)), + &Expr::BinaryOp { + left: Box::new(Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( + "1 DAY" + )))), + leading_field: None, + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + })), + op: BinaryOperator::Gt, + right: Box::new(Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( + "1 SECOND" + )))), + leading_field: None, + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + })) + } + ); } #[test] -fn interval_unit_not_required_gt() { - let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); +fn interval_unit_not_preferred_gt() { + let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); let expr = dialect_no_unit.verified_expr("INTERVAL '1 second' > x"); assert_eq!( expr, @@ -5094,8 +5177,8 @@ fn interval_unit_not_required_gt() { } #[test] -fn interval_unit_not_required_double_colon() { - let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); +fn interval_unit_not_preferred_double_colon() { + let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); let expr = dialect_no_unit.verified_expr("INTERVAL '1 second'::TEXT"); assert_eq!( expr, @@ -5122,7 +5205,7 @@ fn parse_interval_and_or_xor() { WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'"; - let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); + let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); let actual_ast = dialect_no_unit.parse_sql_statements(sql).unwrap(); let expected_ast = vec![Statement::Query(Box::new(Query { @@ -5225,7 +5308,7 @@ fn parse_interval_and_or_xor() { assert_eq!(actual_ast, expected_ast); - let dialect_no_unit = all_dialects_except(|d| d.require_interval_units()); + let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); dialect_no_unit.verified_stmt( "SELECT col FROM test \ WHERE d3_date > d1_date + INTERVAL '5 days' \ From b6dcbf22e5301f4b3ef75f649cbc396b5d18a2e8 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Wed, 28 Aug 2024 21:35:07 +0100 Subject: [PATCH 07/14] remove unnecessary changes --- tests/sqlparser_bigquery.rs | 2 +- tests/sqlparser_common.rs | 17 ++++++++++++++++- tests/sqlparser_postgres.rs | 5 +---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index 22d086d8a..57cf9d7fd 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -1146,7 +1146,7 @@ fn parse_typed_struct_syntax_bigquery_and_generic() { assert_eq!(2, select.projection.len()); assert_eq!( &Expr::Struct { - values: vec![Expr::Interval(Interval { + values: vec![Expr::Interval(ast::Interval { value: Box::new(Expr::Value(Value::SingleQuotedString( "1-2 3 4:5:6.789999".to_string() ))), diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 17f4bc6ea..e762eb216 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4969,6 +4969,21 @@ fn parse_interval_all() { expr_from_projection(only(&select.projection)), ); + let sql = "SELECT INTERVAL '1 DAY'"; + let select = verified_only_select(sql); + assert_eq!( + &Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( + "1 DAY" + )))), + leading_field: None, + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + }), + expr_from_projection(only(&select.projection)), + ); + let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND"); assert_eq!( ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()), @@ -5000,7 +5015,7 @@ fn parse_interval_all() { verified_only_select("SELECT INTERVAL 1 HOUR"); verified_only_select("SELECT INTERVAL 1 MINUTE"); verified_only_select("SELECT INTERVAL 1 SECOND"); - // these are allow with both variants for backwards compatibility + // these are allowed with both variants for backwards compatibility verified_only_select("SELECT INTERVAL '1 YEAR'"); verified_only_select("SELECT INTERVAL '1 MONTH'"); verified_only_select("SELECT INTERVAL '1 DAY'"); diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 4ba338dc0..2f9fe86c9 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -4353,10 +4353,7 @@ fn parse_mat_cte() { fn parse_at_time_zone() { pg_and_generic().verified_expr("CURRENT_TIMESTAMP AT TIME ZONE tz"); pg_and_generic().verified_expr("CURRENT_TIMESTAMP AT TIME ZONE ('America/' || 'Los_Angeles')"); -} -#[test] -fn parse_at_time_zone_interval() { // check precedence let expr = Expr::BinaryOp { left: Box::new(Expr::AtTimeZone { @@ -4385,7 +4382,7 @@ fn parse_at_time_zone_interval() { })), }; pretty_assertions::assert_eq!( - pg().verified_expr( + pg_and_generic().verified_expr( "TIMESTAMP '2001-09-28 01:00' AT TIME ZONE 'America/Los_Angeles'::TEXT + INTERVAL '23 hours'", ), expr From 6c2a9717897d7986c655c24bfcbcd7e814fd3d40 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Wed, 28 Aug 2024 21:38:20 +0100 Subject: [PATCH 08/14] clippy --- src/dialect/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index 0d232eac0..b30789d7b 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -518,7 +518,7 @@ pub trait Dialect: Debug + Any { /// * `INTERVAL '1' + '1' DAY` is VALID /// * `INTERVAL '1'` is INVALID /// * `INTERVAL '2' SECOND > INTERVAL '1' SECOND` is interpreted correctly, but - /// `INTERVAL '2' > INTERVAL '1'` is not! + /// * `INTERVAL '2' > INTERVAL '1'` is not! /// /// When `false`: /// * `INTERVAL '1' DAY` is VALID From 522f981b7ae032c1e66d44a0c717f86574b1141d Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Wed, 28 Aug 2024 22:19:11 +0100 Subject: [PATCH 09/14] prefer_interval_units -> allow_interval_expressions --- src/dialect/duckdb.rs | 2 +- src/dialect/mod.rs | 8 ++++--- src/dialect/postgresql.rs | 2 +- src/dialect/redshift.rs | 2 +- src/dialect/snowflake.rs | 2 +- src/parser/mod.rs | 2 +- tests/sqlparser_common.rs | 44 +++++++++++++++++++-------------------- 7 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs index 316068844..3457dafe4 100644 --- a/src/dialect/duckdb.rs +++ b/src/dialect/duckdb.rs @@ -56,7 +56,7 @@ impl Dialect for DuckDbDialect { true } - fn prefer_interval_units(&self) -> bool { + fn allow_interval_expressions(&self) -> bool { false } } diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index b30789d7b..037d03f42 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -510,13 +510,15 @@ pub trait Dialect: Debug + Any { false } - /// Whether or not units are required with interval expressions. + /// Whether expressions (like `1 + 1`) are allowed in `INTERVAL` expressions. + /// + /// See for more information. /// /// When `true`: /// * `INTERVAL '1' DAY` is VALID /// * `INTERVAL 1 + 1 DAY` is VALID /// * `INTERVAL '1' + '1' DAY` is VALID - /// * `INTERVAL '1'` is INVALID + /// * `INTERVAL '1'` is VALID /// * `INTERVAL '2' SECOND > INTERVAL '1' SECOND` is interpreted correctly, but /// * `INTERVAL '2' > INTERVAL '1'` is not! /// @@ -526,7 +528,7 @@ pub trait Dialect: Debug + Any { /// * `INTERVAL '1 second'` is VALID /// * `INTERVAL 1 + 1 DAY` is INVALID /// * `INTERVAL '2' > INTERVAL '1'` is interpreted correctly - fn prefer_interval_units(&self) -> bool { + fn allow_interval_expressions(&self) -> bool { true } } diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs index d663570fb..d9026943a 100644 --- a/src/dialect/postgresql.rs +++ b/src/dialect/postgresql.rs @@ -163,7 +163,7 @@ impl Dialect for PostgreSqlDialect { true } - fn prefer_interval_units(&self) -> bool { + fn allow_interval_expressions(&self) -> bool { false } } diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs index d554d6285..c0b6371fe 100644 --- a/src/dialect/redshift.rs +++ b/src/dialect/redshift.rs @@ -64,7 +64,7 @@ impl Dialect for RedshiftSqlDialect { true } - fn prefer_interval_units(&self) -> bool { + fn allow_interval_expressions(&self) -> bool { false } } diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs index a712fd0f8..43b9906bc 100644 --- a/src/dialect/snowflake.rs +++ b/src/dialect/snowflake.rs @@ -167,7 +167,7 @@ impl Dialect for SnowflakeDialect { true } - fn prefer_interval_units(&self) -> bool { + fn allow_interval_expressions(&self) -> bool { false } } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index a89858547..a4af15395 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2044,7 +2044,7 @@ impl<'a> Parser<'a> { // don't currently try to parse it. (The sign can instead be included // inside the value string.) - let value = if self.dialect.prefer_interval_units() { + let value = if self.dialect.allow_interval_expressions() { // parse a whole expression so `INTERVAL 1 + 1 DAY` is valid self.parse_expr()? } else { diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index e762eb216..505e99435 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -5025,11 +5025,11 @@ fn parse_interval_all() { } #[test] -fn parse_interval_unit_preferred() { - let dialects_unit_preferred = all_dialects_where(|d| d.prefer_interval_units()); +fn parse_interval_allow_interval_expr() { + let dialects = all_dialects_where(|d| d.allow_interval_expressions()); let sql = "SELECT INTERVAL 1 + 1 DAY"; - let select = dialects_unit_preferred.verified_only_select(sql); + let select = dialects.verified_only_select(sql); assert_eq!( expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { @@ -5046,7 +5046,7 @@ fn parse_interval_unit_preferred() { ); let sql = "SELECT INTERVAL '1' + '1' DAY"; - let select = dialects_unit_preferred.verified_only_select(sql); + let select = dialects.verified_only_select(sql); assert_eq!( expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { @@ -5063,7 +5063,7 @@ fn parse_interval_unit_preferred() { ); let sql = "SELECT INTERVAL '1' + '2' - '3' DAY"; - let select = dialects_unit_preferred.verified_only_select(sql); + let select = dialects.verified_only_select(sql); assert_eq!( expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { @@ -5085,7 +5085,7 @@ fn parse_interval_unit_preferred() { // NOTE: this is invalid SQL, hence why we need prefer_interval_units->false let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'"; - let select = dialects_unit_preferred.verified_only_select(sql); + let select = dialects.verified_only_select(sql); assert_eq!( expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { @@ -5113,11 +5113,11 @@ fn parse_interval_unit_preferred() { } #[test] -fn parse_interval_unit_not_preferred() { - let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); +fn parse_interval_disallow_interval_expr() { + let dialects = all_dialects_except(|d| d.allow_interval_expressions()); let sql = "SELECT INTERVAL '1 DAY'"; - let select = dialect_no_unit.verified_only_select(sql); + let select = dialects.verified_only_select(sql); assert_eq!( expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { @@ -5131,15 +5131,15 @@ fn parse_interval_unit_not_preferred() { }), ); - dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR'"); - dialect_no_unit.verified_only_select("SELECT INTERVAL '1 YEAR' AS one_year"); - dialect_no_unit.one_statement_parses_to( + dialects.verified_only_select("SELECT INTERVAL '1 YEAR'"); + dialects.verified_only_select("SELECT INTERVAL '1 YEAR' AS one_year"); + dialects.one_statement_parses_to( "SELECT INTERVAL '1 YEAR' one_year", "SELECT INTERVAL '1 YEAR' AS one_year", ); let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'"; - let select = dialect_no_unit.verified_only_select(sql); + let select = dialects.verified_only_select(sql); assert_eq!( expr_from_projection(only(&select.projection)), &Expr::BinaryOp { @@ -5167,9 +5167,9 @@ fn parse_interval_unit_not_preferred() { } #[test] -fn interval_unit_not_preferred_gt() { - let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); - let expr = dialect_no_unit.verified_expr("INTERVAL '1 second' > x"); +fn interval_disallow_interval_expr_gt() { + let dialects = all_dialects_except(|d| d.allow_interval_expressions()); + let expr = dialects.verified_expr("INTERVAL '1 second' > x"); assert_eq!( expr, Expr::BinaryOp { @@ -5192,9 +5192,9 @@ fn interval_unit_not_preferred_gt() { } #[test] -fn interval_unit_not_preferred_double_colon() { - let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); - let expr = dialect_no_unit.verified_expr("INTERVAL '1 second'::TEXT"); +fn interval_disallow_interval_expr_double_colon() { + let dialects = all_dialects_except(|d| d.allow_interval_expressions()); + let expr = dialects.verified_expr("INTERVAL '1 second'::TEXT"); assert_eq!( expr, Expr::Cast { @@ -5220,8 +5220,8 @@ fn parse_interval_and_or_xor() { WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'"; - let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); - let actual_ast = dialect_no_unit.parse_sql_statements(sql).unwrap(); + let dialects = all_dialects_except(|d| d.allow_interval_expressions()); + let actual_ast = dialects.parse_sql_statements(sql).unwrap(); let expected_ast = vec![Statement::Query(Box::new(Query { with: None, @@ -5323,7 +5323,7 @@ fn parse_interval_and_or_xor() { assert_eq!(actual_ast, expected_ast); - let dialect_no_unit = all_dialects_except(|d| d.prefer_interval_units()); + let dialect_no_unit = all_dialects_except(|d| d.allow_interval_expressions()); dialect_no_unit.verified_stmt( "SELECT col FROM test \ WHERE d3_date > d1_date + INTERVAL '5 days' \ From aebf8580444c0c99ca26c7d74c87589180e5a906 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Thu, 29 Aug 2024 09:42:01 +0100 Subject: [PATCH 10/14] make generic more lax by not MySQL etc. --- src/dialect/generic.rs | 4 ++ src/dialect/mod.rs | 16 +++++++- src/dialect/postgresql.rs | 4 ++ src/dialect/redshift.rs | 4 ++ src/dialect/snowflake.rs | 4 ++ src/parser/mod.rs | 4 ++ tests/sqlparser_bigquery.rs | 20 ++++------ tests/sqlparser_common.rs | 76 +++++++++++++++++++++---------------- 8 files changed, 87 insertions(+), 45 deletions(-) diff --git a/src/dialect/generic.rs b/src/dialect/generic.rs index 211abd976..0ed1ada0d 100644 --- a/src/dialect/generic.rs +++ b/src/dialect/generic.rs @@ -86,4 +86,8 @@ impl Dialect for GenericDialect { fn allow_extract_single_quotes(&self) -> bool { true } + + fn require_interval_units(&self) -> bool { + false + } } diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index 037d03f42..2a12d8700 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -525,12 +525,26 @@ pub trait Dialect: Debug + Any { /// When `false`: /// * `INTERVAL '1' DAY` is VALID /// * `INTERVAL '1'` is VALID - /// * `INTERVAL '1 second'` is VALID /// * `INTERVAL 1 + 1 DAY` is INVALID /// * `INTERVAL '2' > INTERVAL '1'` is interpreted correctly fn allow_interval_expressions(&self) -> bool { true } + + /// Whether the `INTERVAL` keyword requires units to be specified, e.g. `INTERVAL 1 DAY` vs `INTERVAL 1`. + /// + /// See for more information. + /// + /// When `true`: + /// * `INTERVAL '1' DAY` is VALID + /// * `INTERVAL '1'` is INVALID + /// + /// When `false`: + /// * `INTERVAL '1' DAY` is VALID + /// * `INTERVAL '1'` is VALID + fn require_interval_units(&self) -> bool { + true + } } /// This represents the operators for which precedence must be defined diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs index d9026943a..7276ea16c 100644 --- a/src/dialect/postgresql.rs +++ b/src/dialect/postgresql.rs @@ -166,6 +166,10 @@ impl Dialect for PostgreSqlDialect { fn allow_interval_expressions(&self) -> bool { false } + + fn require_interval_units(&self) -> bool { + false + } } pub fn parse_comment(parser: &mut Parser) -> Result { diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs index c0b6371fe..1ae407f3f 100644 --- a/src/dialect/redshift.rs +++ b/src/dialect/redshift.rs @@ -67,4 +67,8 @@ impl Dialect for RedshiftSqlDialect { fn allow_interval_expressions(&self) -> bool { false } + + fn require_interval_units(&self) -> bool { + false + } } diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs index 43b9906bc..07a5092da 100644 --- a/src/dialect/snowflake.rs +++ b/src/dialect/snowflake.rs @@ -170,6 +170,10 @@ impl Dialect for SnowflakeDialect { fn allow_interval_expressions(&self) -> bool { false } + + fn require_interval_units(&self) -> bool { + false + } } /// Parse snowflake create table statement. diff --git a/src/parser/mod.rs b/src/parser/mod.rs index a4af15395..2117fc8be 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2060,6 +2060,10 @@ impl<'a> Parser<'a> { // this more general implementation. let leading_field = if self.next_token_is_unit() { Some(self.parse_date_time_field()?) + } else if self.dialect.require_interval_units() { + return Err(ParserError::ParserError( + "INTERVAL requires a unit like DAY after the literal value".to_string(), + )); } else { None }; diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index 57cf9d7fd..b17f1ae78 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -830,16 +830,14 @@ fn parse_typed_struct_syntax_bigquery() { expr_from_projection(&select.projection[3]) ); - let sql = r#"SELECT STRUCT(INTERVAL '1-2 3 4:5:6.789999'), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; + let sql = r#"SELECT STRUCT(INTERVAL '2' HOUR), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; let select = bigquery().verified_only_select(sql); assert_eq!(2, select.projection.len()); assert_eq!( &Expr::Struct { - values: vec![Expr::Interval(ast::Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString( - "1-2 3 4:5:6.789999".to_string() - ))), - leading_field: None, + values: vec![Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString("2".to_string()))), + leading_field: Some(DateTimeField::Hour), leading_precision: None, last_field: None, fractional_seconds_precision: None @@ -1141,16 +1139,14 @@ fn parse_typed_struct_syntax_bigquery_and_generic() { expr_from_projection(&select.projection[3]) ); - let sql = r#"SELECT STRUCT(INTERVAL '1-2 3 4:5:6.789999'), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; + let sql = r#"SELECT STRUCT(INTERVAL '1' MONTH), STRUCT(JSON '{"class" : {"students" : [{"name" : "Jane"}]}}')"#; let select = bigquery_and_generic().verified_only_select(sql); assert_eq!(2, select.projection.len()); assert_eq!( &Expr::Struct { - values: vec![Expr::Interval(ast::Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString( - "1-2 3 4:5:6.789999".to_string() - ))), - leading_field: None, + values: vec![Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString("1".to_string()))), + leading_field: Some(DateTimeField::Month), leading_precision: None, last_field: None, fractional_seconds_precision: None diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 505e99435..844fb2a94 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4472,7 +4472,8 @@ fn parse_window_functions() { sum(qux) OVER (ORDER BY a \ GROUPS BETWEEN 1 PRECEDING AND 1 FOLLOWING) \ FROM foo"; - let select = verified_only_select(sql); + let dialects = all_dialects_except(|d| d.require_interval_units()); + let select = dialects.verified_only_select(sql); const EXPECTED_PROJ_QTY: usize = 7; assert_eq!(EXPECTED_PROJ_QTY, select.projection.len()); @@ -4969,21 +4970,6 @@ fn parse_interval_all() { expr_from_projection(only(&select.projection)), ); - let sql = "SELECT INTERVAL '1 DAY'"; - let select = verified_only_select(sql); - assert_eq!( - &Expr::Interval(Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( - "1 DAY" - )))), - leading_field: None, - leading_precision: None, - last_field: None, - fractional_seconds_precision: None, - }), - expr_from_projection(only(&select.projection)), - ); - let result = parse_sql_statements("SELECT INTERVAL '1' SECOND TO SECOND"); assert_eq!( ParserError::ParserError("Expected: end of statement, found: SECOND".to_string()), @@ -5015,13 +5001,32 @@ fn parse_interval_all() { verified_only_select("SELECT INTERVAL 1 HOUR"); verified_only_select("SELECT INTERVAL 1 MINUTE"); verified_only_select("SELECT INTERVAL 1 SECOND"); - // these are allowed with both variants for backwards compatibility - verified_only_select("SELECT INTERVAL '1 YEAR'"); - verified_only_select("SELECT INTERVAL '1 MONTH'"); - verified_only_select("SELECT INTERVAL '1 DAY'"); - verified_only_select("SELECT INTERVAL '1 HOUR'"); - verified_only_select("SELECT INTERVAL '1 MINUTE'"); - verified_only_select("SELECT INTERVAL '1 SECOND'"); +} + +#[test] +fn parse_interval_dont_require_unit() { + let dialects = all_dialects_except(|d| d.require_interval_units()); + + let sql = "SELECT INTERVAL '1 DAY'"; + let select = dialects.verified_only_select(sql); + assert_eq!( + &Expr::Interval(Interval { + value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( + "1 DAY" + )))), + leading_field: None, + leading_precision: None, + last_field: None, + fractional_seconds_precision: None, + }), + expr_from_projection(only(&select.projection)), + ); + dialects.verified_only_select("SELECT INTERVAL '1 YEAR'"); + dialects.verified_only_select("SELECT INTERVAL '1 MONTH'"); + dialects.verified_only_select("SELECT INTERVAL '1 DAY'"); + dialects.verified_only_select("SELECT INTERVAL '1 HOUR'"); + dialects.verified_only_select("SELECT INTERVAL '1 MINUTE'"); + dialects.verified_only_select("SELECT INTERVAL '1 SECOND'"); } #[test] @@ -5082,10 +5087,18 @@ fn parse_interval_allow_interval_expr() { fractional_seconds_precision: None, }), ); +} +#[test] +fn parse_interval_generic() { // NOTE: this is invalid SQL, hence why we need prefer_interval_units->false let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'"; - let select = dialects.verified_only_select(sql); + + let generic = TestedDialects { + dialects: vec![Box::new(GenericDialect {}) as Box], + options: None, + }; + let select = generic.verified_only_select(sql); assert_eq!( expr_from_projection(only(&select.projection)), &Expr::Interval(Interval { @@ -5114,7 +5127,7 @@ fn parse_interval_allow_interval_expr() { #[test] fn parse_interval_disallow_interval_expr() { - let dialects = all_dialects_except(|d| d.allow_interval_expressions()); + let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); let sql = "SELECT INTERVAL '1 DAY'"; let select = dialects.verified_only_select(sql); @@ -5168,7 +5181,7 @@ fn parse_interval_disallow_interval_expr() { #[test] fn interval_disallow_interval_expr_gt() { - let dialects = all_dialects_except(|d| d.allow_interval_expressions()); + let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); let expr = dialects.verified_expr("INTERVAL '1 second' > x"); assert_eq!( expr, @@ -5193,7 +5206,7 @@ fn interval_disallow_interval_expr_gt() { #[test] fn interval_disallow_interval_expr_double_colon() { - let dialects = all_dialects_except(|d| d.allow_interval_expressions()); + let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); let expr = dialects.verified_expr("INTERVAL '1 second'::TEXT"); assert_eq!( expr, @@ -5220,7 +5233,7 @@ fn parse_interval_and_or_xor() { WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'"; - let dialects = all_dialects_except(|d| d.allow_interval_expressions()); + let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); let actual_ast = dialects.parse_sql_statements(sql).unwrap(); let expected_ast = vec![Statement::Query(Box::new(Query { @@ -5323,20 +5336,19 @@ fn parse_interval_and_or_xor() { assert_eq!(actual_ast, expected_ast); - let dialect_no_unit = all_dialects_except(|d| d.allow_interval_expressions()); - dialect_no_unit.verified_stmt( + dialects.verified_stmt( "SELECT col FROM test \ WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'", ); - dialect_no_unit.verified_stmt( + dialects.verified_stmt( "SELECT col FROM test \ WHERE d3_date > d1_date + INTERVAL '5 days' \ OR d2_date > d1_date + INTERVAL '3 days'", ); - dialect_no_unit.verified_stmt( + dialects.verified_stmt( "SELECT col FROM test \ WHERE d3_date > d1_date + INTERVAL '5 days' \ XOR d2_date > d1_date + INTERVAL '3 days'", From 4e13da3205a2b878fc1437d8571733b8b0ce2eaf Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Thu, 29 Aug 2024 09:47:15 +0100 Subject: [PATCH 11/14] git commit cleanup --- src/parser/mod.rs | 7 ++++--- tests/sqlparser_common.rs | 24 ++++++++++++++++++++---- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 2117fc8be..5616d01b7 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2061,9 +2061,10 @@ impl<'a> Parser<'a> { let leading_field = if self.next_token_is_unit() { Some(self.parse_date_time_field()?) } else if self.dialect.require_interval_units() { - return Err(ParserError::ParserError( - "INTERVAL requires a unit like DAY after the literal value".to_string(), - )); + return parser_err!( + "INTERVAL requires a unit after the literal value", + self.peek_token().location + ); } else { None }; diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 844fb2a94..76f2acb46 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -5029,6 +5029,18 @@ fn parse_interval_dont_require_unit() { dialects.verified_only_select("SELECT INTERVAL '1 SECOND'"); } +#[test] +fn parse_interval_require_unit() { + let dialects = all_dialects_where(|d| d.require_interval_units()); + + let sql = "SELECT INTERVAL '1 DAY'"; + let err = dialects.parse_sql_statements(sql).unwrap_err(); + assert_eq!( + err.to_string(), + "sql parser error: INTERVAL requires a unit after the literal value" + ) +} + #[test] fn parse_interval_allow_interval_expr() { let dialects = all_dialects_where(|d| d.allow_interval_expressions()); @@ -5127,7 +5139,8 @@ fn parse_interval_generic() { #[test] fn parse_interval_disallow_interval_expr() { - let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); + let dialects = + all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); let sql = "SELECT INTERVAL '1 DAY'"; let select = dialects.verified_only_select(sql); @@ -5181,7 +5194,8 @@ fn parse_interval_disallow_interval_expr() { #[test] fn interval_disallow_interval_expr_gt() { - let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); + let dialects = + all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); let expr = dialects.verified_expr("INTERVAL '1 second' > x"); assert_eq!( expr, @@ -5206,7 +5220,8 @@ fn interval_disallow_interval_expr_gt() { #[test] fn interval_disallow_interval_expr_double_colon() { - let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); + let dialects = + all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); let expr = dialects.verified_expr("INTERVAL '1 second'::TEXT"); assert_eq!( expr, @@ -5233,7 +5248,8 @@ fn parse_interval_and_or_xor() { WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'"; - let dialects = all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); + let dialects = + all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); let actual_ast = dialects.parse_sql_statements(sql).unwrap(); let expected_ast = vec![Statement::Query(Box::new(Query { From 31a19f77a502d10fda487cffc0c65bfedcf4ffc9 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Thu, 29 Aug 2024 09:55:21 +0100 Subject: [PATCH 12/14] fix duckdb and linting --- src/dialect/duckdb.rs | 4 ++++ tests/sqlparser_bigquery.rs | 1 - 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs index 3457dafe4..d8411972f 100644 --- a/src/dialect/duckdb.rs +++ b/src/dialect/duckdb.rs @@ -59,4 +59,8 @@ impl Dialect for DuckDbDialect { fn allow_interval_expressions(&self) -> bool { false } + + fn require_interval_units(&self) -> bool { + false + } } diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index b17f1ae78..4f84b376d 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -13,7 +13,6 @@ #[macro_use] mod test_utils; -use sqlparser::ast; use std::ops::Deref; use sqlparser::ast::*; From 4cc404a3d471fad34e5110dcdc6e70f7f21429d9 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Wed, 4 Sep 2024 12:48:49 +0100 Subject: [PATCH 13/14] rename and document next_token_is_temporal_unit --- src/parser/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 6281f65c6..3ff6bcc02 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2058,7 +2058,7 @@ impl<'a> Parser<'a> { // // Note that PostgreSQL allows omitting the qualifier, so we provide // this more general implementation. - let leading_field = if self.next_token_is_unit() { + let leading_field = if self.next_token_is_temporal_unit() { Some(self.parse_date_time_field()?) } else if self.dialect.require_interval_units() { return parser_err!( @@ -2103,7 +2103,9 @@ impl<'a> Parser<'a> { })) } - pub fn next_token_is_unit(&mut self) -> bool { + /// Peek at the next token and determine if it is a temporal unit + /// like `second`. + pub fn next_token_is_temporal_unit(&mut self) -> bool { if let Token::Word(word) = self.peek_token().token { matches!( word.keyword, From 297df8eda08e93a9fcfce25071eda9916ef2e697 Mon Sep 17 00:00:00 2001 From: Samuel Colvin Date: Wed, 4 Sep 2024 15:11:52 +0100 Subject: [PATCH 14/14] simplify syntax choices --- src/dialect/ansi.rs | 4 +++ src/dialect/bigquery.rs | 4 +++ src/dialect/clickhouse.rs | 4 +++ src/dialect/databricks.rs | 4 +++ src/dialect/duckdb.rs | 8 ------ src/dialect/generic.rs | 4 --- src/dialect/hive.rs | 4 +++ src/dialect/mod.rs | 31 ++++++--------------- src/dialect/mysql.rs | 4 +++ src/dialect/postgresql.rs | 8 ------ src/dialect/redshift.rs | 8 ------ src/dialect/snowflake.rs | 8 ------ src/parser/mod.rs | 7 +++-- tests/sqlparser_common.rs | 58 ++++++--------------------------------- 14 files changed, 46 insertions(+), 110 deletions(-) diff --git a/src/dialect/ansi.rs b/src/dialect/ansi.rs index d07bc07eb..61ae5829e 100644 --- a/src/dialect/ansi.rs +++ b/src/dialect/ansi.rs @@ -24,4 +24,8 @@ impl Dialect for AnsiDialect { fn is_identifier_part(&self, ch: char) -> bool { ch.is_ascii_lowercase() || ch.is_ascii_uppercase() || ch.is_ascii_digit() || ch == '_' } + + fn require_interval_qualifier(&self) -> bool { + true + } } diff --git a/src/dialect/bigquery.rs b/src/dialect/bigquery.rs index d3673337f..3bce6702b 100644 --- a/src/dialect/bigquery.rs +++ b/src/dialect/bigquery.rs @@ -63,4 +63,8 @@ impl Dialect for BigQueryDialect { fn supports_select_wildcard_except(&self) -> bool { true } + + fn require_interval_qualifier(&self) -> bool { + true + } } diff --git a/src/dialect/clickhouse.rs b/src/dialect/clickhouse.rs index 34940467a..09735cbee 100644 --- a/src/dialect/clickhouse.rs +++ b/src/dialect/clickhouse.rs @@ -37,4 +37,8 @@ impl Dialect for ClickHouseDialect { fn describe_requires_table_keyword(&self) -> bool { true } + + fn require_interval_qualifier(&self) -> bool { + true + } } diff --git a/src/dialect/databricks.rs b/src/dialect/databricks.rs index 42d432d3d..d3661444b 100644 --- a/src/dialect/databricks.rs +++ b/src/dialect/databricks.rs @@ -38,4 +38,8 @@ impl Dialect for DatabricksDialect { fn supports_select_wildcard_except(&self) -> bool { true } + + fn require_interval_qualifier(&self) -> bool { + true + } } diff --git a/src/dialect/duckdb.rs b/src/dialect/duckdb.rs index d8411972f..1fc211685 100644 --- a/src/dialect/duckdb.rs +++ b/src/dialect/duckdb.rs @@ -55,12 +55,4 @@ impl Dialect for DuckDbDialect { fn support_map_literal_syntax(&self) -> bool { true } - - fn allow_interval_expressions(&self) -> bool { - false - } - - fn require_interval_units(&self) -> bool { - false - } } diff --git a/src/dialect/generic.rs b/src/dialect/generic.rs index a120323f1..c8f1c00d9 100644 --- a/src/dialect/generic.rs +++ b/src/dialect/generic.rs @@ -90,8 +90,4 @@ impl Dialect for GenericDialect { fn supports_create_index_with_clause(&self) -> bool { true } - - fn require_interval_units(&self) -> bool { - false - } } diff --git a/src/dialect/hive.rs b/src/dialect/hive.rs index 2340df611..b32d44cb9 100644 --- a/src/dialect/hive.rs +++ b/src/dialect/hive.rs @@ -42,4 +42,8 @@ impl Dialect for HiveDialect { fn supports_numeric_prefix(&self) -> bool { true } + + fn require_interval_qualifier(&self) -> bool { + true + } } diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index 3206a0db8..0be8c17c7 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -516,7 +516,10 @@ pub trait Dialect: Debug + Any { false } - /// Whether expressions (like `1 + 1`) are allowed in `INTERVAL` expressions. + /// Whether `INTERVAL` expressions require units (called "qualifiers" in the ANSI SQL spec) to be specified, + /// e.g. `INTERVAL 1 DAY` vs `INTERVAL 1`. + /// + /// Expressions within intervals (e.g. `INTERVAL '1' + '1' DAY`) are only allowed when units are required. /// /// See for more information. /// @@ -524,32 +527,14 @@ pub trait Dialect: Debug + Any { /// * `INTERVAL '1' DAY` is VALID /// * `INTERVAL 1 + 1 DAY` is VALID /// * `INTERVAL '1' + '1' DAY` is VALID - /// * `INTERVAL '1'` is VALID - /// * `INTERVAL '2' SECOND > INTERVAL '1' SECOND` is interpreted correctly, but - /// * `INTERVAL '2' > INTERVAL '1'` is not! - /// - /// When `false`: - /// * `INTERVAL '1' DAY` is VALID - /// * `INTERVAL '1'` is VALID - /// * `INTERVAL 1 + 1 DAY` is INVALID - /// * `INTERVAL '2' > INTERVAL '1'` is interpreted correctly - fn allow_interval_expressions(&self) -> bool { - true - } - - /// Whether the `INTERVAL` keyword requires units to be specified, e.g. `INTERVAL 1 DAY` vs `INTERVAL 1`. - /// - /// See for more information. - /// - /// When `true`: - /// * `INTERVAL '1' DAY` is VALID /// * `INTERVAL '1'` is INVALID /// /// When `false`: - /// * `INTERVAL '1' DAY` is VALID /// * `INTERVAL '1'` is VALID - fn require_interval_units(&self) -> bool { - true + /// * `INTERVAL '1' DAY` is VALID — unit is not required, but still allowed + /// * `INTERVAL 1 + 1 DAY` is INVALID + fn require_interval_qualifier(&self) -> bool { + false } } diff --git a/src/dialect/mysql.rs b/src/dialect/mysql.rs index 32525658b..b8c4631fd 100644 --- a/src/dialect/mysql.rs +++ b/src/dialect/mysql.rs @@ -84,6 +84,10 @@ impl Dialect for MySqlDialect { None } } + + fn require_interval_qualifier(&self) -> bool { + true + } } /// `LOCK TABLES` diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs index 759c52b28..eba3a6989 100644 --- a/src/dialect/postgresql.rs +++ b/src/dialect/postgresql.rs @@ -166,14 +166,6 @@ impl Dialect for PostgreSqlDialect { fn supports_create_index_with_clause(&self) -> bool { true } - - fn allow_interval_expressions(&self) -> bool { - false - } - - fn require_interval_units(&self) -> bool { - false - } } pub fn parse_comment(parser: &mut Parser) -> Result { diff --git a/src/dialect/redshift.rs b/src/dialect/redshift.rs index 1ae407f3f..bd4dc817c 100644 --- a/src/dialect/redshift.rs +++ b/src/dialect/redshift.rs @@ -63,12 +63,4 @@ impl Dialect for RedshiftSqlDialect { fn supports_connect_by(&self) -> bool { true } - - fn allow_interval_expressions(&self) -> bool { - false - } - - fn require_interval_units(&self) -> bool { - false - } } diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs index 07a5092da..4f37004b1 100644 --- a/src/dialect/snowflake.rs +++ b/src/dialect/snowflake.rs @@ -166,14 +166,6 @@ impl Dialect for SnowflakeDialect { fn allow_extract_single_quotes(&self) -> bool { true } - - fn allow_interval_expressions(&self) -> bool { - false - } - - fn require_interval_units(&self) -> bool { - false - } } /// Parse snowflake create table statement. diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 3ff6bcc02..b7d44d2f2 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2044,7 +2044,10 @@ impl<'a> Parser<'a> { // don't currently try to parse it. (The sign can instead be included // inside the value string.) - let value = if self.dialect.allow_interval_expressions() { + // to match the different flavours of INTERVAL syntax, we only allow expressions + // if the dialect requires an interval qualifier, + // see https://github.com/sqlparser-rs/sqlparser-rs/pull/1398 for more details + let value = if self.dialect.require_interval_qualifier() { // parse a whole expression so `INTERVAL 1 + 1 DAY` is valid self.parse_expr()? } else { @@ -2060,7 +2063,7 @@ impl<'a> Parser<'a> { // this more general implementation. let leading_field = if self.next_token_is_temporal_unit() { Some(self.parse_date_time_field()?) - } else if self.dialect.require_interval_units() { + } else if self.dialect.require_interval_qualifier() { return parser_err!( "INTERVAL requires a unit after the literal value", self.peek_token().location diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 5a5131824..0ed677db9 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4472,7 +4472,7 @@ fn parse_window_functions() { sum(qux) OVER (ORDER BY a \ GROUPS BETWEEN 1 PRECEDING AND 1 FOLLOWING) \ FROM foo"; - let dialects = all_dialects_except(|d| d.require_interval_units()); + let dialects = all_dialects_except(|d| d.require_interval_qualifier()); let select = dialects.verified_only_select(sql); const EXPECTED_PROJ_QTY: usize = 7; @@ -5005,7 +5005,7 @@ fn parse_interval_all() { #[test] fn parse_interval_dont_require_unit() { - let dialects = all_dialects_except(|d| d.require_interval_units()); + let dialects = all_dialects_except(|d| d.require_interval_qualifier()); let sql = "SELECT INTERVAL '1 DAY'"; let select = dialects.verified_only_select(sql); @@ -5031,7 +5031,7 @@ fn parse_interval_dont_require_unit() { #[test] fn parse_interval_require_unit() { - let dialects = all_dialects_where(|d| d.require_interval_units()); + let dialects = all_dialects_where(|d| d.require_interval_qualifier()); let sql = "SELECT INTERVAL '1 DAY'"; let err = dialects.parse_sql_statements(sql).unwrap_err(); @@ -5042,8 +5042,8 @@ fn parse_interval_require_unit() { } #[test] -fn parse_interval_allow_interval_expr() { - let dialects = all_dialects_where(|d| d.allow_interval_expressions()); +fn parse_interval_require_qualifier() { + let dialects = all_dialects_where(|d| d.require_interval_qualifier()); let sql = "SELECT INTERVAL 1 + 1 DAY"; let select = dialects.verified_only_select(sql); @@ -5101,46 +5101,9 @@ fn parse_interval_allow_interval_expr() { ); } -#[test] -fn parse_interval_generic() { - // NOTE: this is invalid SQL, hence why we need prefer_interval_units->false - let sql = "SELECT INTERVAL '1 DAY' > INTERVAL '1 SECOND'"; - - let generic = TestedDialects { - dialects: vec![Box::new(GenericDialect {}) as Box], - options: None, - }; - let select = generic.verified_only_select(sql); - assert_eq!( - expr_from_projection(only(&select.projection)), - &Expr::Interval(Interval { - value: Box::new(Expr::BinaryOp { - left: Box::new(Expr::Value(Value::SingleQuotedString(String::from( - "1 DAY" - )))), - op: BinaryOperator::Gt, - right: Box::new(Expr::Interval(Interval { - value: Box::new(Expr::Value(Value::SingleQuotedString(String::from( - "1 SECOND" - )))), - leading_field: None, - leading_precision: None, - last_field: None, - fractional_seconds_precision: None, - })) - }), - leading_field: None, - leading_precision: None, - last_field: None, - fractional_seconds_precision: None, - }), - ); -} - #[test] fn parse_interval_disallow_interval_expr() { - let dialects = - all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); + let dialects = all_dialects_except(|d| d.require_interval_qualifier()); let sql = "SELECT INTERVAL '1 DAY'"; let select = dialects.verified_only_select(sql); @@ -5194,8 +5157,7 @@ fn parse_interval_disallow_interval_expr() { #[test] fn interval_disallow_interval_expr_gt() { - let dialects = - all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); + let dialects = all_dialects_except(|d| d.require_interval_qualifier()); let expr = dialects.verified_expr("INTERVAL '1 second' > x"); assert_eq!( expr, @@ -5220,8 +5182,7 @@ fn interval_disallow_interval_expr_gt() { #[test] fn interval_disallow_interval_expr_double_colon() { - let dialects = - all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); + let dialects = all_dialects_except(|d| d.require_interval_qualifier()); let expr = dialects.verified_expr("INTERVAL '1 second'::TEXT"); assert_eq!( expr, @@ -5248,8 +5209,7 @@ fn parse_interval_and_or_xor() { WHERE d3_date > d1_date + INTERVAL '5 days' \ AND d2_date > d1_date + INTERVAL '3 days'"; - let dialects = - all_dialects_where(|d| !d.allow_interval_expressions() && !d.require_interval_units()); + let dialects = all_dialects_except(|d| d.require_interval_qualifier()); let actual_ast = dialects.parse_sql_statements(sql).unwrap(); let expected_ast = vec![Statement::Query(Box::new(Query {