[CALCITE-3357] Trivial null checking in RelSet#addAbstractConverters

• Type: Bug
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Fix Version/s: 1.22.0
• Component/s: core
• Labels: pull-request-available
• Assignee: Unassigned
• Reporter: Jin Xing
• Created: 18/Sep/19 04:01
• Updated: 06/Mar/20 04:36
• Resolved: 21/Sep/19 00:49

Description

In current code of RelSet#addAbstractConverters, null checking of curRelTrait happens after clause of assert curRelTrait.getTraitDef() == traitDef;

It makes more sense to adjust the order;

In my understanding, this issue was not found previously for two reasons:

1. AbstractConverter is not enabled by default (https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableConvention.java#L65)
2. It rarely happens that two algebra expression operators have identical semantics but different RelTraitDef

I found this issue when working on https://issues.apache.org/jira/browse/CALCITE-2970

Issue Links

• relates to: CALCITE-2970 Performance issue when enabling abstract converter for EnumerableConvention
• links to: GitHub Pull Request #1461