Lucene - Core
  1. Lucene - Core
  2. LUCENE-6609

FieldCacheSource (or it's subclasses) should override getSortField

    Details

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

      Description

      ValueSource defines the following method...

        public SortField getSortField(boolean reverse) {
          return new ValueSourceSortField(reverse);
        }
      

      ...where ValueSourceSortField builds up a ValueSourceComparator containing a double[] based on the FunctionValues of the original ValueSource.

      meanwhile, the abstract FieldCacheSource exists as a base implementation for classes like IntFieldSource and DoubleFieldSource which wrap a ValueSource around DocValues for the specified field.

      But neither FieldCacheSource nor any of it's subclasses override the getSortField(boolean) method – so attempting to sort on something like an IntFieldSource winds up using a bunch of ram to build that double[] to give users a less accurate sort (because of casting) then if they just sorted directly on the field.

      is there any good reason why FieldCacheSource subclases like IntFieldSource shouldn't all override getSortField with something like...

        public SortField getSortField(boolean reverse) {
          return new SortField(field, Type.INT, reverse);
        }
      

      ?

        Activity

        Hide
        Hoss Man added a comment -

        I think the same thing would make sense for SortedSetFieldSource.getSortField() – it can return an instance of SortedSetSortField.

        Show
        Hoss Man added a comment - I think the same thing would make sense for SortedSetFieldSource.getSortField() – it can return an instance of SortedSetSortField .
        Hide
        Hoss Man added a comment -

        here's a patch with the changes and some basic tests (both whitebox at the lucene level and more pragmatic at the solr level)

        Only one existing test needed changed in this patch: TestFunctionQuerySort.testSearchAfterWhenSortingByFunctionValues was assumed the sort values in the FieldDoc would be Doubles when using an IntFieldSource as the basis of the Sort – to keep the spirit of the original test, I wrapped that IntFieldSource in a SumFloatFunction.

        Show
        Hoss Man added a comment - here's a patch with the changes and some basic tests (both whitebox at the lucene level and more pragmatic at the solr level) Only one existing test needed changed in this patch: TestFunctionQuerySort.testSearchAfterWhenSortingByFunctionValues was assumed the sort values in the FieldDoc would be Doubles when using an IntFieldSource as the basis of the Sort – to keep the spirit of the original test, I wrapped that IntFieldSource in a SumFloatFunction.
        Hide
        Adrien Grand added a comment -

        +1

        Show
        Adrien Grand added a comment - +1
        Hide
        ASF subversion and git services added a comment -

        Commit 1693979 from hossman@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1693979 ]

        LUCENE-6609: Add getSortField impls to many subclasses of FieldCacheSource which return the most direct SortField implementation

        Show
        ASF subversion and git services added a comment - Commit 1693979 from hossman@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1693979 ] LUCENE-6609 : Add getSortField impls to many subclasses of FieldCacheSource which return the most direct SortField implementation
        Hide
        Hoss Man added a comment -

        thanks for the review adrian.

        (running full tests and precommit on 5x now)

        Show
        Hoss Man added a comment - thanks for the review adrian. (running full tests and precommit on 5x now)
        Hide
        ASF subversion and git services added a comment -

        Commit 1693987 from hossman@apache.org in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1693987 ]

        LUCENE-6609: Add getSortField impls to many subclasses of FieldCacheSource which return the most direct SortField implementation (merge r1693979)

        Show
        ASF subversion and git services added a comment - Commit 1693987 from hossman@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1693987 ] LUCENE-6609 : Add getSortField impls to many subclasses of FieldCacheSource which return the most direct SortField implementation (merge r1693979)
        Hide
        Hoss Man added a comment -

        causing some randomized test failures due to codecs that don't support max.

        reopening until fixed.

        Show
        Hoss Man added a comment - causing some randomized test failures due to codecs that don't support max. reopening until fixed.
        Hide
        ASF subversion and git services added a comment -

        Commit 1694090 from hossman@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1694090 ]

        LUCENE-6609: refactor Solr tests to isolate SortedSetSelector dependent code into a test that SuppressCodecs the problematic docvalues formats

        Show
        ASF subversion and git services added a comment - Commit 1694090 from hossman@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1694090 ] LUCENE-6609 : refactor Solr tests to isolate SortedSetSelector dependent code into a test that SuppressCodecs the problematic docvalues formats
        Hide
        ASF subversion and git services added a comment -

        Commit 1694092 from hossman@apache.org in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1694092 ]

        LUCENE-6609: refactor Solr tests to isolate SortedSetSelector dependent code into a test that SuppressCodecs the problematic docvalues formats (merge r1694090)

        Show
        ASF subversion and git services added a comment - Commit 1694092 from hossman@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1694092 ] LUCENE-6609 : refactor Solr tests to isolate SortedSetSelector dependent code into a test that SuppressCodecs the problematic docvalues formats (merge r1694090)
        Hide
        Shalin Shekhar Mangar added a comment -

        Bulk close for 5.3.0 release

        Show
        Shalin Shekhar Mangar added a comment - Bulk close for 5.3.0 release

          People

          • Assignee:
            Hoss Man
            Reporter:
            Hoss Man
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development