Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.2, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      See the mail thread "is this lucene 4.1.0 bug in PerFieldPostingsFormat".

      If anyone seriously thinks adding a null check to ctor will cause measurable slowdown to things like regexp or wildcards, they should have their head examined.

      All queries should just check this crap in ctor and throw exceptions if parameters are invalid.

      1. LUCENE-6345.patch
        16 kB
        Lee Hinman
      2. LUCENE-6345.patch
        16 kB
        Lee Hinman

        Activity

        Hide
        Michael McCandless added a comment -

        +1

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

        I'm going to work on this.

        Looking through the code, I see a mixture of:

        Term t = Objects.requireNonNull(term);
        

        As well as:

        if (term == null) {
          throw new IllegalArgumentException("Term must not be null");
        }
        

        Any particular preference here? I think an explicit message is nicer but I can go either way. If no one has an opinion about it I'll pick one and go with it

        Show
        Lee Hinman added a comment - I'm going to work on this. Looking through the code, I see a mixture of: Term t = Objects.requireNonNull(term); As well as: if (term == null) { throw new IllegalArgumentException("Term must not be null"); } Any particular preference here? I think an explicit message is nicer but I can go either way. If no one has an opinion about it I'll pick one and go with it
        Hide
        Lee Hinman added a comment -

        Here's a patch that adds a lot of null checks to Querys as well as things like BooleanClause.

        It doesn't add tests for every single query for this (yet), though I see there are some already for FilteredQuery.

        Should I work on adding tests for every query type for this, or are adding the checks alone sufficient?

        Show
        Lee Hinman added a comment - Here's a patch that adds a lot of null checks to Querys as well as things like BooleanClause . It doesn't add tests for every single query for this (yet), though I see there are some already for FilteredQuery . Should I work on adding tests for every query type for this, or are adding the checks alone sufficient?
        Hide
        Lee Hinman added a comment -

        Updated patch that re-adds an assert that I removed mistakenly.

        Show
        Lee Hinman added a comment - Updated patch that re-adds an assert that I removed mistakenly.
        Hide
        Michael McCandless added a comment -

        Thanks Lee Hinman, patch looks great. I found one small fix:

           public void add(BooleanClause clause) {
        +    Objects.requireNonNull("BooleanClause must not be null");
             if (clauses.size() >= maxClauseCount) {
               throw new TooManyClauses();
             }
        

        I'll fix & commit ...

        Show
        Michael McCandless added a comment - Thanks Lee Hinman , patch looks great. I found one small fix: public void add(BooleanClause clause) { + Objects.requireNonNull("BooleanClause must not be null"); if (clauses.size() >= maxClauseCount) { throw new TooManyClauses(); } I'll fix & commit ...
        Hide
        ASF subversion and git services added a comment -

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

        LUCENE-6345: add null checking for query parameters

        Show
        ASF subversion and git services added a comment - Commit 1674124 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1674124 ] LUCENE-6345 : add null checking for query parameters
        Hide
        ASF subversion and git services added a comment -

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

        LUCENE-6345: add null checking for query parameters

        Show
        ASF subversion and git services added a comment - Commit 1674134 from Michael McCandless in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1674134 ] LUCENE-6345 : add null checking for query parameters
        Hide
        Michael McCandless added a comment -

        Thanks Lee Hinman!

        Show
        Michael McCandless added a comment - Thanks Lee Hinman !
        Hide
        Anshum Gupta added a comment -

        Bulk close for 5.2.0.

        Show
        Anshum Gupta added a comment - Bulk close for 5.2.0.

          People

          • Assignee:
            Unassigned
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development