[CALCITE-3865] RelCollationTraitDef.canConvert should always return true

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.23.0
• Component/s: core
• Labels: pull-request-available
• Assignee: Unassigned
• Reporter: Haisheng Yuan
• Created: 19/Mar/20 21:55
• Updated: 25/Feb/21 14:47
• Resolved: 12/Apr/20 19:58

Description

CALCITE-1148 introduced the following change to RelCollationTraitDef to fix RelTrait conversion bug, but it is just hiding the underlying issue and adding redundant and unnecessary check to planner.

@Override public boolean canConvert(RelOptPlanner planner,
      RelCollation fromTrait, RelCollation toTrait, RelNode fromRel) {
    // Returns true only if we can convert.  In this case, we can only convert
    // if the fromTrait (the input) has fields that the toTrait wants to sort.
    for (RelFieldCollation field : toTrait.getFieldCollations()) {
      int index = field.getFieldIndex();
      if (index >= fromRel.getRowType().getFieldCount()) {
        return false;
      }
    }
    return true;
  }

The root cause is that logical operators, especially LogicalSort can have traits, which is a bad design decision, and AggregateReduceFunctionsRule and RelBuilder fails to adjust the column mapping in RelTraitSet. The newly created LogicalProject has collation on column 5 (it just copy its input's RelTraitSet blindly), but it only has 2 columns.

Issue Links

• causes: CALCITE-4513 RelBuilder.aggregate destroys input traits when pruning unused fields of project
• links to: GitHub Pull Request #1863