From ebcd31e6b1ffa7fd282f179f573215036a04b57e Mon Sep 17 00:00:00 2001
From: Nick Presta <nick.presta@shopify.com>
Date: Fri, 5 Jul 2024 14:56:50 -0400
Subject: [PATCH] Add tests for invalid syntax

---
 src/parser/mod.rs             | 33 +++++++++++++++++-
 tests/sqlparser_clickhouse.rs | 64 +++++++++++++++++++++++++++++++++--
 2 files changed, 94 insertions(+), 3 deletions(-)

diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index 05b449205..f8ba93be3 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -7888,7 +7888,9 @@ impl<'a> Parser<'a> {
             let body = self.parse_boxed_query_body(0)?;
 
             let order_by = if self.parse_keywords(&[Keyword::ORDER, Keyword::BY]) {
-                self.parse_comma_separated(Parser::parse_order_by_expr)?
+                let order_by_exprs = self.parse_comma_separated(Parser::parse_order_by_expr)?;
+                self.validate_order_by_exprs_for_interpolate_and_with_fill(&order_by_exprs)?;
+                order_by_exprs
             } else {
                 vec![]
             };
@@ -10465,6 +10467,35 @@ impl<'a> Parser<'a> {
         Ok(WithFill { from, to, step })
     }
 
+    pub fn validate_order_by_exprs_for_interpolate_and_with_fill(
+        &mut self,
+        order_by_exprs: &Vec<OrderByExpr>,
+    ) -> Result<(), ParserError> {
+        if dialect_of!(self is ClickHouseDialect | GenericDialect) {
+            let mut has_with_fill = false;
+            let mut has_interpolate = false;
+            for order_by_expr in order_by_exprs {
+                if order_by_expr.with_fill.is_some() {
+                    has_with_fill = true;
+                }
+                if order_by_expr.interpolate.is_some() {
+                    if has_interpolate {
+                        return Err(ParserError::ParserError(
+                            "Only the last ORDER BY expression can contain interpolate".to_string(),
+                        ));
+                    }
+                    if !has_with_fill {
+                        return Err(ParserError::ParserError(
+                            "INTERPOLATE requires WITH FILL".to_string(),
+                        ));
+                    }
+                    has_interpolate = true;
+                }
+            }
+        }
+        Ok(())
+    }
+
     // Parse a set of comma seperated INTERPOLATE expressions (ClickHouse dialect)
     // that follow the INTERPOLATE keyword in an ORDER BY clause with the WITH FILL modifier
     pub fn parse_interpolations(&mut self) -> Result<Vec<Interpolate>, ParserError> {
diff --git a/tests/sqlparser_clickhouse.rs b/tests/sqlparser_clickhouse.rs
index 10a74456c..fe5d9a6f0 100644
--- a/tests/sqlparser_clickhouse.rs
+++ b/tests/sqlparser_clickhouse.rs
@@ -727,10 +727,51 @@ fn parse_select_order_by_with_fill_interpolate() {
     assert_eq!(Some(Expr::Value(number("2"))), select.limit);
 }
 
+#[test]
+fn parse_select_order_by_with_fill_interpolate_multi_interpolates() {
+    let sql = "SELECT id, fname, lname FROM customer ORDER BY fname WITH FILL \
+        INTERPOLATE (col1 AS col1 + 1) INTERPOLATE (col2 AS col2 + 2)";
+    clickhouse_and_generic()
+        .parse_sql_statements(sql)
+        .expect_err("ORDER BY only accepts a single INTERPOLATE clause");
+}
+
+#[test]
+fn parse_select_order_by_with_fill_interpolate_multi_with_fill_interpolates() {
+    let sql = "SELECT id, fname, lname FROM customer \
+        ORDER BY \
+            fname WITH FILL INTERPOLATE (col1 AS col1 + 1), \
+            lname WITH FILL INTERPOLATE (col2 AS col2 + 2)";
+    clickhouse_and_generic()
+        .parse_sql_statements(sql)
+        .expect_err("ORDER BY only accepts a single INTERPOLATE clause");
+}
+
+#[test]
+fn parse_select_order_by_interpolate_missing_with_fill() {
+    let sql = "SELECT id, fname, lname FROM customer \
+        ORDER BY fname, lname \
+            INTERPOLATE (col2 AS col2 + 2)";
+    clickhouse_and_generic()
+        .parse_sql_statements(sql)
+        .expect_err("ORDER BY INTERPOLATE must have at least one WITH FILL");
+}
+
+#[test]
+fn parse_select_order_by_interpolate_not_last() {
+    let sql = "SELECT id, fname, lname FROM customer \
+        ORDER BY \
+            fname INTERPOLATE (col2 AS col2 + 2),
+            lname";
+    clickhouse_and_generic()
+        .parse_sql_statements(sql)
+        .expect_err("ORDER BY INTERPOLATE must be in the last position");
+}
+
 #[test]
 fn parse_with_fill() {
-    let sql = "SELECT fname FROM customer \
-        ORDER BY fname WITH FILL FROM 10 TO 20 STEP 2";
+    let sql = "SELECT fname FROM customer ORDER BY fname \
+        WITH FILL FROM 10 TO 20 STEP 2";
     let select = clickhouse().verified_query(sql);
     assert_eq!(
         Some(WithFill {
@@ -742,6 +783,25 @@ fn parse_with_fill() {
     );
 }
 
+#[test]
+fn parse_with_fill_missing_single_argument() {
+    let sql = "SELECT id, fname, lname FROM customer ORDER BY \
+            fname WITH FILL FROM TO 20";
+    clickhouse_and_generic()
+        .parse_sql_statements(sql)
+        .expect_err("WITH FILL requires expressions for all arguments");
+}
+
+#[test]
+fn parse_with_fill_multiple_incomplete_arguments() {
+    let sql = "SELECT id, fname, lname FROM customer ORDER BY \
+            fname WITH FILL FROM TO 20, lname WITH FILL FROM TO STEP 1";
+    clickhouse_and_generic()
+        .parse_sql_statements(sql)
+        .expect_err("WITH FILL requires expressions for all arguments");
+}
+
+
 #[test]
 fn parse_interpolate_body_with_columns() {
     let sql = "SELECT fname FROM customer ORDER BY fname WITH FILL \