Lucene - Core
  1. Lucene - Core
  2. LUCENE-3600

BlockJoinQuery advance fails on an assert in case of a single parent with child segment

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.5, 4.0-ALPHA
    • Fix Version/s: 3.6, 4.0-ALPHA
    • Component/s: modules/join
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      The BlockJoinQuery will fail on an assert when advance in called on a segment with a single parent with a child. The call to parentBits.prevSetBit(parentTarget - 1) will cause -1 to be returned, and the assert will fail, though its valid. Just removing the assert fixes the problem, since nextDoc will handle it properly.

      Also, I don't understand the "assert parentTarget != 0;", with a comment of each parent must have one child. There isn't really a reason to add this constraint, as far as I can tell..., just call nextDoc in this case, no?

        Activity

        Hide
        Simon Willnauer added a comment -

        added fixVersion 3.6 & 4.0

        Show
        Simon Willnauer added a comment - added fixVersion 3.6 & 4.0
        Hide
        Michael McCandless added a comment -

        Hmm, the docID passed to BJQ's advance should be from the parent docID space (ie, obtained from a parent-level query you're ANDing with).

        I can't get a test case to fail, if indeed the parent has a child doc...

        Do you have a case (Query) where a non-parent docID is passed to BJQ.advance?

        Separately, I think you're right: if we change that "assert parentTarget != 0" instead to "if (parentDoc == 0) return nextDoc();", then BJQ can support parents w/ no children (which can never match, since BJQ does effectively an inner join).

        Show
        Michael McCandless added a comment - Hmm, the docID passed to BJQ's advance should be from the parent docID space (ie, obtained from a parent-level query you're ANDing with). I can't get a test case to fail, if indeed the parent has a child doc... Do you have a case (Query) where a non-parent docID is passed to BJQ.advance? Separately, I think you're right: if we change that "assert parentTarget != 0" instead to "if (parentDoc == 0) return nextDoc();", then BJQ can support parents w/ no children (which can never match, since BJQ does effectively an inner join).
        Hide
        Shay Banon added a comment -

        Thanks for fixing the parent with no children!. I think there is still a problem with the assert on a single segment with one parent and child. I just took your test case: testAdvanceSingleParentSingleChild and it fails for me with asserts enabled on the "assert prevParentDoc >= parentDoc;" (I copied over the code, having problems with Lucene IDEA project config now...).

        Imagine there is a single parent and single child. Child is docId 0, and parent is docId 1. A filter for the parent is used in conjunction with the child query. The parent filter is used first, and returns docId 1, which then the BlockJoinQuery advance is called. But, because its the first parent, this call: "final int prevParentDoc = parentBits.prevSetBit(parentTarget - 1);" will cause -1 to be returned, which will cause this assert: "assert prevParentDoc >= parentDoc;" to fail (parentDoc is 0). Just removing the assert fixes the problem.

        Show
        Shay Banon added a comment - Thanks for fixing the parent with no children!. I think there is still a problem with the assert on a single segment with one parent and child. I just took your test case: testAdvanceSingleParentSingleChild and it fails for me with asserts enabled on the "assert prevParentDoc >= parentDoc;" (I copied over the code, having problems with Lucene IDEA project config now...). Imagine there is a single parent and single child. Child is docId 0, and parent is docId 1. A filter for the parent is used in conjunction with the child query. The parent filter is used first, and returns docId 1, which then the BlockJoinQuery advance is called. But, because its the first parent, this call: "final int prevParentDoc = parentBits.prevSetBit(parentTarget - 1);" will cause -1 to be returned, which will cause this assert: "assert prevParentDoc >= parentDoc;" to fail (parentDoc is 0). Just removing the assert fixes the problem.
        Hide
        Michael McCandless added a comment -

        Shay, maybe you missed copying over init'ing parentDoc to -1? (I don't see this assert failing – "ant test" runs w/ asserts).

        Show
        Michael McCandless added a comment - Shay, maybe you missed copying over init'ing parentDoc to -1? (I don't see this assert failing – "ant test" runs w/ asserts).
        Hide
        Shay Banon added a comment -

        Ahh, I see, missed the change of setting it to -1 for calling nextDoc on parentId 0, cool.

        Show
        Shay Banon added a comment - Ahh, I see, missed the change of setting it to -1 for calling nextDoc on parentId 0, cool.

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Shay Banon
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development