[ 
https://issues.apache.org/jira/browse/CALCITE-5177?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ruben Q L updated CALCITE-5177:
-------------------------------
    Description: 
This seems to be a regression caused by CALCITE-5107, which enlarged the list 
of hintable RelNodes by making Filter, SetOp, Sort, Window, Values hintable.
However, it seems that this patch "missed" some Calcite preprocessing elements 
that can change a plan, and therefore require a manual / ad-hoc treatment of 
hints to avoid losing them in their processing, e.g. RelDecorrelator.

In my specific example, let's have this query:
{code:sql}
SELECT /*+ MY_HINT_FOR_JOIN */ c.c_custkey, , c.c_name FROM customer c
WHERE NOT EXISTS (
  SELECT 1 FROM orders o WHERE o.o_custkey = c.c_custkey AND o.o_orderstatus <> 
'abc'
) ORDER BY c.c_custkey
{code}
Before CALCITE-5107, when the query was parsed, a logical plan was created, my 
hint was attached to a LogicalProject. The plan contained a LogicalCorrelate 
(to implement the NOT EXISTS). The plan was then decorrelated with 
RelDecorrelator, and as a result, we obtained a new plan with a LogicalJoin 
(instead of correlate), where the hint had been propagated from the projection 
until the join. Everything is fine.

After CALCITE-5107, when the query was parsed, now we obtain the same logical 
plan, but now the hint is attached to the LogicalSort (not to the 
LogicalProject). When the decorrelator is executed, the plan is transformed (to 
have LogicalJoin), but {*}the hint is lost{*}, it is not in the new plan's sort 
(or project, or join, it's nowhere).
Running the debugger, it seems the problem is inside 
RelDecorrelator#decorrelateQuery:
{code:java}
...
  if (!decorrelator.cm.mapCorToCorRel.isEmpty()) {
    newRootRel = decorrelator.decorrelate(newRootRel); // <-- HINT LOST HERE!
  }
  // NOTHING GETS PROPAGATED BECAUSE THE HINT WAS LOST!
  newRootRel = RelOptUtil.propagateRelHints(newRootRel, true);
  
  return newRootRel;
{code}
The root cause seems to be that, inside [RelDecorrelator's 
code|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java],
 there are only a few places where hints are copied from the "old" RelNode to 
the newly created RelNode: 3 calls to RelOptUtil#copyRelHints 
([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L589],
 
[here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L685],
 and 
[here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L796]),
 only for projections and aggregates; + RelBuilder#hints [to copy them for 
joins|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L1308].
To sum up, it seems RelDecorrelator only cared to copy hints for the 
"traditional hintables", so probably something like that is missing for sorts 
(and other newly hintable RelNodes added by CALCITE-5107).

Apart from RelDecorrelator, other classes that might suffer from a similar 
problem are:
 - 
[RelFieldTrimmer|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java]:
 which currently only propagates hints for projections and aggregates 
([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L535],
 
[here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L574],
 and 
[here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L1115]);
 and copies hints for joins 
([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L909]).
- 
[RelStructuredTypeFlattener|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java],
 which seems to copy hints only for the "traditional hintables".

  was:
This seems to be a regression caused by CALCITE-5107, which enlarged the list 
of hintable RelNodes by making Filter, SetOp, Sort, Window, Values hintable.
However, it seems that this patch "missed" some Calcite preprocessing elements 
that can change a plan, and therefore require a manual / ad-hoc treatment of 
hints to avoid losing them in their processing, e.g. RelDecorrelator.

In my specific example, let's have this query:
{code:sql}
SELECT /*+ MY_HINT_FOR_JOIN */ c.c_custkey, , c.c_name FROM customer c
WHERE NOT EXISTS (
  SELECT 1 FROM orders o WHERE o.o_custkey = c.c_custkey AND o.o_orderstatus <> 
'abc'
) ORDER BY c.c_custkey
{code}
Before CALCITE-5107, when the query was parsed, a logical plan was created, my 
hint was attached to a LogicalProject. The plan contained a LogicalCorrelate 
(to implement the NOT EXISTS). The plan was then decorrelated with 
RelDecorrelator, and as a result, we obtained a new plan with a LogicalJoin 
(instead of correlate), where the hint had been propagated from the projection 
until the join. Everything is fine.

After CALCITE-5107, when the query was parsed, now we obtain the same logical 
plan, but now the hint is attached to the LogicalSort (not to the 
LogicalProject). When the decorrelator is executed, the plan is transformed (to 
have LogicalJoin), but {*}the hint is lost{*}, it is not in the new plan's sort 
(or project, or join, it's nowhere).
Running the debugger, it seems the problem is inside 
RelDecorrelator#decorrelateQuery:
{code:java}
...
  if (!decorrelator.cm.mapCorToCorRel.isEmpty()) {
    newRootRel = decorrelator.decorrelate(newRootRel); // <-- HINT LOST HERE!
  }
  // NOTHING GETS PROPAGATED BECAUSE THE HINT WAS LOST!
  newRootRel = RelOptUtil.propagateRelHints(newRootRel, true);
  
  return newRootRel;
{code}
The root cause seem to be that, inside [RelDecorrelator's 
code|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java],
 there are only a few places where hints are copied from the "old" RelNode to 
the newly created RelNode: 3 calls to RelOptUtil#copyRelHints 
([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L589],
 
[here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L685],
 and 
[here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L796]),
 only for projections and aggregates; + RelBuilder#hints [to copy them for 
joins|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L1308].
To sum up, it seems RelDecorrelator only cared to copy hints for the 
"traditional hintables", so probably something like that is missing for sorts 
(and other newly hintable RelNodes added by CALCITE-5107).

Apart from RelDecorrelator, other classes that might suffer from a similar 
problem are:
 - 
[RelFieldTrimmer|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java]:
 which currently only propagates hints for projections and aggregates 
([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L535],
 
[here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L574],
 and 
[here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L1115]);
 and copies hints for joins 
([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L909]).
- 
[RelStructuredTypeFlattener|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java],
 which seems to copy hints only for the "traditional hintables".


> Query loses hint after decorrelation
> ------------------------------------
>
>                 Key: CALCITE-5177
>                 URL: https://issues.apache.org/jira/browse/CALCITE-5177
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>            Reporter: Ruben Q L
>            Priority: Blocker
>             Fix For: 1.31.0
>
>
> This seems to be a regression caused by CALCITE-5107, which enlarged the list 
> of hintable RelNodes by making Filter, SetOp, Sort, Window, Values hintable.
> However, it seems that this patch "missed" some Calcite preprocessing 
> elements that can change a plan, and therefore require a manual / ad-hoc 
> treatment of hints to avoid losing them in their processing, e.g. 
> RelDecorrelator.
> In my specific example, let's have this query:
> {code:sql}
> SELECT /*+ MY_HINT_FOR_JOIN */ c.c_custkey, , c.c_name FROM customer c
> WHERE NOT EXISTS (
>   SELECT 1 FROM orders o WHERE o.o_custkey = c.c_custkey AND o.o_orderstatus 
> <> 'abc'
> ) ORDER BY c.c_custkey
> {code}
> Before CALCITE-5107, when the query was parsed, a logical plan was created, 
> my hint was attached to a LogicalProject. The plan contained a 
> LogicalCorrelate (to implement the NOT EXISTS). The plan was then 
> decorrelated with RelDecorrelator, and as a result, we obtained a new plan 
> with a LogicalJoin (instead of correlate), where the hint had been propagated 
> from the projection until the join. Everything is fine.
> After CALCITE-5107, when the query was parsed, now we obtain the same logical 
> plan, but now the hint is attached to the LogicalSort (not to the 
> LogicalProject). When the decorrelator is executed, the plan is transformed 
> (to have LogicalJoin), but {*}the hint is lost{*}, it is not in the new 
> plan's sort (or project, or join, it's nowhere).
> Running the debugger, it seems the problem is inside 
> RelDecorrelator#decorrelateQuery:
> {code:java}
> ...
>   if (!decorrelator.cm.mapCorToCorRel.isEmpty()) {
>     newRootRel = decorrelator.decorrelate(newRootRel); // <-- HINT LOST HERE!
>   }
>   // NOTHING GETS PROPAGATED BECAUSE THE HINT WAS LOST!
>   newRootRel = RelOptUtil.propagateRelHints(newRootRel, true);
>   
>   return newRootRel;
> {code}
> The root cause seems to be that, inside [RelDecorrelator's 
> code|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java],
>  there are only a few places where hints are copied from the "old" RelNode to 
> the newly created RelNode: 3 calls to RelOptUtil#copyRelHints 
> ([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L589],
>  
> [here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L685],
>  and 
> [here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L796]),
>  only for projections and aggregates; + RelBuilder#hints [to copy them for 
> joins|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java#L1308].
> To sum up, it seems RelDecorrelator only cared to copy hints for the 
> "traditional hintables", so probably something like that is missing for sorts 
> (and other newly hintable RelNodes added by CALCITE-5107).
> Apart from RelDecorrelator, other classes that might suffer from a similar 
> problem are:
>  - 
> [RelFieldTrimmer|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java]:
>  which currently only propagates hints for projections and aggregates 
> ([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L535],
>  
> [here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L574],
>  and 
> [here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L1115]);
>  and copies hints for joins 
> ([here|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/sql2rel/RelFieldTrimmer.java#L909]).
> - 
> [RelStructuredTypeFlattener|https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql2rel/RelStructuredTypeFlattener.java],
>  which seems to copy hints only for the "traditional hintables".



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to