amaliujia edited a comment on pull request #2006: URL: https://github.com/apache/calcite/pull/2006#issuecomment-643722774
Ok, in fact, I need to refactor a lot on this PR. I ignored one biggest fact, which caused my implementation wrong in some places. Basically, I believe, per reading EnumerableMergeJoin implementation, it will 1) construct key selector based on ordering of left join keys and right join keys. 2) construct key comparator by `keys must be sorted in ascending order, nulls last`: https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableMergeJoin.java#L306. This implementation limits traits propagation in EnumerableMergeJoin to 1. Collations that is constructed for join (not the required one, but the one we want to construct) must have the same ordering as join keys. 2. Collations must be in ascending order, null last So I think based on this understanding, even the exact key matching is not correct: 1. violates the second requirement. 2. just consider orderless case so not following join key's ordering. However, the most useful way is, EnumerableMergeJoin construct key selector and key comparator based on collations. @hsyuan do you agree? ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
