LucaCappelletti94 commented on code in PR #2614:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2614#discussion_r4282590147
##########
tests/sqlparser_common.rs:
##########
@@ -6764,9 +6766,87 @@ fn parse_interval_require_unit() {
)
}
+#[test]
+fn parse_interval_string_without_qualifier() {
+ let dialects = all_dialects_where(|d|
d.supports_interval_string_without_qualifier());
+ for sql in [
+ "SELECT INTERVAL '2 months'",
+ "SELECT INTERVAL '1' DAY",
+ "SELECT INTERVAL '1-2' YEAR TO MONTH",
+ "SELECT INTERVAL -'1' DAY",
+ "SELECT INTERVAL 3 DAY",
+ "SELECT INTERVAL -1 DAY, INTERVAL +2 HOURS",
+ "SELECT INTERVAL '2 seconds' * 2",
+ "SELECT INTERVAL \"2 months\"",
+ "SELECT INTERVAL \"1\" DAY",
+ ] {
+ dialects.verified_stmt(sql);
+ }
+ for sql in [
+ "SELECT INTERVAL 1",
+ "SELECT INTERVAL 1 + 1 DAY",
+ "SELECT INTERVAL x DAY",
+ ] {
+ assert!(
+ dialects.parse_sql_statements(sql).is_err(),
+ "{sql} should not parse"
+ );
+ }
+ dialects.verified_stmt("SELECT interval, x FROM t");
+ dialects.verified_stmt("SELECT max(interval) FROM t WHERE interval > 1");
+ dialects.verified_stmt("SELECT interval - 1, interval + 1 FROM t");
+ dialects.one_statement_parses_to("SELECT INTERVAL -1", "SELECT INTERVAL -
1");
+ dialects.one_statement_parses_to(
+ "SELECT INTERVAL -'2 months'",
+ "SELECT INTERVAL - '2 months'",
+ );
+ dialects.verified_stmt(
+ "SELECT sum(a) OVER (ORDER BY ts RANGE BETWEEN '1 day' PRECEDING AND
CURRENT ROW) FROM t",
+ );
+ assert_eq!(
+ dialects
+ .parse_sql_statements("SELECT INTERVAL '1' DAY TO")
+ .unwrap_err()
+ .to_string(),
+ "sql parser error: Expected: date/time field, found: EOF"
Review Comment:
You should assert the error on `SELECT INTERVAL '1' DAY TO x` instead. Spark
4.0.1 accepts `SELECT INTERVAL '1' DAY TO` as `INTERVAL '1' DAY AS TO`, so this
assertion pins a rejection of valid Spark SQL. `TO x` is a syntax error in
Spark 4.0.1 too and still fails if the string arms of the `INTERVAL` guard are
removed.
```suggestion
.parse_sql_statements("SELECT INTERVAL '1' DAY TO x")
.unwrap_err()
.to_string(),
"sql parser error: Expected: date/time field, found: x"
```
##########
tests/sqlparser_common.rs:
##########
@@ -6764,9 +6766,87 @@ fn parse_interval_require_unit() {
)
}
+#[test]
+fn parse_interval_string_without_qualifier() {
+ let dialects = all_dialects_where(|d|
d.supports_interval_string_without_qualifier());
+ for sql in [
+ "SELECT INTERVAL '2 months'",
+ "SELECT INTERVAL '1' DAY",
+ "SELECT INTERVAL '1-2' YEAR TO MONTH",
+ "SELECT INTERVAL -'1' DAY",
+ "SELECT INTERVAL 3 DAY",
+ "SELECT INTERVAL -1 DAY, INTERVAL +2 HOURS",
+ "SELECT INTERVAL '2 seconds' * 2",
+ "SELECT INTERVAL \"2 months\"",
+ "SELECT INTERVAL \"1\" DAY",
+ ] {
+ dialects.verified_stmt(sql);
+ }
+ for sql in [
+ "SELECT INTERVAL 1",
+ "SELECT INTERVAL 1 + 1 DAY",
+ "SELECT INTERVAL x DAY",
+ ] {
+ assert!(
+ dialects.parse_sql_statements(sql).is_err(),
+ "{sql} should not parse"
+ );
+ }
+ dialects.verified_stmt("SELECT interval, x FROM t");
+ dialects.verified_stmt("SELECT max(interval) FROM t WHERE interval > 1");
+ dialects.verified_stmt("SELECT interval - 1, interval + 1 FROM t");
+ dialects.one_statement_parses_to("SELECT INTERVAL -1", "SELECT INTERVAL -
1");
+ dialects.one_statement_parses_to(
+ "SELECT INTERVAL -'2 months'",
+ "SELECT INTERVAL - '2 months'",
+ );
+ dialects.verified_stmt(
+ "SELECT sum(a) OVER (ORDER BY ts RANGE BETWEEN '1 day' PRECEDING AND
CURRENT ROW) FROM t",
+ );
+ assert_eq!(
+ dialects
+ .parse_sql_statements("SELECT INTERVAL '1' DAY TO")
+ .unwrap_err()
+ .to_string(),
+ "sql parser error: Expected: date/time field, found: EOF"
+ );
+}
+
+#[test]
+fn parse_window_frame_bound_string_as_interval() {
+ let dialects = all_dialects_where(|d|
!d.supports_interval_string_without_qualifier());
+ dialects.one_statement_parses_to(
+ "SELECT sum(a) OVER (ORDER BY ts RANGE BETWEEN '1' DAY PRECEDING AND
CURRENT ROW) FROM t",
+ "SELECT sum(a) OVER (ORDER BY ts RANGE BETWEEN INTERVAL '1' DAY
PRECEDING AND CURRENT ROW) FROM t",
+ );
+}
+
+#[test]
+fn parse_interval_number_without_unit_error() {
+ for dialect in all_dialects().dialects {
+ if let Err(e) = Parser::parse_sql(&*dialect, "SELECT INTERVAL 1") {
+ assert_eq!(
+ e.to_string(),
+ "sql parser error: INTERVAL requires a unit after the literal
value",
+ "{dialect:?}"
+ );
+ }
+ }
+}
+
+#[test]
+fn parse_interval_expression_value() {
+ let dialects = all_dialects_where(|d| {
+ d.require_interval_qualifier() &&
!d.supports_interval_string_without_qualifier()
+ });
+ dialects.verified_stmt("SELECT INTERVAL 1 + 1 DAY");
+}
Review Comment:
You should drop this test, since `parse_interval_require_qualifier` right
below already runs `verified_only_select` on `SELECT INTERVAL 1 + 1 DAY` over
the same dialects.
```suggestion
```
##########
tests/sqlparser_common.rs:
##########
@@ -6764,9 +6766,87 @@ fn parse_interval_require_unit() {
)
}
+#[test]
+fn parse_interval_string_without_qualifier() {
+ let dialects = all_dialects_where(|d|
d.supports_interval_string_without_qualifier());
+ for sql in [
+ "SELECT INTERVAL '2 months'",
+ "SELECT INTERVAL '1' DAY",
+ "SELECT INTERVAL '1-2' YEAR TO MONTH",
+ "SELECT INTERVAL -'1' DAY",
+ "SELECT INTERVAL 3 DAY",
+ "SELECT INTERVAL -1 DAY, INTERVAL +2 HOURS",
+ "SELECT INTERVAL '2 seconds' * 2",
+ "SELECT INTERVAL \"2 months\"",
+ "SELECT INTERVAL \"1\" DAY",
+ ] {
+ dialects.verified_stmt(sql);
+ }
+ for sql in [
+ "SELECT INTERVAL 1",
+ "SELECT INTERVAL 1 + 1 DAY",
+ "SELECT INTERVAL x DAY",
+ ] {
+ assert!(
+ dialects.parse_sql_statements(sql).is_err(),
+ "{sql} should not parse"
+ );
+ }
+ dialects.verified_stmt("SELECT interval, x FROM t");
+ dialects.verified_stmt("SELECT max(interval) FROM t WHERE interval > 1");
+ dialects.verified_stmt("SELECT interval - 1, interval + 1 FROM t");
+ dialects.one_statement_parses_to("SELECT INTERVAL -1", "SELECT INTERVAL -
1");
+ dialects.one_statement_parses_to(
+ "SELECT INTERVAL -'2 months'",
+ "SELECT INTERVAL - '2 months'",
+ );
+ dialects.verified_stmt(
+ "SELECT sum(a) OVER (ORDER BY ts RANGE BETWEEN '1 day' PRECEDING AND
CURRENT ROW) FROM t",
+ );
+ assert_eq!(
+ dialects
+ .parse_sql_statements("SELECT INTERVAL '1' DAY TO")
+ .unwrap_err()
+ .to_string(),
+ "sql parser error: Expected: date/time field, found: EOF"
+ );
+}
+
+#[test]
+fn parse_window_frame_bound_string_as_interval() {
+ let dialects = all_dialects_where(|d|
!d.supports_interval_string_without_qualifier());
+ dialects.one_statement_parses_to(
+ "SELECT sum(a) OVER (ORDER BY ts RANGE BETWEEN '1' DAY PRECEDING AND
CURRENT ROW) FROM t",
+ "SELECT sum(a) OVER (ORDER BY ts RANGE BETWEEN INTERVAL '1' DAY
PRECEDING AND CURRENT ROW) FROM t",
+ );
+}
+
+#[test]
+fn parse_interval_number_without_unit_error() {
+ for dialect in all_dialects().dialects {
+ if let Err(e) = Parser::parse_sql(&*dialect, "SELECT INTERVAL 1") {
+ assert_eq!(
+ e.to_string(),
+ "sql parser error: INTERVAL requires a unit after the literal
value",
+ "{dialect:?}"
+ );
+ }
+ }
+}
+
Review Comment:
You should drop this test. It skips every dialect that accepts `SELECT
INTERVAL 1`, and for Spark and Databricks `test_interval_numeric_requires_unit`
already asserts the same message.
```suggestion
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
