[ https://issues.apache.org/jira/browse/CALCITE-4055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17130628#comment-17130628 ]
Ruben Q L edited comment on CALCITE-4055 at 6/10/20, 3:45 PM: -------------------------------------------------------------- [~danny0405], I understand. However, I think that this approach could lead to undesired consequences in RelFieldTrimmer. Considering we must trim an Aggregate (_origAgg_, with _aggHint_) in {{RelFielTrimmer#trimFields(Aggregate,...)}}. At the end of that method, we would build the new aggregate and return it: {code} ... relBuilder.aggregate(...); RelNode output = relBuilder.build(); return result(output, mapping); {code} If just before the return we introduce RelOptUtil#copyRelHints: {code} ... relBuilder.aggregate(...); RelNode output = relBuilder.build(); RelOptUtil.copyRelHints(origAgg, output); // <-- return result(output, mapping); {code} There is no guarantee that "output" is in fact an Aggregate. It could be a non-Hintable, in which case there would be no problem, since copyRelHints will be a NO-OP. But it could be a Project (due to RelBuilder#aggregate implementation), which is Hintable, and in that case we would be transferring the "aggHint" from the original Aggregate to the new Project, and I assume this should not be the case. was (Author: rubenql): [~danny0405], I understand. However, I think that this approach could lead to undesired consequences in RelFieldTrimmer. Considering we must trim an Aggregate (origAgg, with aggHint) in {{RelFielTrimmer#trimFields(Aggregate,...)}}. At the end of that method, we would build the new aggregate and return it: {code} ... relBuilder.aggregate(...); RelNode output = relBuilder.build(); return result(output, mapping); {code} If just before the return we introduce: {code} ... RelOptUtil.copyRelHints(origAgg, output); return result(output, mapping); {code} There is no guarantee that "output" is in fact an Aggregate. It could be a non-Hintable, in which case there would be no problem, since copyRelHints will be a NO-OP. But it could be a Project (due to RelBuilder#aggregate implementation), which is Hintable, and in that case we would be transferring the "aggHint" from the original Aggregate to the new Project, and I assume this should not be the case. > RelFieldTrimmer loses join hints > -------------------------------- > > Key: CALCITE-4055 > URL: https://issues.apache.org/jira/browse/CALCITE-4055 > Project: Calcite > Issue Type: Bug > Components: core > Affects Versions: 1.23.0 > Reporter: Ruben Q L > Assignee: Ruben Q L > Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > The trimmed plan generated by RelFieldTrimmer can lose the hints that might > be present in the input plan. > The issue can be reproduced with the following test (to be added to > RelFieldTrimmerTest): > {code:java} > @Test void testJoinWithHints() { > final RelHint noHashJoinHint = > RelHint.builder("NO_HASH_JOIN").inheritPath(0).build(); > final RelBuilder builder = RelBuilder.create(config().build()); > final RelNode original = > builder.scan("EMP") > .scan("DEPT") > .join(JoinRelType.INNER, > builder.equals( > builder.field(2, 0, "DEPTNO"), > builder.field(2, 1, "DEPTNO"))) > .hints(noHashJoinHint) > .project( > builder.field("ENAME"), > builder.field("DNAME")) > .build(); > final RelFieldTrimmer fieldTrimmer = new RelFieldTrimmer(null, builder); > final RelNode trimmed = fieldTrimmer.trim(original); > final String expected = "" > + "LogicalProject(ENAME=[$1], DNAME=[$4])\n" > + " LogicalJoin(condition=[=($2, $3)], joinType=[inner])\n" > + " LogicalProject(EMPNO=[$0], ENAME=[$1], DEPTNO=[$7])\n" > + " LogicalTableScan(table=[[scott, EMP]])\n" > + " LogicalProject(DEPTNO=[$0], DNAME=[$1])\n" > + " LogicalTableScan(table=[[scott, DEPT]])\n"; > assertThat(trimmed, hasTree(expected)); > assertTrue(original.getInput(0) instanceof Join); > final Join originalJoin = (Join) original.getInput(0); > assertTrue(originalJoin.getHints().contains(noHashJoinHint)); > assertTrue(trimmed.getInput(0) instanceof Join); > final Join join = (Join) trimmed.getInput(0); > assertTrue(join.getHints().contains(noHashJoinHint)); > } > {code} > which fails in the last line: > {{assertTrue(join.getHints().contains(noHashJoinHint));}} -- This message was sent by Atlassian Jira (v8.3.4#803005)