You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@datafusion.apache.org by "alexander-beedie (via GitHub)" <gi...@apache.org> on 2026/01/08 14:22:13 UTC

[PR] Fix identifier parsing not breaking on the pipe operator [datafusion-sqlparser-rs]

alexander-beedie opened a new pull request, #2156:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2156

   Fixes a bug in Pipe SQL parsing with identifiers followed by the `|>` operator; `parse_identifiers` needed to break on  `Token::VerticalBarRightAngleBracket` as well, otherwise it continues to consume the following tokens as `Ident`.
   
   ## Example
   ```sql
   FROM t |> DROP c |> RENAME a AS x
   ```
   Without the fix, the query after the "FROM" section gets incorrectly parsed as:
   ```rust
   Drop { columns: [
       Ident { value: "c", ... },
       Ident { value: "RENAME", ... },
       Ident { value: "a", ... },
       Ident { value: "AS", ... },
       Ident { value: "x", ... }]
   }
   ```
   After the fix the `|>`-separated sections are parsed correctly:
   ```
   Drop { columns: [Ident { value: "c" }] }
   Rename { mappings: [
       IdentWithAlias { ident: Ident { value: "a" }, alias: Ident { value: "x" } }]
   }
   ```
   
   ## Also
   
   Updated the pipe operator tests with some new coverage for the above, and a helper that additionally validates queries that start "FROM tbl" with the pipe operator (which was missing).


-- 
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-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] Fix identifier parsing not breaking on the `|>` pipe operator [datafusion-sqlparser-rs]

Posted by "iffyio (via GitHub)" <gi...@apache.org>.

iffyio commented on code in PR #2156:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2156#discussion_r2716270817


##########
tests/sqlparser_common.rs:
##########
@@ -16157,302 +16157,411 @@ fn parse_set_names() {
 
 #[test]
 fn parse_pipeline_operator() {
-    let dialects = all_dialects_where(|d| d.supports_pipe_operator());
-
-    // select pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> SELECT id");
-    dialects.verified_stmt("SELECT * FROM users |> SELECT id, name");
-    dialects.verified_query_with_canonical(
-        "SELECT * FROM users |> SELECT id user_id",
-        "SELECT * FROM users |> SELECT id AS user_id",
-    );
-    dialects.verified_stmt("SELECT * FROM users |> SELECT id AS user_id");
-
-    // extend pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> EXTEND id + 1 AS new_id");
-    dialects.verified_stmt("SELECT * FROM users |> EXTEND id AS new_id, name AS new_name");
-    dialects.verified_query_with_canonical(
-        "SELECT * FROM users |> EXTEND id user_id",
-        "SELECT * FROM users |> EXTEND id AS user_id",
-    );
+    // Helper for testing pipe operators
+    struct PipeTest<'a> {
+        dialects: &'a TestedDialects,
+        from_first: bool,
+    }
 
-    // set pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> SET id = id + 1");
-    dialects.verified_stmt("SELECT * FROM users |> SET id = id + 1, name = name + ' Doe'");
+    impl PipeTest<'_> {
+        fn query(&self, table: &str, pipe_ops: &str) -> String {
+            if self.from_first {
+                format!("FROM {table} |> {pipe_ops}")
+            } else {
+                format!("SELECT * FROM {table} |> {pipe_ops}")
+            }
+        }
 
-    // drop pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> DROP id");
-    dialects.verified_stmt("SELECT * FROM users |> DROP id, name");
+        fn pipe(&self, table: &str, pipe_ops: &str) {
+            self.dialects.verified_stmt(&self.query(table, pipe_ops));
+        }
 
-    // as pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> AS new_users");
+        fn pipe_canonical(&self, table: &str, input: &str, canonical: &str) {
+            self.dialects.verified_query_with_canonical(
+                &self.query(table, input),
+                &self.query(table, canonical),
+            );
+        }
 
-    // limit pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> LIMIT 10");
-    dialects.verified_stmt("SELECT * FROM users |> LIMIT 10 OFFSET 5");
-    dialects.verified_stmt("SELECT * FROM users |> LIMIT 10 |> LIMIT 5");
-    dialects.verified_stmt("SELECT * FROM users |> LIMIT 10 |> WHERE true");
+        fn pipe_implicit_alias(&self, table: &str, expr: &str, alias: &str) {
+            // Test implicit alias syntax is canonicalized to explicit AS
+            // e.g., "SELECT id user_id" -> "SELECT id AS user_id"
+            self.dialects.verified_query_with_canonical(
+                &self.query(table, &format!("{expr} {alias}")),
+                &self.query(table, &format!("{expr} AS {alias}")),
+            );
+        }
+    }
 
-    // where pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> WHERE id = 1");
-    dialects.verified_stmt("SELECT * FROM users |> WHERE id = 1 AND name = 'John'");
-    dialects.verified_stmt("SELECT * FROM users |> WHERE id = 1 OR name = 'John'");
+    // Test pipe operators with two dialect configurations:
+    // 1. Dialects supporting FROM-first syntax (e.g., "FROM users |> ...")
+    // 2. Dialects requiring SELECT-first syntax (e.g., "SELECT * FROM users |> ...")
+    let from_first_dialects =
+        all_dialects_where(|d| d.supports_pipe_operator() && d.supports_from_first_select());
+    let select_first_dialects =
+        all_dialects_where(|d| d.supports_pipe_operator() && !d.supports_from_first_select());
 
-    // aggregate pipe operator full table
-    dialects.verified_stmt("SELECT * FROM users |> AGGREGATE COUNT(*)");
-    dialects.verified_query_with_canonical(
-        "SELECT * FROM users |> AGGREGATE COUNT(*) total_users",
-        "SELECT * FROM users |> AGGREGATE COUNT(*) AS total_users",
-    );
-    dialects.verified_stmt("SELECT * FROM users |> AGGREGATE COUNT(*) AS total_users");
-    dialects.verified_stmt("SELECT * FROM users |> AGGREGATE COUNT(*), MIN(id)");
+    // Run tests for both dialect configurations
+    for (dialects, from_first) in [
+        (&from_first_dialects, true),
+        (&select_first_dialects, false),
+    ] {
+        let t = PipeTest {
+            dialects,
+            from_first,
+        };
 
-    // aggregate pipe opeprator with grouping
-    dialects.verified_stmt(
-        "SELECT * FROM users |> AGGREGATE SUM(o_totalprice) AS price, COUNT(*) AS cnt GROUP BY EXTRACT(YEAR FROM o_orderdate) AS year",
-    );
-    dialects.verified_stmt(
-        "SELECT * FROM users |> AGGREGATE GROUP BY EXTRACT(YEAR FROM o_orderdate) AS year",
-    );
-    dialects
-        .verified_stmt("SELECT * FROM users |> AGGREGATE GROUP BY EXTRACT(YEAR FROM o_orderdate)");
-    dialects.verified_stmt("SELECT * FROM users |> AGGREGATE GROUP BY a, b");
-    dialects.verified_stmt("SELECT * FROM users |> AGGREGATE SUM(c) GROUP BY a, b");
-    dialects.verified_stmt("SELECT * FROM users |> AGGREGATE SUM(c) ASC");
-
-    // order by pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> ORDER BY id ASC");
-    dialects.verified_stmt("SELECT * FROM users |> ORDER BY id DESC");
-    dialects.verified_stmt("SELECT * FROM users |> ORDER BY id DESC, name ASC");
-
-    // tablesample pipe operator
-    dialects.verified_stmt("SELECT * FROM tbl |> TABLESAMPLE BERNOULLI (50)");
-    dialects.verified_stmt("SELECT * FROM tbl |> TABLESAMPLE SYSTEM (50 PERCENT)");
-    dialects.verified_stmt("SELECT * FROM tbl |> TABLESAMPLE SYSTEM (50) REPEATABLE (10)");
-
-    // rename pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> RENAME old_name AS new_name");
-    dialects.verified_stmt("SELECT * FROM users |> RENAME id AS user_id, name AS user_name");
-    dialects.verified_query_with_canonical(
-        "SELECT * FROM users |> RENAME id user_id",
-        "SELECT * FROM users |> RENAME id AS user_id",
-    );
-
-    // union pipe operator
-    dialects.verified_stmt("SELECT * FROM users |> UNION ALL (SELECT * FROM admins)");
-    dialects.verified_stmt("SELECT * FROM users |> UNION DISTINCT (SELECT * FROM admins)");
-    dialects.verified_stmt("SELECT * FROM users |> UNION (SELECT * FROM admins)");
-
-    // union pipe operator with multiple queries
-    dialects.verified_stmt(
-        "SELECT * FROM users |> UNION ALL (SELECT * FROM admins), (SELECT * FROM guests)",
-    );
-    dialects.verified_stmt("SELECT * FROM users |> UNION DISTINCT (SELECT * FROM admins), (SELECT * FROM guests), (SELECT * FROM employees)");
-    dialects.verified_stmt(
-        "SELECT * FROM users |> UNION (SELECT * FROM admins), (SELECT * FROM guests)",
-    );
+        // 'select' pipe operator

Review Comment:
   if its possible can we use the previous format of explicitly writing out the test cases and expected results? I imagine that might be more verbose but its useful readability wise to know the cases that are being covered and what the tests do at a glance



-- 
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-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org