-
Notifications
You must be signed in to change notification settings - Fork 633
Add support for TABLESAMPLE
pipe operator
#1860
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
4739c45
caacef8
b08d9b1
9b6157e
ed4cf2f
c2d713f
89f51d3
2e33f91
075484a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,7 +104,7 @@ impl fmt::Display for Query { | |
format.fmt(f)?; | ||
} | ||
for pipe_operator in &self.pipe_operators { | ||
f.write_str(" |> ")?; | ||
f.write_str(" |>")?; | ||
pipe_operator.fmt(f)?; | ||
} | ||
Ok(()) | ||
|
@@ -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())) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I found a way to fix the formatting for |
||
} | ||
PipeOperator::Extend { exprs } => { | ||
write!(f, "EXTEND {}", display_comma_separated(exprs.as_slice())) | ||
write!(f, " EXTEND {}", display_comma_separated(exprs.as_slice())) | ||
} | ||
PipeOperator::Set { assignments } => { | ||
write!(f, "SET {}", display_comma_separated(assignments.as_slice())) | ||
write!(f, " SET {}", display_comma_separated(assignments.as_slice())) | ||
} | ||
PipeOperator::Drop { columns } => { | ||
write!(f, "DROP {}", display_comma_separated(columns.as_slice())) | ||
write!(f, " DROP {}", display_comma_separated(columns.as_slice())) | ||
} | ||
PipeOperator::As { alias } => { | ||
write!(f, "AS {}", alias) | ||
write!(f, " AS {}", alias) | ||
} | ||
PipeOperator::Limit { expr, offset } => { | ||
write!(f, "LIMIT {}", expr)?; | ||
write!(f, " LIMIT {}", expr)?; | ||
if let Some(offset) = offset { | ||
write!(f, " OFFSET {}", offset)?; | ||
} | ||
|
@@ -2711,7 +2715,7 @@ impl fmt::Display for PipeOperator { | |
full_table_exprs, | ||
group_by_expr, | ||
} => { | ||
write!(f, "AGGREGATE")?; | ||
write!(f, " AGGREGATE")?; | ||
if !full_table_exprs.is_empty() { | ||
write!( | ||
f, | ||
|
@@ -2726,10 +2730,14 @@ impl fmt::Display for PipeOperator { | |
} | ||
|
||
PipeOperator::Where { expr } => { | ||
write!(f, "WHERE {}", expr) | ||
write!(f, " WHERE {}", expr) | ||
} | ||
PipeOperator::OrderBy { exprs } => { | ||
write!(f, "ORDER BY {}", display_comma_separated(exprs.as_slice())) | ||
write!(f, " ORDER BY {}", display_comma_separated(exprs.as_slice())) | ||
} | ||
|
||
PipeOperator::TableSample { sample } => { | ||
write!(f, "{}", sample) | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the officially supported spec from the paper but doesn't cover everything that is technically supported in this PR. I'm wondering how to best deal with that (see the open question in the PR description).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Syntax: `|> TABLESAMPLE BERNOULLI(50)