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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
