Details

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

      Description

      Following CALCITE-1023, AggregateConstantKeyRule is a superset of AggregateProjectPullUpConstantsRule because it matches any RelNode whose output columns are constants, not just a Project.

      This task is to check that AggregateConstantKeyRule does indeed have all required functionality, convert test cases to use it, mark AggregateProjectPullUpConstantsRule deprecated, and remove as much of its code as possible.

      Or to obsolete AggregateConstantKeyRule if that is easier.

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        I decided to obsolete AggregateConstantKeyRule, moving its functionality into AggregateProjectPullUpConstantsRule.

        We now detect constants using metadata (RelMdPredicates). The input does not need to be a Project, and constants do not need to be literals.

        Show
        julianhyde Julian Hyde added a comment - I decided to obsolete AggregateConstantKeyRule, moving its functionality into AggregateProjectPullUpConstantsRule. We now detect constants using metadata (RelMdPredicates). The input does not need to be a Project, and constants do not need to be literals.
        Show
        julianhyde Julian Hyde added a comment - Jesus Camacho Rodriguez , Can you please review my proposed fix: https://github.com/julianhyde/calcite/commit/4958e90372ebd1bae50e6fc64c2c5caf2b72ea81
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Julian Hyde, LGTM. Test changes are fine too.

        nitpick. Line 517 in ReduceExpressionsRule.java: probably you meant RexNode?

        Thanks

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Julian Hyde , LGTM. Test changes are fine too. nitpick. Line 517 in ReduceExpressionsRule.java: probably you meant RexNode? Thanks
        Hide
        julianhyde Julian Hyde added a comment -

        Jesus Camacho Rodriguez, Thanks the review. Yes, I meant RexNode; I've amended the commit in my github fork. I will commit to master shortly.

        Show
        julianhyde Julian Hyde added a comment - Jesus Camacho Rodriguez , Thanks the review. Yes, I meant RexNode; I've amended the commit in my github fork. I will commit to master shortly.
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/904c73da .
        Hide
        julianhyde Julian Hyde added a comment -

        Resolved in release 1.6.0 (2016-01-22).

        Show
        julianhyde Julian Hyde added a comment - Resolved in release 1.6.0 (2016-01-22).

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            julianhyde Julian Hyde
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development