Solr
  1. Solr
  2. SOLR-648

SpellcheckComponent throws NullPointerException on restart without build

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.3
    • Component/s: None
    • Labels:
      None

      Description

      from http://mail-archives.apache.org/mod_mbox/lucene-solr-user/200807.mbox/%3cC4AC838E.ADE7%25jonathan_lee@comcast.com%3e

      "I believe there is a bug in IndexBased- and FileBasedSpellChecker.java
      where the analyzer variable is only set on the build command. Therefore,
      when the index is reloaded, but not built after starting solr, issuing a
      query with the spellcheck.q parameter will cause a NullPointerException to
      be thrown (SpellCheckComponent.java:158). Moving the analyzer logic to the
      constructor seems to fix the problem."

      1. SOLR-648.patch
        16 kB
        Shalin Shekhar Mangar
      2. SOLR-648.patch
        13 kB
        Shalin Shekhar Mangar
      3. SOLR-648.patch
        11 kB
        Jonathan Lee

        Issue Links

          Activity

          Hide
          Jonathan Lee added a comment -

          Here is the patch that was originally sent by email, which probably needs to be changed: http://mail-archives.apache.org/mod_mbox/lucene-solr-user/200807.mbox/%3cC4ACB4E4.AE1F%25jonathan_lee@comcast.com%3e

          Show
          Jonathan Lee added a comment - Here is the patch that was originally sent by email, which probably needs to be changed: http://mail-archives.apache.org/mod_mbox/lucene-solr-user/200807.mbox/%3cC4ACB4E4.AE1F%25jonathan_lee@comcast.com%3e
          Hide
          Shalin Shekhar Mangar added a comment -

          This fixes the NullPointerException. The changes are:

          • The signature of the init method is changed to init(NamedList, SolrCore)
          • All analyzer defining code is moved to AbstractLuceneSpellChecker#init from the build methods in IndexBasedSpellChecker and FileBasedSpellChecker.

          I'll add a test and then give another patch shortly.

          Show
          Shalin Shekhar Mangar added a comment - This fixes the NullPointerException. The changes are: The signature of the init method is changed to init(NamedList, SolrCore) All analyzer defining code is moved to AbstractLuceneSpellChecker#init from the build methods in IndexBasedSpellChecker and FileBasedSpellChecker. I'll add a test and then give another patch shortly.
          Hide
          Shalin Shekhar Mangar added a comment -

          Added testcase as SpellCheckComponentTest#testReloadOnStart which reproduces the bug reported in this issue.

          Grant, would you like to review this patch? It contains minor API change so you'll need to revisit the custom SolrSpellChecker implementation you had. I'd like to commit this asap so that I can continue with SOLR-622.

          An alternate way would have been to change the signature of the SolrSpellChecker.reload method to take in atleast the SolrCore or the same params as the build method.

          Show
          Shalin Shekhar Mangar added a comment - Added testcase as SpellCheckComponentTest#testReloadOnStart which reproduces the bug reported in this issue. Grant, would you like to review this patch? It contains minor API change so you'll need to revisit the custom SolrSpellChecker implementation you had. I'd like to commit this asap so that I can continue with SOLR-622 . An alternate way would have been to change the signature of the SolrSpellChecker.reload method to take in atleast the SolrCore or the same params as the build method.
          Hide
          Grant Ingersoll added a comment -

          I perused the patch and it looks reasonable since we are just getting the ResourceLoader from the Core there anyway. It does look like we should add a test case for this issue.

          Show
          Grant Ingersoll added a comment - I perused the patch and it looks reasonable since we are just getting the ResourceLoader from the Core there anyway. It does look like we should add a test case for this issue.
          Hide
          Shalin Shekhar Mangar added a comment -

          Yes, I've added a testcase which fails otherwise.

          Show
          Shalin Shekhar Mangar added a comment - Yes, I've added a testcase which fails otherwise.
          Hide
          Shalin Shekhar Mangar added a comment - - edited

          Committed revision 679816.

          Thanks Jonathan and Geoffrey!

          Show
          Shalin Shekhar Mangar added a comment - - edited Committed revision 679816. Thanks Jonathan and Geoffrey!

            People

            • Assignee:
              Shalin Shekhar Mangar
              Reporter:
              Geoffrey Young
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development