rubenada commented on code in PR #2829:
URL: https://github.com/apache/calcite/pull/2829#discussion_r893570140
##########
core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java:
##########
@@ -1595,6 +1590,7 @@ static Map<Integer, Integer> identityMap(int count) {
Frame register(RelNode rel, RelNode newRel,
Map<Integer, Integer> oldToNewOutputs,
NavigableMap<CorDef, Integer> corDefOutputs) {
+ newRel = RelOptUtil.copyRelHints(rel, newRel);
Review Comment:
Unfortunately, it is not that simple, using a propagation here instead of a
copy breaks some unit tests. It seems it is expected to copy the hints inside
RelDecorrelator, and propagate them only once at the end of the process
(RelDecorrelator:234). If we propagate them "too soon", they'll be lost.
Also, take into account that propagating the hints in the middle of a
transformation is risky, since the propagation also "filters" the hints (and
only attach the hints to a RelNode if it can be applied to it based on the
HintStrategy).
All in all, it seems the hint mechanism is still quite fragile, but IMHO the
current patch fixes at least one issue, so I'd propose to apply it to avoid a
regression, and maybe in the next release review the whole hint mechanism.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
