[
https://issues.apache.org/jira/browse/CALCITE-5177?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17550215#comment-17550215
]
Ruben Q L commented on CALCITE-5177:
------------------------------------
[~julianhyde] I totally agree with you.
There are a few places where the new RelNode will get the source hints
transparently, such as the relevant RelNodes' copy methods (e.g. [here in
LogicalJoin|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/rel/logical/LogicalJoin.java#L185]),
or when a rule gets applied (via
[RelOptRuleCall#transformTo|https://github.com/apache/calcite/blob/63a15128a332af7a641b26a3224392960882170a/core/src/main/java/org/apache/calcite/plan/RelOptRuleCall.java#L269]).
However, in other transformations (such as ReflectiveVisitors like
RelFieldTrimmer, RelStructuredTypeFlattener or RelDecorrelator), the new
RelNode is created otherwise, e.g. recreated "from scratch" by using a
RelBuilder, or with LogicalXXX#create. In such scenarios, as of today, the
hints need to be explicitly transferred from the old node to the new, which is
a bit fragile and an error-prone situation.
Perhaps we could improve this situation inside {{RelDecorrelator}} by
centralizing the hints transfer in a single location: inside the {{register}}
method (which has the "old" and "new" nodes as parameters: if both are hintable
then transfer the hints from the former to the latter, or something like that),
instead of each individual {{decorrelateRel}} method.
The same idea might be applicable for {{RelStructuredTypeFlattener}} (using
{{setNewForOldRel}} method).
Not sure yet how we could simplify RelFielTrimer...
> 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 also seems to copy hints only for the "traditional hintables".
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
