[CALCITE-3997] Logical rules matched with physical operators but failed to handle traits#1976
[CALCITE-3997] Logical rules matched with physical operators but failed to handle traits#1976hsyuan wants to merge 1 commit intoapache:masterfrom hsyuan:CALCITE-3997
Conversation
hsyuan
changed the title
hsyuan
added
the
LGTM-will-merge-soon
|
@hsyuan it looks like you modified 92 files... |
…ed to handle traits Logical transformation rule, only logical operator can be rule operand, and only generate logical alternatives. It is only visible to VolcanoPlanner, HepPlanner will ignore this interface. That means, in HepPlanner, the rule that implements TransformationRule can still match with physical operator of PhysicalNode and generate physical alternatives. But in VolcanoPlanner, TransformationRule doesn't match with physical operator that implements PhysicalNode. It is highly discouraged to generate physical operators in TransformationRule, unless you are using it in HepPlanner. Close #1976
|
btw I see now that you added a marker interface. |
|
There won't be conflicts. It is the thorough fix. |
|
How do you guarantee only logical nodes are generated in transformation rule? Also please make sure to include this in the release note as a potential breaking change. |
It is allowed, but not encouraged. Just a contract, unless we add a check inside
What do you think it will break? |
|
If this's something we believe should be enforced, why don't we just add such check? There are a number of rules you can be applied to both logical and physical nodes today (matching the base rel class instead of the logical rel). Those rules won't fire anymore with your change. Certain scenarios could break due to this. |
|
If it can match physical operators, that means it is already matches with logical operators, it just generates redundant operators. What scenario will it break? |
|
If one physical node is created by a rule, rather than being converted from a logical node, then there's no logical counterpart and won't be processed by your transformation rule. We don't have such rule in Calcite, but it doesn't mean the user cannot create one. |
|
The only possible case is the user's special rule creates a physical operator that is a sub-class of Enumerable operator, at the same use Calcite's built-in logical rule to process it. |
|
Not necessarily a sub-class of Enumerable. It could be a sub-class of the base RelNode, such as Project or Aggregate. |
|
The restriction only applies operator that implements |
3 participants
Logical transformation rule, only logical operator can be rule operand, and
only generate logical alternatives. It is only visible to VolcanoPlanner,
HepPlanner will ignore this interface. That means, in HepPlanner, the rule that
implements TransformationRule can still match with physical operator of
PhysicalNode and generate physical alternatives. But in VolcanoPlanner,
TransformationRule doesn't match with physical operator that implements
PhysicalNode. It is highly discouraged to generate physical operators in
TransformationRule, unless you are using it in HepPlanner.
This will also fix issue CALCITE-3968.