cpcloud commented on a change in pull request #11243:
URL: https://github.com/apache/arrow/pull/11243#discussion_r717040533



##########
File path: experimental/computeir/Relation.fbs
##########
@@ -120,10 +120,10 @@ table Join {
   left: Relation (required);
   /// Right relation
   right: Relation (required);
-  /// The expression which will be evaluated against rows from each
+  /// Conjunction of expressions that will be evaluated against rows from each
   /// input to determine whether they should be included in the
   /// join relation's output.
-  on_expression: Expression (required);
+  predicates: [Expression] (required);

Review comment:
       > This may be a foolish question but what do we gain from allowing a 
list of expressions
   
   Not foolish at all, thanks for asking. The goal is to mainly to make it a 
less annoying to deal with predicates that are already in list form, such as 
here 
https://github.com/duckdb/duckdb/pull/2331/files#diff-5ad901777f48a6ceb50fe357b6b76fbca1b512fe079a97576cb6c1c1a5f7b951R418-R425.
   
   > combined by an implicit conjunction (AND? OR?)
   
   Conjunction is `AND`, disjunction is `OR`. Here it's an implicit 
conjunction, so `AND`.
   
   > Couldn't the conjunction just be supplied explicitly by the producer?
   
   The question is whether most consumers/producers will be bringing a list of 
predicates or will have them already `AND`ed together and how they want to work 
with those predicates.
   
   FWIW I'm fine not merging this PR, since it just makes code a little uglier 
and as of now doesn't prevent any functionality from working.




-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to