Repository: prql/prql (rust)

Issue:
Filter with a list returns an incorrect result
```elm
from employees
filter [age > 25, age < 40]
```

```elm
SELECT
  employees.*
FROM
  employees
WHERE
  age > 25 age < 40
```

(in this case we can use `in` & `..` but the principle holds)

---

I don't see much downside in disallowing lists and joining them with `and`, even though it's very easy to do two `filter`s:

```elm
from employees
filter age > 25
filter age < 40
```

Pull request:
Filter does not take lists
Closes #469

Patch:
diff --git a/prql-compiler/src/ast/ast_fold.rs b/prql-compiler/src/ast/ast_fold.rs
index 093a0ae8..88c664c8 100644
--- a/prql-compiler/src/ast/ast_fold.rs
+++ b/prql-compiler/src/ast/ast_fold.rs
@@ -183,7 +183,7 @@ pub fn fold_transform<T: ?Sized + AstFold>(
             by: fold.fold_nodes(by)?,
         }),
 
-        Transform::Filter(items) => Ok(Transform::Filter(fold.fold_nodes(items)?)),
+        Transform::Filter(f) => Ok(Transform::Filter(Box::new(fold.fold_node(*f)?))),
         Transform::Sort(items) => Ok(Transform::Sort(fold.fold_column_sorts(items)?)),
         Transform::Join { side, with, filter } => Ok(Transform::Join {
             side,
diff --git a/prql-compiler/src/ast/query.rs b/prql-compiler/src/ast/query.rs
index 1f2fb1e9..4939af08 100644
--- a/prql-compiler/src/ast/query.rs
+++ b/prql-compiler/src/ast/query.rs
@@ -33,7 +33,7 @@ pub struct Table {
 pub enum Transform {
     From(TableRef),
     Select(Vec<Node>),
-    Filter(Vec<Node>),
+    Filter(Box<Node>),
     Derive(Vec<Node>),
     Aggregate {
         assigns: Vec<Node>,
diff --git a/prql-compiler/src/semantic/transforms.rs b/prql-compiler/src/semantic/transforms.rs
index 9f6bb0fd..57525aad 100644
--- a/prql-compiler/src/semantic/transforms.rs
+++ b/prql-compiler/src/semantic/transforms.rs
@@ -29,7 +29,7 @@ pub fn cast_transform(func_call: FuncCall, span: Option<Span>) -> Result<Transfo
         "filter" => {
             let ([filter], []) = unpack(func_call, [])?;
 
-            Transform::Filter(filter.coerce_to_vec())
+            Transform::Filter(Box::new(filter))
         }
         "derive" => {
             let ([assigns], []) = unpack(func_call, [])?;
diff --git a/prql-compiler/src/sql/translator.rs b/prql-compiler/src/sql/translator.rs
index 0faa6ff7..e1b0cae9 100644
--- a/prql-compiler/src/sql/translator.rs
+++ b/prql-compiler/src/sql/translator.rs
@@ -204,18 +204,19 @@ fn sql_query_of_atomic_table(table: AtomicTable, dialect: &Dialect) -> Result<sq
     let (before, after) = transforms.split_at(aggregate_position);
 
     // Convert the filters in a pipeline into an Expr
+    #[allow(unstable_name_collisions)] // Same behavior as the std lib; we can remove this + itertools when that's released.
     fn filter_of_pipeline(pipeline: &[Transform]) -> Result<Option<Expr>> {
-        let filters: Vec<Vec<Node>> = pipeline
+        let filters: Vec<Node> = pipeline
             .iter()
             .filter_map(|t| match t {
-                Transform::Filter(filter) => Some(filter),
+                Transform::Filter(filter) => Some(*filter.clone()),
                 _ => None,
             })
-            .cloned()
+            .intersperse(Node::from(Item::Operator("and".to_owned())))
             .collect();
 
         Ok(if !filters.is_empty() {
-            Some((Item::Expr(combine_filters(filters))).try_into()?)
+            Some((Item::Expr(filters)).try_into()?)
         } else {
             None
         })
@@ -388,16 +389,6 @@ fn prepend_with_from(pipeline: &mut Pipeline, table: Option<TableRef>) {
     }
 }
 
-/// Combines filters by putting them in parentheses and then joining them with `and`.
-#[allow(unstable_name_collisions)] // Same behavior as the std lib; we can remove this + itertools when that's released.
-fn combine_filters(filters: Vec<Vec<Node>>) -> Vec<Node> {
-    filters
-        .into_iter()
-        .map(|f| Item::Expr(f).into())
-        .intersperse(Item::Raw("and".to_owned()).into())
-        .collect()
-}
-
 fn expr_of_i64(number: i64) -> Expr {
     Expr::Value(Value::Number(
         number.to_string(),
@@ -1463,4 +1454,47 @@ take 20
 
         Ok(())
     }
+
+    #[test]
+    fn test_filter() {
+        // https://github.com/prql/prql/issues/469
+        let query: Query = parse(
+            r###"
+        from employees
+        filter [age > 25, age < 40]
+        "###,
+        )
+        .unwrap();
+
+        assert!(resolve_and_translate(query).is_err());
+
+        assert_display_snapshot!((resolve_and_translate(parse(r###"
+        from employees
+        filter age > 25 and age < 40
+        "###,
+        ).unwrap()).unwrap()), @r###"
+        SELECT
+          employees.*
+        FROM
+          employees
+        WHERE
+          age > 25
+          AND age < 40
+        "###);
+
+        assert_display_snapshot!((resolve_and_translate(parse(r###"
+        from employees
+        filter age > 25
+        filter age < 40
+        "###,
+        ).unwrap()).unwrap()), @r###"
+        SELECT
+          employees.*
+        FROM
+          employees
+        WHERE
+          age > 25
+          AND age < 40
+        "###);
+    }
 }


Test patch:
diff --git a/prql-compiler/src/sql/snapshots/prql_compiler__sql__materializer__test__materialize_2.snap b/prql-compiler/src/sql/snapshots/prql_compiler__sql__materializer__test__materialize_2.snap
index d7d151a8..5b573253 100644
--- a/prql-compiler/src/sql/snapshots/prql_compiler__sql__materializer__test__materialize_2.snap
+++ b/prql-compiler/src/sql/snapshots/prql_compiler__sql__materializer__test__materialize_2.snap
@@ -9,22 +9,22 @@ expression: mat
       declared_at: 35
 - Transform:
     Filter:
-      - Expr:
-          - Ident: country
-          - Operator: "=="
-          - String: USA
+      Expr:
+        - Ident: country
+        - Operator: "=="
+        - String: USA
 - Transform:
     Filter:
-      - Expr:
-          - Expr:
-              - Expr:
-                  - Ident: salary
-                  - Operator: +
-                  - Ident: payroll_tax
-              - Operator: +
-              - Ident: benefits_cost
-          - Operator: ">"
-          - Raw: "0"
+      Expr:
+        - Expr:
+            - Expr:
+                - Ident: salary
+                - Operator: +
+                - Ident: payroll_tax
+            - Operator: +
+            - Ident: benefits_cost
+        - Operator: ">"
+        - Raw: "0"
 - Transform:
     Group:
       by:
@@ -92,20 +92,20 @@ expression: mat
                     - Ident: country
 - Transform:
     Filter:
-      - Expr:
-          - SString:
-              - String: SUM(
-              - Expr:
-                  Expr:
-                    - Expr:
-                        - Ident: salary
-                        - Operator: +
-                        - Ident: payroll_tax
-                    - Operator: +
-                    - Ident: benefits_cost
-              - String: )
-          - Operator: ">"
-          - Raw: "200"
+      Expr:
+        - SString:
+            - String: SUM(
+            - Expr:
+                Expr:
+                  - Expr:
+                      - Ident: salary
+                      - Operator: +
+                      - Ident: payroll_tax
+                  - Operator: +
+                  - Ident: benefits_cost
+            - String: )
+        - Operator: ">"
+        - Raw: "200"
 - Transform:
     Take: 20
 
diff --git a/prql-compiler/src/sql/snapshots/prql_compiler__sql__materializer__test__replace_variables_2.snap b/prql-compiler/src/sql/snapshots/prql_compiler__sql__materializer__test__replace_variables_2.snap
index 2cabb62b..7ed1a676 100644
--- a/prql-compiler/src/sql/snapshots/prql_compiler__sql__materializer__test__replace_variables_2.snap
+++ b/prql-compiler/src/sql/snapshots/prql_compiler__sql__materializer__test__replace_variables_2.snap
@@ -9,22 +9,22 @@ expression: "&mat"
       declared_at: 35
 - Transform:
     Filter:
-      - Expr:
-          - Ident: country
-          - Operator: "=="
-          - String: USA
+      Expr:
+        - Ident: country
+        - Operator: "=="
+        - String: USA
 - Transform:
     Filter:
-      - Expr:
-          - Expr:
-              - Expr:
-                  - Ident: salary
-                  - Operator: +
-                  - Ident: payroll_tax
-              - Operator: +
-              - Ident: benefits_cost
-          - Operator: ">"
-          - Raw: "0"
+      Expr:
+        - Expr:
+            - Expr:
+                - Ident: salary
+                - Operator: +
+                - Ident: payroll_tax
+            - Operator: +
+            - Ident: benefits_cost
+        - Operator: ">"
+        - Raw: "0"
 - Transform:
     Group:
       by:
@@ -92,20 +92,20 @@ expression: "&mat"
                     - Ident: country
 - Transform:
     Filter:
-      - Expr:
-          - SString:
-              - String: SUM(
-              - Expr:
-                  Expr:
-                    - Expr:
-                        - Ident: salary
-                        - Operator: +
-                        - Ident: payroll_tax
-                    - Operator: +
-                    - Ident: benefits_cost
-              - String: )
-          - Operator: ">"
-          - Raw: "200"
+      Expr:
+        - SString:
+            - String: SUM(
+            - Expr:
+                Expr:
+                  - Expr:
+                      - Ident: salary
+                      - Operator: +
+                      - Ident: payroll_tax
+                  - Operator: +
+                  - Ident: benefits_cost
+            - String: )
+        - Operator: ">"
+        - Raw: "200"
 - Transform:
     Take: 20