iffyio commented on code in PR #1860:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/1860#discussion_r2110927693
##########
src/ast/query.rs:
##########
@@ -2680,28 +2680,32 @@ pub enum PipeOperator {
full_table_exprs: Vec<ExprWithAliasAndOrderBy>,
group_by_expr: Vec<ExprWithAliasAndOrderBy>,
},
+ /// Selects a random sample of rows from the input table.
+ /// Syntax: `|> TABLESAMPLE <method> (<size> {ROWS | PERCENT})`
+ /// See more at
<https://cloud.google.com/bigquery/docs/reference/standard-sql/pipe-syntax#tablesample_pipe_operator>
+ TableSample { sample: Box <TableSample> },
}
impl fmt::Display for PipeOperator {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
PipeOperator::Select { exprs } => {
- write!(f, "SELECT {}",
display_comma_separated(exprs.as_slice()))
+ write!(f, " SELECT {}",
display_comma_separated(exprs.as_slice()))
Review Comment:
Oh I think we'd ideally undo these changes, its better for the operator
display to be standalone where possible (i.e. they shouldn't assume surrounding
space formatting, rather it should be left up to the caller), it makes it
easier to compose nodes in the AST when displaying
##########
src/ast/query.rs:
##########
@@ -2680,28 +2680,32 @@ pub enum PipeOperator {
full_table_exprs: Vec<ExprWithAliasAndOrderBy>,
group_by_expr: Vec<ExprWithAliasAndOrderBy>,
},
+ /// Selects a random sample of rows from the input table.
+ /// Syntax: `|> TABLESAMPLE <method> (<size> {ROWS | PERCENT})`
Review Comment:
I think the doc comment doesn't necessarily need to spell out the full spec.
usually its enough to give a rough example of what the statement looks like e.g.
```sql
Syntax: `|> TABLESAMPLE BERNOULLI(50)
```
##########
tests/sqlparser_common.rs:
##########
@@ -15155,6 +15155,12 @@ fn parse_pipeline_operator() {
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)");
+ // TODO: Technically, REPEATABLE is not available in BigQuery, but it is
used with TABLESAMPLE in other dialects
Review Comment:
```suggestion
```
Yeah I think this behavior would be fine
--
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-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
