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

Further extend simplify for reducing expressions

    Details

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

      Description

      We would extend expression simplification, e.g.:

      • add walker to perform recursive simplification of expressions for Filter operators, and
      • extends simplification for CASE expressions to cover more cases e.g. if all return values are equal (including ELSE), CASE can be removed

        Issue Links

          Activity

          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          I created a PR with the code in : https://github.com/apache/calcite/pull/270

          Julian Hyde, could you review it? Thanks

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - I created a PR with the code in : https://github.com/apache/calcite/pull/270 Julian Hyde , could you review it? Thanks
          Hide
          julianhyde Julian Hyde added a comment -

          I see you fixed CALCITE-1290. Can you break out that fix into a separate commit, and add some tests for it? Maybe carry over some of the tests you added for Hive.

          Show
          julianhyde Julian Hyde added a comment - I see you fixed CALCITE-1290 . Can you break out that fix into a separate commit, and add some tests for it? Maybe carry over some of the tests you added for Hive.
          Hide
          julianhyde Julian Hyde added a comment -

          Regarding the other simplifications. It doesn't look as if you have added tests for call of the new cases you handle. Can you please add those tests?

          Show
          julianhyde Julian Hyde added a comment - Regarding the other simplifications. It doesn't look as if you have added tests for call of the new cases you handle. Can you please add those tests?
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - - edited

          Julian Hyde, I have updated the PR with CALCITE-1290 taken out and extended test coverage (existing ones are already covering some of the extensions as you can see).

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - - edited Julian Hyde , I have updated the PR with CALCITE-1290 taken out and extended test coverage (existing ones are already covering some of the extensions as you can see).
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/e8729597 .
          Hide
          julianhyde Julian Hyde added a comment - - edited

          Jesus Camacho Rodriguez, In RexUtil, why did you write

                    terms.remove(i);
                    terms.add(i, term);
          

          instead of

                    terms.set(i, term);
          

          On an ArrayList, the former are two O(n) operations, and the latter is an O(1) operation.

          Show
          julianhyde Julian Hyde added a comment - - edited Jesus Camacho Rodriguez , In RexUtil, why did you write terms.remove(i); terms.add(i, term); instead of terms.set(i, term); On an ArrayList, the former are two O(n) operations, and the latter is an O(1) operation.
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          You are right, I did not even realize, I should have used set.

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - You are right, I did not even realize, I should have used set .
          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - Fix up in : http://git-wip-us.apache.org/repos/asf/calcite/commit/216035f
          Hide
          jcamachorodriguez Jesus Camacho Rodriguez added a comment -

          Resolved in release 1.9.0 (2016-09-22)

          Show
          jcamachorodriguez Jesus Camacho Rodriguez added a comment - Resolved in release 1.9.0 (2016-09-22)

            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