asolimando commented on PR #4267: URL: https://github.com/apache/calcite/pull/4267#issuecomment-2773167902
> @asolimando Got it. Your concerns are also what I have considered. As the notes I described in jira, this rule must be guaranteed to be applied only once to avoid generating same redundant expression. A simple example is that it cannot be in the same rule set as `FilterJoinRule`. We need to apply this rule first and then `FilterJoinRule`. Of course, as you said, other rules may also interact with this rule and cause an infinite loop, especially additional operations for expressions. > > My intention is that it is applied in HepPlanner and avoids interaction with other rules related to expression processing. It is applied separately, and then performs predicate pushdown, expression simplification, etc. This is indeed more strict, but I think it is safe and effective. > > So should I remove it from `CoreRules` now (is that what 'not have it added to core right away and leave it as an optional rule' means?), or is there a better solution? I apologize because my comment was indeed ambiguous, what I meant was not to remove it from `CoreRules`, but to make sure the rule doesn't end up in one the sets of standard rules as defined in `RelOptRules`, this is not the case at the moment. I think that a word of caution could be added as a comment in the javadoc of the rule itself, briefly describing known issues (what rules could be problematic when paired with this one, how to limit the danger via the bloat parameter, etc.), this will help users understanding the implications of adding this rule, especially in combination with some others known to cause infinite loops. In the current javadoc, it is correctly suggested to use this rule before `CoreRules#FILTER_INTO_JOIN` and `CoreRules#JOIN_CONDITION_PUSH`, maybe we can be more explicit suggesting the use of the `HepPlanner` too? It can help for discussions like [this one](https://lists.apache.org/thread/0yotdv62r6b3wg1qztftdpnkzpc4dvzv) in the mailing list to have better description of what rules to add and how they are expected to be used. -- 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]
