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

Do not add SINGLE_VALUE aggregate function to a sub-query that will never return more than one row

    Details

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

      Description

      In the following query, the subquery is producing a scalar value but the logical plan still creates a SqlSingleValueAggFunction.

      select r_regionkey from region
         where r_regionkey > (select min(n_regionkey) * 2 from nation);
      

      If the aggregate is just min(n_regionkey) instead of the expression min(n_regionkey) * 2 then no SqlSingleValueAggFunction is created. Ideally, both should behave the same.

      This is not necessarily a bug but it does create a burden on the underlying execution engine to support this function even in cases where it may not be needed.

        Activity

        Hide
        julianhyde Julian Hyde added a comment - - edited

        I'd say it's a bug, because we are generating a more complex, and perhaps less efficient, expression than we need.

        It could be fixed (a) by recognizing the pattern at sql-to-rel time, of (b) using a planner rule. For (b) it is not safe to rely on estimated row count, but you could rely on RelMetadataQuery.areColumnsUnique or RelMetadataQuery.getUniqueKeys for an empty list of columns.

        Show
        julianhyde Julian Hyde added a comment - - edited I'd say it's a bug, because we are generating a more complex, and perhaps less efficient, expression than we need. It could be fixed (a) by recognizing the pattern at sql-to-rel time, of (b) using a planner rule. For (b) it is not safe to rely on estimated row count, but you could rely on RelMetadataQuery.areColumnsUnique or RelMetadataQuery.getUniqueKeys for an empty list of columns.
        Hide
        seanhychu Sean Hsuan-Yi Chu added a comment -

        I am working on this issue.

        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - I am working on this issue.
        Hide
        seanhychu Sean Hsuan-Yi Chu added a comment -

        Sent a pull request:
        https://github.com/apache/incubator-calcite/pull/80

        Util.java has a new method to determine if the output for a subquery is scalar. Essentially, when this method suggests scalar output, no necessity to add SqlSingleValueAggFunction.

        Show
        seanhychu Sean Hsuan-Yi Chu added a comment - Sent a pull request: https://github.com/apache/incubator-calcite/pull/80 Util.java has a new method to determine if the output for a subquery is scalar. Essentially, when this method suggests scalar output, no necessity to add SqlSingleValueAggFunction.
        Hide
        amansinha100 Aman Sinha added a comment -

        It looks like Julian has reviewed this. I agree with his comments. Sean Hsuan-Yi Chu I still think that if someone was to accidentally use the new Util.isSingleValue() function for a subquery that had an aggregate function and a group-by and the caller did not check for the grouping list, they would incorrectly get True.

        Show
        amansinha100 Aman Sinha added a comment - It looks like Julian has reviewed this. I agree with his comments. Sean Hsuan-Yi Chu I still think that if someone was to accidentally use the new Util.isSingleValue() function for a subquery that had an aggregate function and a group-by and the caller did not check for the grouping list, they would incorrectly get True.
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/f076a9ba .
        Show
        julianhyde Julian Hyde added a comment - I added some tests for scalar sub-queries in https://git1-us-west.apache.org/repos/asf?p=incubator-calcite.git;a=blob;f=core/src/test/resources/sql/scalar.oq;h=103df2926309e18d380b2bff1e89875ba6287086;hb=f107218b956a1b25d11c8d06c6378bdd22c75378 so I think we're good.
        Hide
        julianhyde Julian Hyde added a comment -

        Resolved in release 1.3.0-incubating (2015-05-30).

        Show
        julianhyde Julian Hyde added a comment - Resolved in release 1.3.0-incubating (2015-05-30).

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            amansinha100 Aman Sinha
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development