SteveStevenpoor commented on PR #26689: URL: https://github.com/apache/flink/pull/26689#issuecomment-2983576055
> If you have the capacity, you could open a PR against that branch to cover mostly [FLINK-37890](https://issues.apache.org/jira/browse/FLINK-37890). It'll be automatically retargeted to master as soon as we merge #26687. Some observations from reviewing this PR: I'll do it! > No commonJoinKey in general does not mean we don't use MultiJoin, it just means we have to break it into multiple MultiJoins. It's in the description of [FLINK-37890](https://issues.apache.org/jira/browse/FLINK-37890). testInnerJoinChainNoCommonJoinKey e.g. would have two multiple joins with two inputs each. We want a test to check if we can have for example 2, 3 MultiJoins combined. Say we have A JOIN B JOIN C JOIN D and A, B, C have commonJoinKey but D doesnt. My code will give MJ(A, B, C) JOIN D. But what we want is actually MJ(MJ(A, B, C), D). Am I correct? > We want to use calcilte's MultiJoin for now in the rule to be able to combined it with the native project and merge rule for now, as we did in the other PR. commonJoinKeys doesn't have to be stored. We just need to check if the constraint is respected here. I needed it to make check for commonJoinKey more simple. I can use calcite's mj instead. > The rule shouldn't cover right joins since we got the other for that. What do you mean by covering right joins? The rule will be applied only for inner and left joins: ``` final Join origJoin = call.rel(0); if (origJoin.getJoinType() != JoinRelType.INNER && origJoin.getJoinType() != JoinRelType.LEFT) { return false; } ``` If you are talking about checking that right child is projection\join that's because I did JoinToMultiJoinRule with the respect to RightToLeftJoinRule which swaps inputs and adds projection. I needed to cover this case. > We'd want e2e table programs to test it as shown in the other PR. Affirmative. > If you don't have a lot of time now, that's no problem and I'll get to this probably tomorrow. After the deadline, we can discuss you picking other items for the multi way join without the rush of the deadline. We still have some essential jira tickets to get the feature from an experimental state to production ready [here](https://issues.apache.org/jira/browse/FLINK-37859). I'll start today with 37890 and hope to drop PR by tomorrow. I will keep you informated. -- 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]
