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

Validator error when resolving OVER clause of JOIN query

    Details

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

      Description

      A query such as :
      select sum(t1.deptno) over(partition by t1.deptno)
      from emp t1, emp t2

      would raise exception:
      org.apache.calcite.sql.validate.SqlValidatorException: Column 'EMPNO' is ambiguous.

      Calcite cannot point the column in OVER-CLAUSE to the JOINed table.

        Activity

        Hide
        seanhychu Sean Hsuan-Yi Chu added a comment -

        I would like to work on this bug

        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - I would like to work on this bug
        Hide
        julianhyde Julian Hyde added a comment -

        I've assigned it to you. Change the status to "in progress" if you're working on it.

        Show
        julianhyde Julian Hyde added a comment - I've assigned it to you. Change the status to "in progress" if you're working on it.
        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - https://github.com/apache/incubator-calcite/pull/93
        Hide
        julianhyde Julian Hyde added a comment - - edited

        It struck me as strange that you needed to qualify a column name when a namespace was also passed along. Then I noticed this comment in SqlWindow.isTableSorted:

            // REVIEW: jhyde, 2007/11/7: This is the only use of
            // findAllColumnNames. Find a better way to detect monotonicity, then
            // remove that method.
        

        Basically, we shouldn't be using SqlMoniker. Monotonicity detection should be using column name and namespace, like the rest of the validator code. A namespace represents a table, and it may map to single or multiple strings: EMP, SCHEMA.EMP, DB.SCHEMA.EMP, T1 are all examples.

        That comment basically predicted this problem, so now it's time to fix it.

        Show
        julianhyde Julian Hyde added a comment - - edited It struck me as strange that you needed to qualify a column name when a namespace was also passed along. Then I noticed this comment in SqlWindow.isTableSorted: // REVIEW: jhyde, 2007/11/7: This is the only use of // findAllColumnNames. Find a better way to detect monotonicity, then // remove that method. Basically, we shouldn't be using SqlMoniker. Monotonicity detection should be using column name and namespace, like the rest of the validator code. A namespace represents a table, and it may map to single or multiple strings: EMP, SCHEMA.EMP, DB.SCHEMA.EMP, T1 are all examples. That comment basically predicted this problem, so now it's time to fix it.
        Hide
        seanhychu Sean Hsuan-Yi Chu added a comment -

        I am not sure if we can remove findAllColumnNames() entirely?

        Before moving into Monotonicity detection, we still need to get all the column names right? It is where findAllColumnNames() is necessary?

        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - I am not sure if we can remove findAllColumnNames() entirely? Before moving into Monotonicity detection, we still need to get all the column names right? It is where findAllColumnNames() is necessary?
        Hide
        julianhyde Julian Hyde added a comment -

        I put in another fix which doesn't use findAllColumnNames. See https://github.com/julianhyde/incubator-calcite/tree/754-resolve.

        Show
        julianhyde Julian Hyde added a comment - I put in another fix which doesn't use findAllColumnNames. See https://github.com/julianhyde/incubator-calcite/tree/754-resolve .
        Hide
        julianhyde Julian Hyde added a comment -
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/87acda31 . Sean Hsuan-Yi Chu , thanks for the patch! In the end, I used your test case (see http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/ebce955d ) but developed a better fix.
        Hide
        seanhychu Sean Hsuan-Yi Chu added a comment -

        The solution looks more concise. Thanks!

        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - The solution looks more concise. Thanks!
        Hide
        jnadeau Jacques Nadeau added a comment -

        Resolved in release 1.4.0-incubating (2015-08-23)

        Show
        jnadeau Jacques Nadeau added a comment - Resolved in release 1.4.0-incubating (2015-08-23)

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development