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

Method setAggChildKeys should take into account indicator columns of Aggregate operator

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1.0-incubating
    • Component/s: None
    • Labels:
      None

      Description

      The method setAggChildKeys in RelMdUtil should take into account indicator columns of Aggregate operator.

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        I have reviewed all uses of Aggregate.getGroupCount to make sure that they look for indicator fields.

        In AggregateStarTableRule:

                    public int getSourceOpt(int source) {
                      if (source < aggregate.getGroupCount()) {
                        int in = tileKey.dimensions.nth(source);
                        return aggregate.getGroupSet().indexOf(in);
                      }
                      Lattice.Measure measure =
                          measures.get(source - aggregate.getGroupCount());
                      int i = tileKey.measures.indexOf(measure);
                      assert i >= 0;
                      return tileKey.dimensions.cardinality() + i;
                    }
        

        Can you please fix that as part of this bug? The simplest way might be to throw AssertionError if there are indicators.

        Everywhere else looks OK.

        Show
        julianhyde Julian Hyde added a comment - I have reviewed all uses of Aggregate.getGroupCount to make sure that they look for indicator fields. In AggregateStarTableRule: public int getSourceOpt( int source) { if (source < aggregate.getGroupCount()) { int in = tileKey.dimensions.nth(source); return aggregate.getGroupSet().indexOf(in); } Lattice.Measure measure = measures.get(source - aggregate.getGroupCount()); int i = tileKey.measures.indexOf(measure); assert i >= 0; return tileKey.dimensions.cardinality() + i; } Can you please fix that as part of this bug? The simplest way might be to throw AssertionError if there are indicators. Everywhere else looks OK.
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/c50a6e0d .
        Hide
        julianhyde Julian Hyde added a comment -

        Closing now that 1.1.0-incubating has been released.

        Show
        julianhyde Julian Hyde added a comment - Closing now that 1.1.0-incubating has been released.

          People

          • Assignee:
            jcamachorodriguez Jesus Camacho Rodriguez
            Reporter:
            jcamachorodriguez Jesus Camacho Rodriguez
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development