This sounds reasonable to me. It also sounds like we could make this backwards compatible by retaining (but deprecating) the existing constructors and factory methods that will no longer be needed. -- Michael Mior mm...@apache.org
Le jeu. 20 févr. 2020 à 13:11, Julian Hyde <jh...@apache.org> a écrit : > > I have an idea for a refactoring to RelOptRule. I haven’t fully thought it > through, but I’m going to sketch it out here to see whether folks agree about > the problems/solutions. > > It will be a breaking change (in the sense that people will have to change > their code in order to get it to compile) but relatively safe (in that once > the code compiles, it will have the same behavior as before). Also it will > give Calcite developers and users a lot more flexibility going forward. > > The problem I see is that people often want different variants of planner > rules. An example is FilterJoinRule, which has a 'boolean smart’ parameter, a > predicate (which returns whether to pull up filter conditions), operands > (which determine the precise sub-classes of RelNode that the rule should > match) and a relBuilderFactory (which controls the type of RelNode created by > this rule). > > Suppose you have an instance of FilterJoinRule and you want to change ‘smart’ > from true to false. The ‘smart’ parameter is immutable (good!) but you can’t > easily create a clone of the rule because you don’t know the values of the > other parameters. Your instance might even be (unbeknownst to you) a > sub-class with extra parameters and a private constructor. > > So, my proposal is to put all of the config information of a RelOptRule into > a single ‘config’ parameter that contains all relevant properties. Each > sub-class of RelOptRule would have one constructor with just a ‘config’ > parameter. Each config knows which sub-class of RelOptRule to create. > Therefore it is easy to copy a config, change one or more properties, and > create a new rule instance. > > Adding a property to a rule’s config does not require us to add or deprecate > any constructors. > > The operands are part of the config, so if you have a rule that matches a > EnumerableFilter on an EnumerableJoin and you want to make it match an > EnumerableFilter on an EnumerableNestedLoopJoin, you can easily create one > with one changed operand. > > The config is immutable and self-describing, so we can use it to > automatically generate a unique description for each rule instance. > > Julian > > [1] > https://github.com/apache/calcite/blob/5fa41609cb0fe310a0a11d86319d861423850a36/core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java#L93 > > <https://github.com/apache/calcite/blob/5fa41609cb0fe310a0a11d86319d861423850a36/core/src/main/java/org/apache/calcite/rel/rules/FilterJoinRule.java#L93> >
