Solr
  1. Solr
  2. SOLR-4452

Hunspell stemmer should not merge duplicate dictionary entries

    Details

      Description

      Hunspell dictionaries are on the form

      lucene/ABC
      mahout/X
      

      Each word is listed once with is base form, and the flags after the / define allowed prefixes and suffixes.

      In HunspellDictionary's parsing logic, if the same base word should appear multiple times in the file, the flags from the duplicate entry are added to the flags from the existing entry.

      However, HunSpellStemFilterFactory allows for a comma-separated list of dictionary files to be passed in, the idea being that you can have your own custom extensions and not need to modify the "standard" ones which may change upstream once in a while. This feature now works only for NEW words, not for overriding existing entries from the first dictionary.

      Would like to change this behavior, so that the last line read overwrites any previous one. This will both fix the custom dictionary issue and also fix unintentional wrong original dictionaries, where someone added a word definition at the end without realizing there was another already.

      For the en_UK.dic there are no duplicates. For en_US.dic there is one duplicate, so I argue this behavior is a bug and not a feature dictionary authors depend upon.

      1. SOLR-4452.patch
        4 kB
        Jan Høydahl
      2. SOLR-4452.patch
        0.7 kB
        Jan Høydahl
      3. SOLR-4452-caseinsensitivemerge.patch
        2 kB
        Jan Høydahl
      4. SOLR-4452-caseinsensitive-nomerge.patch
        3 kB
        Jan Høydahl

        Activity

        Hide
        Jan Høydahl added a comment -

        Initial patch

        Show
        Jan Høydahl added a comment - Initial patch
        Hide
        Jan Høydahl added a comment -

        New patch with testcase

        Show
        Jan Høydahl added a comment - New patch with testcase
        Hide
        Jan Høydahl added a comment -

        Think this is ready. Will commit in a day or two unless anyone proves that this is a needed "feature", not a bug.

        Show
        Jan Høydahl added a comment - Think this is ready. Will commit in a day or two unless anyone proves that this is a needed "feature", not a bug.
        Hide
        ASF subversion and git services added a comment -

        Commit 1499164 from janhoy@apache.org
        [ https://svn.apache.org/r1499164 ]

        SOLR-4452: Hunspell stemmer should not merge duplicate dictionary entries

        Show
        ASF subversion and git services added a comment - Commit 1499164 from janhoy@apache.org [ https://svn.apache.org/r1499164 ] SOLR-4452 : Hunspell stemmer should not merge duplicate dictionary entries
        Hide
        ASF subversion and git services added a comment -

        Commit 1499166 from janhoy@apache.org
        [ https://svn.apache.org/r1499166 ]

        SOLR-4452: Hunspell stemmer should not merge duplicate dictionary entries (merge from trunk)

        Show
        ASF subversion and git services added a comment - Commit 1499166 from janhoy@apache.org [ https://svn.apache.org/r1499166 ] SOLR-4452 : Hunspell stemmer should not merge duplicate dictionary entries (merge from trunk)
        Hide
        Robert Muir added a comment -

        This commit is causing test failures.

        Show
        Robert Muir added a comment - This commit is causing test failures.
        Hide
        Jan Høydahl added a comment -

        Yup, the test for case insensitive dictionary assumes that entries are merged. Not sure what is the best action.

        Show
        Jan Høydahl added a comment - Yup, the test for case insensitive dictionary assumes that entries are merged. Not sure what is the best action.
        Hide
        Jan Høydahl added a comment -

        This extra patch takes care of this corner case where two lines are same word but different case.

        For case sensitive dictionary we will treat these as two distinct words. But for case insensitive dictionaries we'll merge them as before.

        Sounds good? Or would it be better to treat these also as duplicates and use only the last one? If so, we should modify the unit test instead.

        Show
        Jan Høydahl added a comment - This extra patch takes care of this corner case where two lines are same word but different case. For case sensitive dictionary we will treat these as two distinct words. But for case insensitive dictionaries we'll merge them as before. Sounds good? Or would it be better to treat these also as duplicates and use only the last one? If so, we should modify the unit test instead.
        Hide
        Jan Høydahl added a comment -

        Changed my mind. This patch treats lower/upper entries the same in caseInsensitive mode, i.e. the last one wins. So we modify the tests instead. Will commit shortly.

        Show
        Jan Høydahl added a comment - Changed my mind. This patch treats lower/upper entries the same in caseInsensitive mode, i.e. the last one wins. So we modify the tests instead. Will commit shortly.
        Hide
        ASF subversion and git services added a comment -

        Commit 1499179 from janhoy@apache.org
        [ https://svn.apache.org/r1499179 ]

        SOLR-4452: Fix test for case insensitive mode

        Show
        ASF subversion and git services added a comment - Commit 1499179 from janhoy@apache.org [ https://svn.apache.org/r1499179 ] SOLR-4452 : Fix test for case insensitive mode
        Hide
        ASF subversion and git services added a comment -

        Commit 1499180 from janhoy@apache.org
        [ https://svn.apache.org/r1499180 ]

        SOLR-4452: Fix test for case insensitive mode

        Show
        ASF subversion and git services added a comment - Commit 1499180 from janhoy@apache.org [ https://svn.apache.org/r1499180 ] SOLR-4452 : Fix test for case insensitive mode
        Hide
        Steve Rowe added a comment -

        Bulk close resolved 4.4 issues

        Show
        Steve Rowe added a comment - Bulk close resolved 4.4 issues

          People

          • Assignee:
            Jan Høydahl
            Reporter:
            Jan Høydahl
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development