Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-1432

FunctionQueries aren't correctly weighted

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.4
    • Fix Version/s: 1.4.1, 1.5, 3.1, 4.0-ALPHA
    • Component/s: None
    • Labels:
      None

      Description

      Nested queries in function queries aren't weighted correctly with the proper Searcher, and this is now even more serious with per-segment searching in Lucene/Solr.

      1. SOLR-1432.patch
        51 kB
        Yonik Seeley
      2. SOLR-1432.patch
        49 kB
        Yonik Seeley

        Activity

        Hide
        yseeley@gmail.com Yonik Seeley added a comment -

        Here's the essence of the patch that adds a simple/flexible untyped weighting scheme to ValueSource:

          /** Implementations should propagate createWeight to sub-ValueSources which can optionally store
           * weight info in the context. The context object will be passed to getValues()
           * where this info can be retrieved. */
          public void createWeight(Map context, Searcher searcher) throws IOException {
          }
        
        Show
        yseeley@gmail.com Yonik Seeley added a comment - Here's the essence of the patch that adds a simple/flexible untyped weighting scheme to ValueSource: /** Implementations should propagate createWeight to sub-ValueSources which can optionally store * weight info in the context. The context object will be passed to getValues() * where this info can be retrieved. */ public void createWeight(Map context, Searcher searcher) throws IOException { }
        Hide
        yseeley@gmail.com Yonik Seeley added a comment -

        Updated patch with tests that fail w/o correct weighting behavior.

        Show
        yseeley@gmail.com Yonik Seeley added a comment - Updated patch with tests that fail w/o correct weighting behavior.
        Hide
        gsingers Grant Ingersoll added a comment -

        Bulk close for Solr 1.4

        Show
        gsingers Grant Ingersoll added a comment - Bulk close for Solr 1.4
        Hide
        lucasjosh josh lucas added a comment -

        I wish this issue would have been called out in the CHANGES file for the 1.4.0 release. It bit me since our ValueSource classes were not overriding this new method.

        Show
        lucasjosh josh lucas added a comment - I wish this issue would have been called out in the CHANGES file for the 1.4.0 release. It bit me since our ValueSource classes were not overriding this new method.
        Hide
        hossman Hoss Man added a comment -

        I wish this issue would have been called out in the CHANGES file for the 1.4.0 release

        You're right ... it was a pretty big oversight on our part that it wasn't mentioned anywhere (let alone specificly called out in the "Upgrading" section.

        retroactively editing CHANGES.txt isn't really feasible, but i've added it to the Solr1.4 wiki page to try and increase the visibility a bit...

        http://wiki.apache.org/solr/Solr1.4

        Show
        hossman Hoss Man added a comment - I wish this issue would have been called out in the CHANGES file for the 1.4.0 release You're right ... it was a pretty big oversight on our part that it wasn't mentioned anywhere (let alone specificly called out in the "Upgrading" section. retroactively editing CHANGES.txt isn't really feasible, but i've added it to the Solr1.4 wiki page to try and increase the visibility a bit... http://wiki.apache.org/solr/Solr1.4
        Hide
        pjaol patrick o'leary added a comment -

        Shouldn't ValueSource simply be updated to

        public DocValues getValues(Map context, IndexReader reader) throws IOException {
            return getValues(reader);
        }
        

        Rather than null, not ideal but at least it doesn't leave folks in a worse position than using 1.3-

        Show
        pjaol patrick o'leary added a comment - Shouldn't ValueSource simply be updated to public DocValues getValues(Map context, IndexReader reader) throws IOException { return getValues(reader); } Rather than null, not ideal but at least it doesn't leave folks in a worse position than using 1.3-
        Hide
        yseeley@gmail.com Yonik Seeley added a comment -

        Yep, that's what it should have been. I'll change.

        Show
        yseeley@gmail.com Yonik Seeley added a comment - Yep, that's what it should have been. I'll change.
        Hide
        hossman Hoss Man added a comment -

        Correcting Fix Version based on CHANGES.txt, see this thread for more details...

        http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E

        (Note: this issue is "special" ... it was originally marked fixed in 1.4 because some changes were made for 1.4 – but those changes were broken so i'm removing 1.4 from the Fix list)

        Show
        hossman Hoss Man added a comment - Correcting Fix Version based on CHANGES.txt, see this thread for more details... http://mail-archives.apache.org/mod_mbox/lucene-dev/201005.mbox/%3Calpine.DEB.1.10.1005251052040.24672@radix.cryptio.net%3E (Note: this issue is "special" ... it was originally marked fixed in 1.4 because some changes were made for 1.4 – but those changes were broken so i'm removing 1.4 from the Fix list)
        Hide
        hossman Hoss Man added a comment -

        Committed revision 949467.

        backported to 1.4 branch for 1.4.1

        Show
        hossman Hoss Man added a comment - Committed revision 949467. backported to 1.4 branch for 1.4.1

          People

          • Assignee:
            yseeley@gmail.com Yonik Seeley
            Reporter:
            yseeley@gmail.com Yonik Seeley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development