fix(redshift): fix INTERVAL quoting (#5545)

Signed-off-by: Luka Peschke <mail@lukapeschke.com>

Author: lukapeschke

prqlc/prqlc/src/sql/dialect.rs

@@ -16,6 +16,7 @@ use std::any::{Any, TypeId};
 
 use chrono::format::{Fixed, Item, Numeric, Pad, StrftimeItems};
 use serde::{Deserialize, Serialize};
+use sqlparser::ast::DateTimeField;
 use strum::VariantNames;
 
 use crate::{Error, Result};
@@ -141,6 +142,16 @@ pub enum IdentQuotingStyle {
     ConditionallyQuoted,
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum IntervalQuotingStyle {
+    // INTERVAL 1 day
+    NoQuotes,
+    // INTERVAL '1' day
+    ValueQuoted,
+    // INTERVAL '1 day'
+    ValueAndUnitQuoted,
+}
+
 pub(super) trait DialectHandler: Any + Debug {
     fn use_fetch(&self) -> bool {
         false
@@ -181,9 +192,9 @@ pub(super) trait DialectHandler: Any + Debug {
     }
 
     /// Whether or not intervals such as `INTERVAL 1 HOUR` require quotes like
-    /// `INTERVAL '1 HOUR'`
-    fn requires_quotes_intervals(&self) -> bool {
-        false
+    /// `INTERVAL '1 HOUR'` or `INTERVAL '1' HOUR`
+    fn interval_quoting_style(&self, _dtf: &DateTimeField) -> IntervalQuotingStyle {
+        IntervalQuotingStyle::NoQuotes
     }
 
     /// Support for GROUP BY *
@@ -249,8 +260,8 @@ impl DialectHandler for GenericDialect {
 }
 
 impl DialectHandler for PostgresDialect {
-    fn requires_quotes_intervals(&self) -> bool {
-        true
+    fn interval_quoting_style(&self, _dtf: &DateTimeField) -> IntervalQuotingStyle {
+        IntervalQuotingStyle::ValueAndUnitQuoted
     }
 
     fn supports_distinct_on(&self) -> bool {
@@ -314,8 +325,12 @@ impl DialectHandler for RedshiftDialect {
         IdentQuotingStyle::ConditionallyQuoted
     }
 
-    fn requires_quotes_intervals(&self) -> bool {
-        true
+    fn interval_quoting_style(&self, dtf: &DateTimeField) -> IntervalQuotingStyle {
+        if matches!(dtf, DateTimeField::Week(_) | DateTimeField::Weeks) {
+            IntervalQuotingStyle::ValueAndUnitQuoted
+        } else {
+            IntervalQuotingStyle::ValueQuoted
+        }
     }
 
     fn supports_distinct_on(&self) -> bool {
@@ -380,8 +395,8 @@ impl DialectHandler for RedshiftDialect {
 }
 
 impl DialectHandler for GlareDbDialect {
-    fn requires_quotes_intervals(&self) -> bool {
-        true
+    fn interval_quoting_style(&self, _dtf: &DateTimeField) -> IntervalQuotingStyle {
+        IntervalQuotingStyle::ValueAndUnitQuoted
     }
 }
 

prqlc/prqlc/src/sql/gen_expr.rs

@@ -16,7 +16,7 @@ use super::{keywords, Context};
 use crate::ir::generic::{ColumnSort, SortDirection, WindowFrame, WindowKind};
 use crate::ir::pl::{self, Ident, Literal};
 use crate::ir::rq;
-use crate::sql::dialect::IdentQuotingStyle;
+use crate::sql::dialect::{IdentQuotingStyle, IntervalQuotingStyle};
 use crate::sql::pq::context::ColumnDecl;
 use crate::utils::{valid_ident, OrMap};
 use crate::{Error, Result, Span, WithErrorInfo};
@@ -428,27 +428,46 @@ pub(super) fn translate_literal(l: Literal, ctx: &Context) -> Result<sql_ast::Ex
                     )))
                 }
             };
-            if ctx.dialect.requires_quotes_intervals() {
-                //postgres requires quotes around number and unit together eg '3 WEEK'
-                let value = Box::new(sql_ast::Expr::Value(
-                    Value::SingleQuotedString(format!("{} {}", vau.n, sql_parser_datetime)).into(),
-                ));
-                sql_ast::Expr::Interval(sqlparser::ast::Interval {
-                    value,
-                    leading_field: None, //set to none since field is now contained in string
-                    leading_precision: None,
-                    last_field: None,
-                    fractional_seconds_precision: None,
-                })
-            } else {
-                let value = Box::new(translate_literal(Literal::Integer(vau.n), ctx)?);
-                sql_ast::Expr::Interval(sqlparser::ast::Interval {
-                    value,
-                    leading_field: Some(sql_parser_datetime),
-                    leading_precision: None,
-                    last_field: None,
-                    fractional_seconds_precision: None,
-                })
+            match ctx.dialect.interval_quoting_style(&sql_parser_datetime) {
+                IntervalQuotingStyle::ValueAndUnitQuoted => {
+                    //postgres requires quotes around number and unit together eg '3 WEEK'
+                    let value = Box::new(sql_ast::Expr::Value(
+                        Value::SingleQuotedString(format!("{} {}", vau.n, sql_parser_datetime))
+                            .into(),
+                    ));
+                    sql_ast::Expr::Interval(sqlparser::ast::Interval {
+                        value,
+                        leading_field: None, //set to none since field is now contained in string
+                        leading_precision: None,
+                        last_field: None,
+                        fractional_seconds_precision: None,
+                    })
+                }
+                IntervalQuotingStyle::NoQuotes => {
+                    let value = Box::new(translate_literal(Literal::Integer(vau.n), ctx)?);
+                    sql_ast::Expr::Interval(sqlparser::ast::Interval {
+                        value,
+                        leading_field: Some(sql_parser_datetime),
+                        leading_precision: None,
+                        last_field: None,
+                        fractional_seconds_precision: None,
+                    })
+                }
+                // Redshift requires quotes around the number only, otherwise months and years are
+                // not supported. eg '3' MONTH
+                IntervalQuotingStyle::ValueQuoted => {
+                    // Adding single quotes around the number
+                    let value = Box::new(sql_ast::Expr::Value(
+                        Value::SingleQuotedString(vau.n.to_string()).into(),
+                    ));
+                    sql_ast::Expr::Interval(sqlparser::ast::Interval {
+                        value,
+                        leading_field: Some(sql_parser_datetime),
+                        leading_precision: None,
+                        last_field: None,
+                        fractional_seconds_precision: None,
+                    })
+                }
             }
         }
     })

prqlc/prqlc/tests/integration/sql.rs

@@ -6380,6 +6380,26 @@ fn test_redshift_uses_double_pipe_over_concat() {
     ");
 }
 
+#[rstest]
+#[case(
+    "from t | select { inter = 2weeks }",
+    "SELECT\n  INTERVAL '2 WEEK' AS inter\nFROM\n  t\n"
+)]
+#[case(
+    "from t | select { inter = 2months }",
+    "SELECT\n  INTERVAL '2' MONTH AS inter\nFROM\n  t\n"
+)]
+#[case(
+    "from t | select { inter = 2hours }",
+    "SELECT\n  INTERVAL '2' HOUR AS inter\nFROM\n  t\n"
+)]
+fn test_redshift_interval_quoting(#[case] query: &str, #[case] expected: &str) {
+    assert_eq!(
+        compile_with_sql_dialect(query, sql::Dialect::Redshift).unwrap(),
+        expected
+    )
+}
+
 #[test]
 fn test_redshift_text_contains_uses_double_pipe() {
     assert_snapshot!(compile_with_sql_dialect(r###"