Good catch. The unwrapping was not intentional on my part. Removing the unwrapping seems like a benefit (less work for RelBuilder, therefore better planning performance; SARGs in more places, therefore better optimization; also the avoidance of cycles as you mention).
There’s a small risk that downstream projects are relying on this unwrapping but they can control using config.simplify(). We don’t promise not to improve our rewrites from one release to the next. I think you should fix it. Julian > On Apr 2, 2022, at 8:30 AM, Vladimir Ozerov <[email protected]> wrote: > > Hi, > > When simplification is enabled in the RelBuilder, a call to the > RelBuilder.project unwraps SARGs into their flat counterparts [1]. This is > not the case for other nodes, like Filter or Join. This behavior may lead > to an infinite loop when the ProjectReduceExpressionsRule is used with the > HepPlanner (which is a common pattern); > > 1. The original Project(e) has the expression e. > 2. ProjectReduceExpressionsRule simplifies it to SARG(e). > 3. ProjectReduceExpressionsRule calls RelBuilder.project which returns > back Project(e) instead of Project(SARG(e)). > 4. ProjectReduceExpressionsRule calls Project(e).transformTo(Project(e)) > which schedules invocation of the rule again, leading to a hang. > > Shall we remove this unwrapping? No tests are affected except for two > trivial failures in MaterializedViewRelOptRulesTest. > > Regards, > Vladimir. > > [1] > https://github.com/apache/calcite/blob/a81cfb2ad001589929e190939cf4db928ebac386/core/src/main/java/org/apache/calcite/tools/RelBuilder.java#L1948
