iffyio commented on code in PR #1630:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1630#discussion_r1905821060
##########
tests/sqlparser_common.rs:
##########
@@ -5374,10 +5396,49 @@ fn parse_interval_all() {
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 WEEK");
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 YEARS");
+ verified_only_select("SELECT INTERVAL 1 MONTHS");
+ verified_only_select("SELECT INTERVAL 1 WEEKS");
+ verified_only_select("SELECT INTERVAL 1 DAYS");
+ verified_only_select("SELECT INTERVAL 1 HOURS");
+ verified_only_select("SELECT INTERVAL 1 MINUTES");
+ verified_only_select("SELECT INTERVAL 1 SECONDS");
+ verified_only_select(
+ "SELECT '2 years 15 months 100 weeks 99 hours 123456789
milliseconds'::INTERVAL",
+ );
+
+ // keep Generic/BigQuery extract week with weekday syntax success
+ let supported_dialects = TestedDialects::new(vec![
+ Box::new(GenericDialect {}),
+ Box::new(BigQueryDialect {}),
+ ]);
+
+ let sql = "SELECT EXTRACT(WEEK(a) FROM date)";
+ supported_dialects.verified_stmt(sql);
Review Comment:
Oh not sure we need to test this here, its [already tested
here](https://github.com/apache/datafusion-sqlparser-rs/blob/ba3acb243007555bfc90885fcc9aad52642c06ca/tests/sqlparser_bigquery.rs#L2181-L2193)
for example
##########
tests/sqlparser_common.rs:
##########
@@ -5374,10 +5396,49 @@ fn parse_interval_all() {
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 WEEK");
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 YEARS");
+ verified_only_select("SELECT INTERVAL 1 MONTHS");
+ verified_only_select("SELECT INTERVAL 1 WEEKS");
+ verified_only_select("SELECT INTERVAL 1 DAYS");
+ verified_only_select("SELECT INTERVAL 1 HOURS");
+ verified_only_select("SELECT INTERVAL 1 MINUTES");
+ verified_only_select("SELECT INTERVAL 1 SECONDS");
+ verified_only_select(
+ "SELECT '2 years 15 months 100 weeks 99 hours 123456789
milliseconds'::INTERVAL",
+ );
+
+ // keep Generic/BigQuery extract week with weekday syntax success
+ let supported_dialects = TestedDialects::new(vec![
+ Box::new(GenericDialect {}),
+ Box::new(BigQueryDialect {}),
+ ]);
+
+ let sql = "SELECT EXTRACT(WEEK(a) FROM date)";
+ supported_dialects.verified_stmt(sql);
+
+ let all_other_dialects = TestedDialects::new(vec![
+ Box::new(PostgreSqlDialect {}),
+ Box::new(MsSqlDialect {}),
+ Box::new(AnsiDialect {}),
+ Box::new(SnowflakeDialect {}),
+ Box::new(HiveDialect {}),
+ Box::new(RedshiftSqlDialect {}),
+ Box::new(MySqlDialect {}),
+ Box::new(SQLiteDialect {}),
+ Box::new(DuckDbDialect {}),
+ ]);
+
+ assert_eq!(
+ ParserError::ParserError("Expected 'FROM' or ','".to_owned()),
+ all_other_dialects
+ .parse_sql_statements("SELECT EXTRACT(WEEK(a) FROM date)")
+ .unwrap_err()
+ );
Review Comment:
similarly I think we can skip this test, if a dialect doesnt support a
feature no need to test that feature with the dialect
##########
src/parser/mod.rs:
##########
@@ -2353,14 +2355,30 @@ impl<'a> Parser<'a> {
};
Ok(DateTimeField::Week(week_day))
}
+ Keyword::WEEKS => {
+ let week_day = if dialect_of!(self is BigQueryDialect)
+ && self.consume_token(&Token::LParen)
+ {
+ let week_day = self.parse_identifier()?;
+ self.expect_token(&Token::RParen)?;
+ Some(week_day)
+ } else {
+ None
+ };
+ Ok(DateTimeField::Weeks(week_day))
+ }
Review Comment:
That sounds reasonable thanks for checking! It seems like
`Weeks(Option<Ident>)` isn't supported by any dialect? I'm thinking if so we
can change that to `Weeks`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
