Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-3865

RelCollationTraitDef.canConvert should always return true

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.23.0
    • Component/s: core

      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.

        Attachments

        1. image-2020-03-19-16-47-21-805.png
          59 kB
          Haisheng Yuan
        2. image-2020-03-19-16-44-25-621.png
          649 kB
          Haisheng Yuan

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                hyuan Haisheng Yuan
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0h
                  0h
                  Logged:
                  Time Spent - 1h 40m
                  1h 40m