Lucene - Core
  1. Lucene - Core
  2. LUCENE-5805

QueryNodeImpl.removeFromParent does a lot of work without any effect

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.7.2, 4.9
    • Fix Version/s: 5.3, 6.0
    • Component/s: modules/queryparser
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      The method removeFromParent of QueryNodeImpl, calls getChildren on the parent and removes any occurrence of "this" from the result.

      However, since a few releases, getChildren returns a copy of the children list, so the code has no effect (except creating a copy of the children list which will then be thrown away).
      Even worse, since setChildren calls removeFromParent on any previous child, setChildren now has a complexity of O(n^2) and creates a lot of throw-away copies of the children list (for nodes with a lot of children)

      public void removeFromParent() {
          if (this.parent != null) {
            List<QueryNode> parentChildren = this.parent.getChildren();
            Iterator<QueryNode> it = parentChildren.iterator();
            
            while (it.hasNext()) {
              if (it.next() == this) {
                it.remove();
              }
            }
            
            this.parent = null;
          }
        }
      

        Issue Links

          Activity

          Hide
          Cao Manh Dat added a comment -

          Here is patch file for this issue.

          Show
          Cao Manh Dat added a comment - Here is patch file for this issue.
          Hide
          Cao Manh Dat added a comment -

          Just thumb this up for committers can see

          Show
          Cao Manh Dat added a comment - Just thumb this up for committers can see
          Hide
          Michael McCandless added a comment -

          Oh this is quite bad, thanks Cao Manh Dat

          Show
          Michael McCandless added a comment - Oh this is quite bad, thanks Cao Manh Dat
          Hide
          ASF subversion and git services added a comment -

          Commit 1683839 from Michael McCandless in branch 'dev/trunk'
          [ https://svn.apache.org/r1683839 ]

          LUCENE-5805: QueryNodeImpl.removeFromParent was doing nothing, in a costly way

          Show
          ASF subversion and git services added a comment - Commit 1683839 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1683839 ] LUCENE-5805 : QueryNodeImpl.removeFromParent was doing nothing, in a costly way
          Hide
          ASF subversion and git services added a comment -

          Commit 1683853 from Michael McCandless in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1683853 ]

          LUCENE-5805: QueryNodeImpl.removeFromParent was doing nothing, in a costly way

          Show
          ASF subversion and git services added a comment - Commit 1683853 from Michael McCandless in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1683853 ] LUCENE-5805 : QueryNodeImpl.removeFromParent was doing nothing, in a costly way
          Hide
          Michael McCandless added a comment -

          Thanks Christoph and Cao Manh Dat!

          Show
          Michael McCandless added a comment - Thanks Christoph and Cao Manh Dat !
          Hide
          Shalin Shekhar Mangar added a comment -

          Bulk close for 5.3.0 release

          Show
          Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

            People

            • Assignee:
              Michael McCandless
              Reporter:
              Christoph Kaser
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development