alamb commented on code in PR #6885:
URL: https://github.com/apache/arrow-datafusion/pull/6885#discussion_r1279341505
##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -114,6 +114,14 @@ fn signature(lhs: &DataType, op: &Operator, rhs:
&DataType) -> Result<Signature>
))
})
}
+ Operator::AtArrow
+ | Operator::ArrowAt => {
+ arrow_coercion(lhs, rhs).map(Signature::uniform).ok_or_else(|| {
Review Comment:
I wonder why the name `arrow_` (and not, `array_`) here
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -70,6 +71,18 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
stack.push(StackEntry::SQLExpr(right));
stack.push(StackEntry::SQLExpr(left));
}
+ SQLExpr::JsonAccess {
+ left,
+ operator,
+ right,
+ } => {
+ // Note the order that we push the entries to the
stack
+ // is important. We want to visit the left node
first.
Review 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
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
