Thanks, I registered Jira task and PR
https://issues.apache.org/jira/browse/CALCITE-4936

On Sat, Dec 11, 2021 at 4:52 AM Julian Hyde <jhyde.apa...@gmail.com> wrote:

> 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