feat: array containment operator @> and <@
#6885
Conversation
github-actions
bot
added
sql
|
|
||
| ## array containment operator | ||
|
|
||
| # array containment operator with scalars #1 (at arrow) |
There was a problem hiding this comment.
The feature supports now only scalars as input, but in the production users prefer to work with columns.
I created the ticket dedicated to that issue: #7119.
|
@alamb @jayzhan211 Can you review this little feature if you have time. |
datafusion/sql/src/expr/mod.rs
Outdated
| right, | ||
| } => { | ||
| // Note the order that we push the entries to the stack | ||
| // is important. We want to visit the left node first. |
There was a problem hiding this comment.
why should we visit the left node first?
There was a problem hiding this comment.
I think this is a copy/paste comment from above.
I believe the rationale is that this code is using an explicit stack (rather than local variables on the call stack) to avoid stack overflows with deeply nested queries.
To be consistent the planner did a depth first search (always following the first / left child first) and thus to replicate that behavior the left child must be put on the stack
|
@alamb can you review that change if you have time? |
|
On my list |
alamb
changed the title
@> and <@
There was a problem hiding this comment.
Looks great to me -- thanks @izveigor - I am happy to merge this as is of can wait for you to address any comments you would like
It would also be great to add these new operators to the SQL documentation, but given a quick look at the user guide suggests we don't have any docs for supported operators yet. https://arrow.apache.org/datafusion/user-guide/sql/index.html
I will file a follow on ticket to add some
| } | ||
| Operator::AtArrow | ||
| | Operator::ArrowAt => { | ||
| arrow_coercion(lhs, rhs).map(Signature::uniform).ok_or_else(|| { |
There was a problem hiding this comment.
I wonder why the name arrow_ (and not, array_) here
There was a problem hiding this comment.
Agree, with array_ it would be more consistent.
datafusion/sql/src/expr/mod.rs
Outdated
| right, | ||
| } => { | ||
| // Note the order that we push the entries to the stack | ||
| // is important. We want to visit the left node first. |
There was a problem hiding this comment.
I think this is a copy/paste comment from above.
I believe the rationale is that this code is using an explicit stack (rather than local variables on the call stack) to avoid stack overflows with deeply nested queries.
To be consistent the planner did a depth first search (always following the first / left child first) and thus to replicate that behavior the left child must be put on the stack
|
Filed #7153 for documentation |
|
@alamb I fixed all nitpicks which you mentioned in your comments. Thank you for review, @alamb and @jayzhan211! |
|
Thanks again @izveigor |
3 participants
Which issue does this PR close?
Closes #6884
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?