[
https://issues.apache.org/jira/browse/CALCITE-4466?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17265126#comment-17265126
]
Rui Wang commented on CALCITE-4466:
-----------------------------------
+1
I think it makes senses. At least based on the javadoc, satisfies() returns
true means equal or stricter, which is better to use only equal.
Will give a review on the PR.
> Do not RelTraitDef.convert if the "from" trait already satisfies the "to"
> trait.
> --------------------------------------------------------------------------------
>
> Key: CALCITE-4466
> URL: https://issues.apache.org/jira/browse/CALCITE-4466
> Project: Calcite
> Issue Type: Task
> Components: core
> Affects Versions: 1.26.0
> Reporter: Vladimir Ozerov
> Assignee: Vladimir Ozerov
> Priority: Minor
> Labels: pull-request-available
> Fix For: 1.27.0
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> Consider that we have two trait defs {{A_def}} and {{B_def}}. We have a node
> with the trait set {{[A_from, B_from]}} that we want to convert to a node
> with the traits set {{[A_to, B_to]}}. Suppose that:
> * {{A_from.satisfies(A_to) = false}}
> * {{B_from.satisfies(B_to) = true}}
> Currently, the {{VolcanoPlanner}} will invoke {{convert}} on both {{A_trait}}
> and {{B_trait}}, because it skips the conversion only when two traits are
> *equal*. In our example, it will lead to an unnecessary call to
> {{B_def.convert(node, B_to)}}. This is not a big problem but it forces
> implementors of custom traits to double-check whether the passed node already
> satisfies the required trait or not.
> The proposal is to replace {{fromTrait.equals(toTrait)}} with
> {{fromTrait.satisfies(toTrait)}}.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
