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

Validate that window functions have OVER clause

        Details

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

          Description

          For instance,

          select rank() 
from emp

          should have given exception.

            Activity

            Ascending order - Click to sort in descending order
            Hide
            Permalink
            seanhychu Sean Hsuan-Yi Chu added a comment -

            https://github.com/apache/incubator-calcite/pull/113

            Show
            seanhychu Sean Hsuan-Yi Chu added a comment - https://github.com/apache/incubator-calcite/pull/113
            Hide
            Permalink
            seanhychu Sean Hsuan-Yi Chu added a comment -

            Julian Hyde Can you help review? Thanks.

            Show
            seanhychu Sean Hsuan-Yi Chu added a comment - Julian Hyde Can you help review? Thanks.
            Hide
            Permalink
            julianhyde Julian Hyde added a comment -

            Looks good. A few minor things:

            1. Rename "isRequireOver" method and "requireOver" field to "requiresOver" to be consistent with "requiresOrder".

            2. I like the approach of having these properties be boolean fields. Please move the SqlRankFunction.requiresOrder field up to SqlAggFunction and add a constructor parameter. You should be able to remove requiresOrder() methods in sub-classes.

            3. Mark as "Deprecated // to be removed before 2.0" all but one constructor of SqlAggFunction. Fix existing code that uses deprecated constructors.

            Show
            julianhyde Julian Hyde added a comment - Looks good. A few minor things: 1. Rename "isRequireOver" method and "requireOver" field to "requiresOver" to be consistent with "requiresOrder". 2. I like the approach of having these properties be boolean fields. Please move the SqlRankFunction.requiresOrder field up to SqlAggFunction and add a constructor parameter. You should be able to remove requiresOrder() methods in sub-classes. 3. Mark as "Deprecated // to be removed before 2.0" all but one constructor of SqlAggFunction. Fix existing code that uses deprecated constructors.
            Hide
            Permalink
            seanhychu Sean Hsuan-Yi Chu added a comment -

            A new patch addressed the review comments above.

            Thanks for reviewing in advance!

            Show
            seanhychu Sean Hsuan-Yi Chu added a comment - A new patch addressed the review comments above. Thanks for reviewing in advance!
            Hide
            Permalink
            julianhyde Julian Hyde added a comment -

            Looks good. I've committed to my branch new-master https://github.com/julianhyde/incubator-calcite/commits/new-master. I will commit to apache master, and close this case and the pull request, when commits open following 1.4. Thanks for the patch!

            Show
            julianhyde Julian Hyde added a comment - Looks good. I've committed to my branch new-master https://github.com/julianhyde/incubator-calcite/commits/new-master . I will commit to apache master, and close this case and the pull request, when commits open following 1.4. Thanks for the patch!
            Hide
            Permalink
            julianhyde Julian Hyde added a comment -

            Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/77674a7c. Thanks for the patch, Sean Hsuan-Yi Chu!

            Show
            julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/77674a7c . Thanks for the patch, Sean Hsuan-Yi Chu !
            Hide
            Permalink
            jcamachorodriguez Jesus Camacho Rodriguez added a comment -

            Resolved in release 1.5.0 (2015-11-10)

            Show
            jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.5.0 (2015-11-10)

              People

              • Assignee:
                julianhyde Julian Hyde
                Reporter:
                seanhychu Sean Hsuan-Yi Chu
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Development