Lucene - Core
  1. Lucene - Core
  2. LUCENE-6304

Add MatchNoDocsQuery that matches no documents

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 5.0
    • Fix Version/s: 5.1, 6.0
    • Component/s: core/search
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      As a followup to LUCENE-6298, it would be nice to have an explicit MatchNoDocsQuery to indicate that no documents should be matched. This would hopefully be a better indicator than a BooleanQuery with no clauses or (even worse) null.

      1. LUCENE-6304.patch
        9 kB
        Lee Hinman
      2. LUCENE-6304.patch
        7 kB
        Lee Hinman
      3. LUCENE-6304.patch
        7 kB
        Lee Hinman
      4. LUCENE-6304.patch
        13 kB
        Lee Hinman

        Activity

        Hide
        Lee Hinman added a comment -

        Patch that adds the MatchNoDocsQuery and uses it for empty SimpleQueryParser queries as well as when a BooleanQuery is rewritten and has no clauses.

        Show
        Lee Hinman added a comment - Patch that adds the MatchNoDocsQuery and uses it for empty SimpleQueryParser queries as well as when a BooleanQuery is rewritten and has no clauses.
        Hide
        Robert Muir added a comment -

        I think its confusing we have MatchAll but not MatchNone.

        I wonder if it should just be sugar and rewrite() to a booleanquery with no clauses?
        Then it wouldn't need a weight and scorer.

        Show
        Robert Muir added a comment - I think its confusing we have MatchAll but not MatchNone. I wonder if it should just be sugar and rewrite() to a booleanquery with no clauses? Then it wouldn't need a weight and scorer.
        Hide
        Adrien Grand added a comment -

        I think the 0x1AA71190 of MatchAllDocsQuery is here to avoid that all Query impls that only wrap a boost end up with the same hash code. Maybe a cleaner way to do it would be to return Float.floatToIntBits(getBoost()) ^ getClass().hashCode().

        I wonder if it should just be sugar and rewrite() to a booleanquery with no clauses?

        +1 to rewrite to an empty BooleanQuery

        Show
        Adrien Grand added a comment - I think the 0x1AA71190 of MatchAllDocsQuery is here to avoid that all Query impls that only wrap a boost end up with the same hash code. Maybe a cleaner way to do it would be to return Float.floatToIntBits(getBoost()) ^ getClass().hashCode(). I wonder if it should just be sugar and rewrite() to a booleanquery with no clauses? +1 to rewrite to an empty BooleanQuery
        Hide
        Michael McCandless added a comment -

        +1

        Show
        Michael McCandless added a comment - +1
        Hide
        Lee Hinman added a comment -

        New patch that changes MatchNoDocsQuery to rewrite to an empty BooleanQuery. Also removes the nocommit as per Adrien's suggestion

        Show
        Lee Hinman added a comment - New patch that changes MatchNoDocsQuery to rewrite to an empty BooleanQuery. Also removes the nocommit as per Adrien's suggestion
        Hide
        Robert Muir added a comment -

        is the hashcode/equals stuff needed here or can the superclass impls in Query be used? They seem to already have this logic.

        In the tests, i would add a call to QueryUtils.check(q) to one of your matchnodocsqueries. This will do some tests on hashcode/equals.

        Show
        Robert Muir added a comment - is the hashcode/equals stuff needed here or can the superclass impls in Query be used? They seem to already have this logic. In the tests, i would add a call to QueryUtils.check(q) to one of your matchnodocsqueries. This will do some tests on hashcode/equals.
        Hide
        Lee Hinman added a comment -

        is the hashcode/equals stuff needed here or can the superclass impls in Query be used?

        The hashcode is required at least, because otherwise the QueryUtils.check(q) fails because both the MatchNoDocsQuery and the superclass Query have the same hashcode, and the anonymous "WhackyQuery" that QueryUtils creates shares the same hash code, so QueryUtils.checkUnequal() fails.

        The .equals() stuff is not required though, it can use the superclass implementation. I've attached a new patch that does this.

        Show
        Lee Hinman added a comment - is the hashcode/equals stuff needed here or can the superclass impls in Query be used? The hashcode is required at least, because otherwise the QueryUtils.check(q) fails because both the MatchNoDocsQuery and the superclass Query have the same hashcode, and the anonymous "WhackyQuery" that QueryUtils creates shares the same hash code, so QueryUtils.checkUnequal() fails. The .equals() stuff is not required though, it can use the superclass implementation. I've attached a new patch that does this.
        Hide
        Adrien Grand added a comment -

        Feels wrong to me to override hashCode but not equals. I think we should move this class part of hashCode() to Query.hashCode()? (ie. return Float.floatToIntBits(getBoost()) ^ getClass().hashCode()

        If it is controversial then I'm happy with the previous patch that overrides both equals and hashCode().

        Show
        Adrien Grand added a comment - Feels wrong to me to override hashCode but not equals. I think we should move this class part of hashCode() to Query.hashCode()? (ie. return Float.floatToIntBits(getBoost()) ^ getClass().hashCode() If it is controversial then I'm happy with the previous patch that overrides both equals and hashCode().
        Hide
        David Smiley added a comment -

        +1 Adrien.

        Show
        David Smiley added a comment - +1 Adrien.
        Hide
        Lee Hinman added a comment - - edited

        Adrien: I agree about having the hashCode.

        Here is a new patch that doesn't override equals or hashCode and changes Query to use the class in the hashCode method as Adrien suggested.

        Show
        Lee Hinman added a comment - - edited Adrien: I agree about having the hashCode. Here is a new patch that doesn't override equals or hashCode and changes Query to use the class in the hashCode method as Adrien suggested.
        Hide
        Robert Muir added a comment -

        Thanks Lee, I like this.

        When i see code overriding hashcode/equals and not calling super.hashcode/super.equals, its a bad sign.

        We should commit this one, and remove duplicated logic and hardcoded constants in e.g. TermQuery and all other places in a followup.

        Show
        Robert Muir added a comment - Thanks Lee, I like this. When i see code overriding hashcode/equals and not calling super.hashcode/super.equals, its a bad sign. We should commit this one, and remove duplicated logic and hardcoded constants in e.g. TermQuery and all other places in a followup.
        Hide
        Lee Hinman added a comment -

        Robert: +1, I opened LUCENE-6333 for this, I'll work on a patch.

        Show
        Lee Hinman added a comment - Robert: +1, I opened LUCENE-6333 for this, I'll work on a patch.
        Hide
        ASF subversion and git services added a comment -

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

        LUCENE-6304: Add MatchNoDocsQuery.

        Show
        ASF subversion and git services added a comment - Commit 1663899 from Adrien Grand in branch 'dev/trunk' [ https://svn.apache.org/r1663899 ] LUCENE-6304 : Add MatchNoDocsQuery.
        Hide
        Adrien Grand added a comment -

        Committed. Thanks Lee!

        Show
        Adrien Grand added a comment - Committed. Thanks Lee!
        Hide
        ASF subversion and git services added a comment -

        Commit 1663901 from Adrien Grand in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1663901 ]

        LUCENE-6304: Add MatchNoDocsQuery.

        Show
        ASF subversion and git services added a comment - Commit 1663901 from Adrien Grand in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1663901 ] LUCENE-6304 : Add MatchNoDocsQuery.
        Hide
        Timothy Potter added a comment -

        Bulk close after 5.1 release

        Show
        Timothy Potter added a comment - Bulk close after 5.1 release

          People

          • Assignee:
            Unassigned
            Reporter:
            Lee Hinman
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development