Support some of pipe operators #1759
Conversation
Part of apache#1758 Still missing - join - union|intersect|except - call - tablesample - pivot - unpivot
There was a problem hiding this comment.
Thanks @simonvandel! The changes look reasonable to me overall, left some comments
| #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] | ||
| #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] | ||
| #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] | ||
| pub enum PipeOperator { |
There was a problem hiding this comment.
Can we add a description and link to the docs https://cloud.google.com/bigquery/docs/pipe-syntax-guide#basic_syntax
| Select { | ||
| exprs: Vec<SelectItem>, | ||
| }, | ||
| Extend { |
There was a problem hiding this comment.
it would be nice to include a short example doc string for the variants and where possible links to the documentation, like here for extended for example https://cloud.google.com/bigquery/docs/pipe-syntax-guide#extend-pipe-operator
| // AGGREGATE <agg_expr> [[AS] alias], ... | ||
| // | ||
| // and | ||
| // | ||
| // AGGREGATE [<agg_expr> [[AS] alias], ...] | ||
| // GROUP BY <grouping_expr> [AS alias], ... |
There was a problem hiding this comment.
Ah so I think these comments would be better placed on in the doc strings for each variant (left similar comment at that site)
|
|
||
| // Syntax from "SQL Has Problems. We Can Fix Them: Pipe Syntax In SQL" | ||
| // https://storage.googleapis.com/gweb-research2023-media/pubtools/1004848.pdf | ||
| while self.consume_token(&Token::VerticalBarRightAngleBracket) { |
There was a problem hiding this comment.
can we move the logic to a dedicated function like parse_pipe_operators()?
| // Syntax from "SQL Has Problems. We Can Fix Them: Pipe Syntax In SQL" | ||
| // https://storage.googleapis.com/gweb-research2023-media/pubtools/1004848.pdf |
There was a problem hiding this comment.
| // Syntax from "SQL Has Problems. We Can Fix Them: Pipe Syntax In SQL" | |
| // https://storage.googleapis.com/gweb-research2023-media/pubtools/1004848.pdf |
| /// |> limit 1 | ||
| /// ``` | ||
| /// | ||
| /// See "SQL Has Problems. We Can Fix Them: Pipe Syntax In SQL" https://research.google/pubs/sql-has-problems-we-can-fix-them-pipe-syntax-in-sql/ |
There was a problem hiding this comment.
I think we can link to a concrete syntax like this instead
https://cloud.google.com/bigquery/docs/pipe-syntax-guide#basic_syntax
The paper wouldn't really help as much in the context of someone working on the codebase or trying to use the library
| let exprs = self.parse_comma_separated(Parser::parse_order_by_expr)?; | ||
| pipe_operators.push(PipeOperator::OrderBy { exprs }) | ||
| } | ||
| _ => {} |
There was a problem hiding this comment.
This clause look unhandled, for the loop i imagine instead we can something like?
while self.consume_token(&Token::VerticalBarRightAngleBracket) {
operators.push(self.parse_pipe_operator()?);
}
fn parse_pipe_operator() {
if self.parse_keyword(SELECT) {
// select operator
} // ...
else {
self.expected("...")
}
}| 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"); | ||
| } |
There was a problem hiding this comment.
can we include a test case with more than one pipe operator? e.g. SELECT * FROM T |> a |> b |> c
Part of #1758
Still missing (ran out of time today, can be done in follow-ups perhaps)
I'm a first time contributor - please let me know if there are better places to store the
PipeOperatorin the AST