Details

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

      Description

      When I use drill, I found a bug when calcite parsing sql.
      in org.apache.calcite.rex.RexLocalRef, there is a method named createName(int index), when this method is called distributed, you find that you're not, NAMES may not be {$0 $1 .... $n}, but {$0...$29,$30...$59,$30...}
      NAMES is SelfPopulatingList.class liked Thread-safe list, but unfortunately it's Thread-unsafe list, although addAll() has a lock, this method is Thread-safe, but in method get(int index)

      @Override public String get(int index) {
            for (;;) {
              try {
                return super.get(index);
              } catch (IndexOutOfBoundsException e) {
                if (index < 0) {
                  throw new IllegalArgumentException();
                }
                addAll(
                    fromTo(
                        prefix, size(), Math.max(index + 1, size() * 2)));
              }
            }
          }
      

      method fromTo() is not thread-safe, so it get error. So as you can see, {$30...$59} is added repeatedly.

      bugfix:
      There are several ways to solve this bug.
      one is, add lock before addAll(), seemed like

      @Override public String get(int index) {
            for (;;) {
              try {
                return super.get(index);
              } catch (IndexOutOfBoundsException e) {
                if (index < 0) {
                  throw new IllegalArgumentException();
                }
      /*********lock************/
                addAll(
                    fromTo(
                        prefix, size(), Math.max(index + 1, size() * 2)));
              }
      /*********unlock************/
            }
          }
      

      But there is over design, catch(e) is not good.

        Issue Links

          Activity

          Hide
          julianhyde Julian Hyde added a comment -

          I've pushed a tentative fix to https://github.com/julianhyde/calcite/commits/master. Let me know whether it works.

          Show
          julianhyde Julian Hyde added a comment - I've pushed a tentative fix to https://github.com/julianhyde/calcite/commits/master . Let me know whether it works.
          Show
          julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/calcite/commit/a850c41e .
          Hide
          huntersjm huntersjm added a comment -

          I've checked it, it works now.

          Show
          huntersjm huntersjm added a comment - I've checked it, it works now.
          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).

            People

            • Assignee:
              julianhyde Julian Hyde
              Reporter:
              huntersjm huntersjm
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development