viirya commented on code in PR #8496:
URL: https://github.com/apache/arrow-datafusion/pull/8496#discussion_r1424468078


##########
datafusion/sql/src/expr/binary_op.rs:
##########
@@ -47,6 +47,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             BinaryOperator::PGBitwiseShiftRight => 
Ok(Operator::BitwiseShiftRight),
             BinaryOperator::PGBitwiseShiftLeft => 
Ok(Operator::BitwiseShiftLeft),
             BinaryOperator::StringConcat => Ok(Operator::StringConcat),
+            BinaryOperator::PGOverlap => Ok(Operator::And),

Review Comment:
   There are questions we should consider:
   
   1. Should we make `AND` as an alias of `array_intersect`?
   2. Should we make `&&` as an alias of `AND` and so `array_intersect` too?
   
   This PR currently seems to do both 1 and 2.
   
   From the doc link (https://duckdb.org/docs/sql/functions/nested) in this 
ticket, I only see DuckDB uses `&&` as alias operator for `list_intersect`.  Is 
`AND` also an alias of `list_intersect` in DuckDB? I don't see it in the doc.
   
   In Postgres, neither `&&` nor `AND` is used as alias of `array_intersect` 
(actually seems it doesn't have this function). `&&` is an alias of array 
overlapping.
   
   I'm not sure what engine DataFusion follow in general, but it looks like 
this doesn't follow completely DuckDB (if `AND` is not alias), and not Postgres 
at all. It looks it is inconsistent.
   
   
   



-- 
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]

Reply via email to