feat: add DROP pipe operator support#20864
Conversation
simonvandel
left a comment
There was a problem hiding this comment.
Very cool, thanks. The changes look good to me.
I'm not a committer, so someone else needs to do the final review.
@simonvandel, whom can I ask for the final review ? @alamb, does this look good? If so, I can pick either |
Jefffrey
left a comment
There was a problem hiding this comment.
Thanks for picking this up; just some minor comments on tests, and once it's merged up from main it should be good to go
| } | ||
| PipeOperator::Drop { columns } => { | ||
| let Some((first, rest)) = columns.split_first() else { | ||
| return plan_err!("DROP requires at least one column"); |
There was a problem hiding this comment.
Would be good to have a test case covering this
There was a problem hiding this comment.
I think the https://github.com/google/googlesql/blob/master/docs/pipe-syntax.md#drop-pipe-operator makes column_name required (|> DROP column_name [, ...]), which would make this a parse error rather than a planner error, and it feels like it belongs in sqlparser instead. Could you confirm I'm reading the spec right? If so, I'll need to fix it in the upstream crate first.
There was a problem hiding this comment.
I'm not familiar with the spec, just basing it off the code I see here (if we need to ensure columns aren't empty here then it means its possible to be empty? Or its just safety?)
| 3.3 | ||
|
|
||
| # Error: DROP non-existent column | ||
| statement error |
There was a problem hiding this comment.
Could we validate the specific error that comes here too?
Which issue does this PR close?
Rationale for this change
The DROP pipe operator (
|> DROP x, y) is part of the BigQuery pipe syntax tracked in #14660. It removes columns from the output, equivalent toSELECT * EXCEPT(x, y).What changes are included in this PR?
PipeOperator::Dropmatch arm inquery.rsthat reuses existingSELECT * EXCEPTinfrastructure (WildcardOptionswithExceptSelectItem)select.mdAre these changes tested?
Yes, four sqllogictest cases added in
pipe_operator.slt:Are there any user-facing changes?
Yes, users can now use
|> DROP column1, column2in pipe operator queries (BigQuery dialect).