AndreaBozzo commented on code in PR #19916:
URL: https://github.com/apache/datafusion/pull/19916#discussion_r2725391890
##########
datafusion/substrait/tests/cases/roundtrip_logical_plan.rs:
##########
@@ -632,12 +632,12 @@ async fn roundtrip_inlist_5() -> Result<()> {
plan,
@r#"
Projection: data.a, data.f
- Filter: data.f = Utf8("a") OR data.f = Utf8("b") OR data.f = Utf8("c")
OR data2.mark
+ Filter: (((data.f = Utf8("a")) OR (data.f = Utf8("b"))) OR (data.f =
Utf8("c"))) OR data2.mark
Review Comment:
I'll try to explain what i found but my current understanding might be
limited.
Seems like DuckDB also always adds parentheses, see
[conjunction_expression.hpp#L42-L48](https://github.com/duckdb/duckdb/blob/main/src/include/duckdb/parser/expression/conjunction_expression.hpp#L42-L48)
they represent a OR b OR c as a flat list of children
(ConjunctionExpression), they get (a OR b OR c) while our nested BinaryExpr
tree produces ((a OR b) OR c).
Its the reason why i think we should discuss this further as its kinda
delicate, and more people should take a look
Thank you for your time mr.Vo
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
