Lucene - Core
  1. Lucene - Core
  2. LUCENE-3238

SpanMultiTermQueryWrapper with Prefix Query issue

    Details

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

      Windows 7, JDK 1.6

    • Lucene Fields:
      New, Patch Available

      Description

      If we try to do a search with SpanQuery and a PrefixQuery this message is returned:

      "You can only use SpanMultiTermQueryWrapper with a suitable SpanRewriteMethod."

      The problem is in the WildcardQuery rewrite function.

      If the wildcard query is a prefix, a new prefix query is created, the rewrite method is set with the SpanRewriteMethod and the prefix query is returned.

      But, that's the rewritten prefix query which should be returned:

      • return rewritten;
        + return rewritten.rewrite(reader);

      I will attach a patch with a unit test included.

      1. LUCENE-3238.patch
        2 kB
        ludovic Boutros
      2. LUCENE-3238.patch
        4 kB
        Robert Muir
      3. LUCENE-3238.patch
        8 kB
        Robert Muir

        Activity

        Hide
        ludovic Boutros added a comment -

        Here is the patch for the branch 3x.

        Show
        ludovic Boutros added a comment - Here is the patch for the branch 3x.
        Hide
        Robert Muir added a comment -

        Hi, definitely a bug, thank you!

        In my opinion, WildcardQuery should not try to override MultiTermQuery's rewrite here, it causes too many problems.

        Instead, in this case it should just return a PrefixTermEnum... this is the way we handle these things in trunk and I think we should fix it here the same way.

        Show
        Robert Muir added a comment - Hi, definitely a bug, thank you! In my opinion, WildcardQuery should not try to override MultiTermQuery's rewrite here, it causes too many problems. Instead, in this case it should just return a PrefixTermEnum... this is the way we handle these things in trunk and I think we should fix it here the same way.
        Hide
        Uwe Schindler added a comment -

        The fix is fine, but in my optinion the problem should be solved differently.

        I would like to make the rewrite method in MultiTermQuery final to prevent override. To correctly fix the issue, WildcardQuery only needs to return a PrefixTermEnum in its getEnum method. This is already fixed in Lucene 4.0.

        From looking at the code, SpanMultiTermQueryWrapper would not work correct in all cases, if the underlying query overwrites rewrite(), as the rewritten query would again have the wrong type.

        Show
        Uwe Schindler added a comment - The fix is fine, but in my optinion the problem should be solved differently. I would like to make the rewrite method in MultiTermQuery final to prevent override. To correctly fix the issue, WildcardQuery only needs to return a PrefixTermEnum in its getEnum method. This is already fixed in Lucene 4.0. From looking at the code, SpanMultiTermQueryWrapper would not work correct in all cases, if the underlying query overwrites rewrite(), as the rewritten query would again have the wrong type.
        Hide
        Uwe Schindler added a comment -

        Patch is fine! Funny overlap, we both responded with same answer

        Show
        Uwe Schindler added a comment - Patch is fine! Funny overlap, we both responded with same answer
        Hide
        Robert Muir added a comment -

        Same patch: except I made MultiTermQuery's rewrite() final.

        In my opinion, this is a good backwards break, it will only fix bugs in someone's code if they have a custom MultiTermQuery: its very tricky to override this (e.g. you must pass along boost, rewriteMethod, ...), and when you do, still might cause problems (like this Span issue).

        Its also much easier to just return a simpler enum.

        Show
        Robert Muir added a comment - Same patch: except I made MultiTermQuery's rewrite() final. In my opinion, this is a good backwards break, it will only fix bugs in someone's code if they have a custom MultiTermQuery: its very tricky to override this (e.g. you must pass along boost, rewriteMethod, ...), and when you do, still might cause problems (like this Span issue). Its also much easier to just return a simpler enum.
        Hide
        ludovic Boutros added a comment -

        I understand the patch, that's better indeed.

        Thanks.

        Show
        ludovic Boutros added a comment - I understand the patch, that's better indeed. Thanks.
        Hide
        Robert Muir added a comment -

        thanks Ludovic, nice catch!

        Show
        Robert Muir added a comment - thanks Ludovic, nice catch!
        Hide
        Robert Muir added a comment -

        bulk close for 3.3

        Show
        Robert Muir added a comment - bulk close for 3.3

          People

          • Assignee:
            Robert Muir
            Reporter:
            ludovic Boutros
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development