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

VolcanoPlanner should clear ruleNames in order to avoid rule name conflicting error

    Details

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

      Description

      When VolcanoPlanner is called multiple times through Frameworks, it may encounter rule name conflicting error, if the first ruleset and second ruleset have rules with identical name.

      The fix is to simply clear the set of ruleNames in VolcanoPlanner.clear() method.

        Issue Links

          Activity

          Hide
          julianhyde Julian Hyde added a comment -

          Resolved in release 1.6.0 (2016-01-22).

          Show
          julianhyde Julian Hyde added a comment - Resolved in release 1.6.0 (2016-01-22).
          Show
          jni Jinfeng Ni added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/d3c5acd3
          Hide
          julianhyde Julian Hyde added a comment -

          I realized I was mistaken about Windows, so I removed the comment. Sorry for the confusion!

          +1 New commit looks good.

          Show
          julianhyde Julian Hyde added a comment - I realized I was mistaken about Windows, so I removed the comment. Sorry for the confusion! +1 New commit looks good.
          Hide
          jni Jinfeng Ni added a comment -

          Julian Hyde, thanks for the reminding and review comments. I revised the PR based on your comments.

          Somehow, I remember you said the new unit test failed on windows. But I just could not find a windows machine to see what's going on. I looked the comments on PR, and did not see the mentioning of failure on windows. If it still fails on windows, please let me know; I'll try to borrow a windows instance to run the test. Thanks.

          Show
          jni Jinfeng Ni added a comment - Julian Hyde , thanks for the reminding and review comments. I revised the PR based on your comments. Somehow, I remember you said the new unit test failed on windows. But I just could not find a windows machine to see what's going on. I looked the comments on PR, and did not see the mentioning of failure on windows. If it still fails on windows, please let me know; I'll try to borrow a windows instance to run the test. Thanks.
          Hide
          julianhyde Julian Hyde added a comment -

          Jinfeng Ni, Any progress? +1 to commit after you fix the cosmetic issues I raised in the PR.

          Show
          julianhyde Julian Hyde added a comment - Jinfeng Ni , Any progress? +1 to commit after you fix the cosmetic issues I raised in the PR.
          Hide
          julianhyde Julian Hyde added a comment -

          See my review comments on the PR

          Show
          julianhyde Julian Hyde added a comment - See my review comments on the PR

            People

            • Assignee:
              jni Jinfeng Ni
              Reporter:
              jni Jinfeng Ni
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development