Lucene - Core
  1. Lucene - Core
  2. LUCENE-5502

equals method of TermsFilter might equate two different filters

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.7
    • Fix Version/s: 4.7.1, 4.8, 6.0
    • Component/s: core/query/scoring
    • Labels:
      None
    • Lucene Fields:
      New, Patch Available

      Description

      If two terms filters have 1) the same number of terms, 2) use the same field in all these terms and 3) term values happened to have the same hash codes, these two filter are considered to be equal as long as the first term is the same in both filters.

      1. LUCENE-5502.patch
        3 kB
        Igor Motov
      2. LUCENE-5502.patch
        2 kB
        Igor Motov
      3. LUCENE-5502.patch
        2 kB
        Igor Motov

        Activity

        Hide
        Igor Motov added a comment -

        Test and patch for the issue.

        Show
        Igor Motov added a comment - Test and patch for the issue.
        Hide
        Adrien Grand added a comment -

        Thank you Igor, I think this is a very bad bug!

        Your fix looks good but I think there is still an issue given that offsets are not used for the equality test. So (the very unlikely yet possible case of having) two filters that have the same fields, same number of terms, same hash code and same term bytes but different offsets would still be considered equal.

        I think we should try to make this equals method easier to read in order not to have similar issues in the future: maybe a start could be try to replace the comparison of the termsAndFields array and the termBytes array with a call to Arrays.equals.

        Show
        Adrien Grand added a comment - Thank you Igor, I think this is a very bad bug! Your fix looks good but I think there is still an issue given that offsets are not used for the equality test. So (the very unlikely yet possible case of having) two filters that have the same fields, same number of terms, same hash code and same term bytes but different offsets would still be considered equal. I think we should try to make this equals method easier to read in order not to have similar issues in the future: maybe a start could be try to replace the comparison of the termsAndFields array and the termBytes array with a call to Arrays.equals .
        Hide
        Igor Motov added a comment -

        Thanks Adrien. You are right, I missed offsets. Here is an updated version. I cannot use Arrays.equals for termsBytes and offsets because we compare only parts of the arrays, but I can switch to ArrayUtil.equals if you think it would make more sense.

        Show
        Igor Motov added a comment - Thanks Adrien. You are right, I missed offsets. Here is an updated version. I cannot use Arrays.equals for termsBytes and offsets because we compare only parts of the arrays, but I can switch to ArrayUtil.equals if you think it would make more sense.
        Hide
        Igor Motov added a comment -

        Updated patch with ArrayUtil.equals

        Show
        Igor Motov added a comment - Updated patch with ArrayUtil.equals
        Hide
        Adrien Grand added a comment -

        Thank you Igor, the patch looks good to me! Simon Willnauer, I think you worked on this filter in the past, so you might want to give a look at this patch?

        Show
        Adrien Grand added a comment - Thank you Igor, the patch looks good to me! Simon Willnauer , I think you worked on this filter in the past, so you might want to give a look at this patch?
        Hide
        Simon Willnauer added a comment -

        LGTM as well! thanks for fixing it!

        Show
        Simon Willnauer added a comment - LGTM as well! thanks for fixing it!
        Hide
        Adrien Grand added a comment -

        Thanks for the review, Simon. I'll commit tomorrow if there is no objection until then.

        Show
        Adrien Grand added a comment - Thanks for the review, Simon. I'll commit tomorrow if there is no objection until then.
        Hide
        ASF subversion and git services added a comment -

        Commit 1576223 from Adrien Grand in branch 'dev/trunk'
        [ https://svn.apache.org/r1576223 ]

        LUCENE-5502: Fix TermsFilter.equals.

        Show
        ASF subversion and git services added a comment - Commit 1576223 from Adrien Grand in branch 'dev/trunk' [ https://svn.apache.org/r1576223 ] LUCENE-5502 : Fix TermsFilter.equals.
        Hide
        ASF subversion and git services added a comment -

        Commit 1576227 from Adrien Grand in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1576227 ]

        LUCENE-5502: Fix TermsFilter.equals.

        Show
        ASF subversion and git services added a comment - Commit 1576227 from Adrien Grand in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1576227 ] LUCENE-5502 : Fix TermsFilter.equals.
        Hide
        Adrien Grand added a comment -

        Committed, thanks Igor!

        Show
        Adrien Grand added a comment - Committed, thanks Igor!
        Hide
        Robert Muir added a comment -

        reopen for 4.7.1 backport

        Show
        Robert Muir added a comment - reopen for 4.7.1 backport
        Hide
        ASF subversion and git services added a comment -

        Commit 1578523 from Robert Muir in branch 'dev/branches/lucene_solr_4_7'
        [ https://svn.apache.org/r1578523 ]

        LUCENE-5502: Fix TermsFilter.equals

        Show
        ASF subversion and git services added a comment - Commit 1578523 from Robert Muir in branch 'dev/branches/lucene_solr_4_7' [ https://svn.apache.org/r1578523 ] LUCENE-5502 : Fix TermsFilter.equals
        Hide
        Steve Rowe added a comment -

        Bulk close 4.7.1 issues

        Show
        Steve Rowe added a comment - Bulk close 4.7.1 issues

          People

          • Assignee:
            Adrien Grand
            Reporter:
            Igor Motov
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development