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

isDeterministic only explores top RexCall

    Details

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

      Issue Links

        Activity

        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -
        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Julian Hyde , could you take a look? https://github.com/apache/calcite/pull/286 Thanks
        Hide
        julianhyde Julian Hyde added a comment -

        Any ideas how we could create a SQL test? Doesn't seem that it would be easy.

        +1

        Show
        julianhyde Julian Hyde added a comment - Any ideas how we could create a SQL test? Doesn't seem that it would be easy. +1
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/ddd70fa .

        Julian Hyde, in Hive it was exposed by a query that used the rand() function to filter:

        rand() > 30 AND rand() <= 30

        Since the top operator of each conjunct is > and <=, respectively, it was considering those conjuncts deterministic and it was folding the full predicate incorrectly to false.

        I think a similar SQL test could be added to Calcite, but I took a quick look and I did not find a function similar to rand() or other non deterministic; that is why I directly added the unit tests.

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/ddd70fa . Julian Hyde , in Hive it was exposed by a query that used the rand() function to filter: rand() > 30 AND rand() <= 30 Since the top operator of each conjunct is > and <=, respectively, it was considering those conjuncts deterministic and it was folding the full predicate incorrectly to false . I think a similar SQL test could be added to Calcite, but I took a quick look and I did not find a function similar to rand() or other non deterministic; that is why I directly added the unit tests.
        Hide
        julianhyde Julian Hyde added a comment -

        When we have a RANDOM function (see CALCITE-1414) we should add a SQL test.

        Show
        julianhyde Julian Hyde added a comment - When we have a RANDOM function (see CALCITE-1414 ) we should add a SQL test.
        Hide
        jcamachorodriguez Jesus Camacho Rodriguez added a comment -

        Resolved in release 1.10.0 (2016-10-12).

        Show
        jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.10.0 (2016-10-12).

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development