Comments inline.

> On Dec 10, 2021, at 4:34 AM, Максим Грамин <mgra...@gmail.com> wrote:
> 
> 1. In FilterCalcMergeRule get rid of casting to LogicalCalc and LogicalFilter:
> 
>  @Override public void onMatch(RelOptRuleCall call) {
>    final Filter filter = call.rel(0);
>    final Calc calc = call.rel(1);

Yes.

But keep the default config to match only Filter and LogicalCalc. People who 
want to match, say, all sub-classes of Calc can create their own config.


> 2. In FilterCalcMergeRule and FilterCalcMergeRule replace
> LogicalCalc.create with calc.copy:
> 
>  final Calc newCalc = calc.copy(calc.getTraitSet(), calc.getInput(),
> mergedProgram);

Yes. (There’s a small chance of hitting bugs if sub-classes have implemented 
copy incorrectly.) 

> 3. In ProjectCalcMergeRule replace the code:
> 
>    if (RexOver.containsOver(program)) {
>      LogicalCalc projectAsCalc = LogicalCalc.create(calc, program);
>      call.transformTo(projectAsCalc);
>      return;
>    }
> 
> with a simple one:
> 
>    if (RexOver.containsOver(program)) {
>      return;
>    }
> 
> because special rule ProjectToCalcRule convert LogicalProject to LogicalCalc.

I’m not convinced. Can you point me to a test in RelOptRulesTest that exercises 
this code and works before and after your change?

Please log a Jira case for this proposed change and we can discuss further 
there.

Julian


Reply via email to