[GitHub] [arrow] cpcloud commented on a change in pull request #11243: ARROW-14142: [IR] [C++] Make join predicate a list of expressions

[GitBox <gi...@apache.org>]

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-unsubscribe@arrow.apache.org

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