Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: core/search
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Solr makes use of the MutableValue* series of classes to improve performance of grouping by FunctionQuery (I think). As such they are used in ValueSource implementations. Consequently we need to move these classes in order to move the ValueSources.

      As Yonik pointed out, these classes have use beyond just FunctionQuerys and might be used by both Solr and other modules. However I don't think they belong in Lucene core, since they aren't really related to search functionality. Therefore I think we should put them into a Common module, which can serve as a dependency to Solr and any module.

      1. LUCENE-3232.patch
        50 kB
        Chris Male
      2. LUCENE-3232.patch
        37 kB
        Chris Male
      3. LUCENE-3232.patch
        55 kB
        Chris Male

        Activity

        Hide
        Chris Male added a comment -

        Committed revision 1139467.

        Show
        Chris Male added a comment - Committed revision 1139467.
        Hide
        Chris Male added a comment -

        Patch which includes files I missed in the past. I've already committed but this is for archival.

        Show
        Chris Male added a comment - Patch which includes files I missed in the past. I've already committed but this is for archival.
        Hide
        Chris Male added a comment -

        if we can get FQs factored out soonish

        This is the last issue preventing me from doing just that

        I guess we keep the name "common" for now.

        Awesome. I find it fairly common (ha) in projects to have a common module. If it doesn't pan out, then we can either rename it or slurp it into another module.

        Show
        Chris Male added a comment - if we can get FQs factored out soonish This is the last issue preventing me from doing just that I guess we keep the name "common" for now. Awesome. I find it fairly common (ha) in projects to have a common module. If it doesn't pan out, then we can either rename it or slurp it into another module.
        Hide
        Michael McCandless added a comment -

        OK this sounds like a good plan... if we can get FQs factored out soonish then we can simply fix grouping module to use that (ie, we don't need common module to hold the ValueSource, etc.).

        I guess we keep the name "common" for now. Maybe as we slurp in more stuff from Solr I'll like the name better

        Show
        Michael McCandless added a comment - OK this sounds like a good plan... if we can get FQs factored out soonish then we can simply fix grouping module to use that (ie, we don't need common module to hold the ValueSource, etc.). I guess we keep the name "common" for now. Maybe as we slurp in more stuff from Solr I'll like the name better
        Hide
        Chris Male added a comment - - edited

        I wonder if we should name this module something more specific, eg docvalues? values?

        Should we also move over ValueSource, *DocValues, FieldCacheSource? I think, then, Solr 3.x grouping could cutover and then group by other field types.

        To be honest, that wasn't my plan

        My plan is to first move these to a Common module which will serve basically as a utility module for other modules. The MutableValue classes are useful in a number of places (or will be in the future). I envisage other useful utility like classes going into this module in the future too. Solr for example has a number of very useful utilities that might be of benefit.

        As such, it doesn't really relate to FunctionQuerys or ValueSources.

        The next step once this is complete is to do what I originally intended and make a Queries module and push FunctionQuery and all the ValueSources / DocValues into that.

        In the end you get the following structure:

        modules/
            common/
              (MutableValue*)
            queries/
              (FunctionQuery, *DocValues, *ValueSource, Queries from contrib/queries)
        

        Seem reasonable?

        Show
        Chris Male added a comment - - edited I wonder if we should name this module something more specific, eg docvalues? values? Should we also move over ValueSource, *DocValues, FieldCacheSource? I think, then, Solr 3.x grouping could cutover and then group by other field types. To be honest, that wasn't my plan My plan is to first move these to a Common module which will serve basically as a utility module for other modules. The MutableValue classes are useful in a number of places (or will be in the future). I envisage other useful utility like classes going into this module in the future too. Solr for example has a number of very useful utilities that might be of benefit. As such, it doesn't really relate to FunctionQuerys or ValueSources. The next step once this is complete is to do what I originally intended and make a Queries module and push FunctionQuery and all the ValueSources / DocValues into that. In the end you get the following structure: modules/ common/ (MutableValue*) queries/ (FunctionQuery, *DocValues, *ValueSource, Queries from contrib/queries) Seem reasonable?
        Hide
        Simon Willnauer added a comment -

        I wonder if we should name this module something more specific, eg docvalues? values?

        dude! no!

        Show
        Simon Willnauer added a comment - I wonder if we should name this module something more specific, eg docvalues? values? dude! no!
        Hide
        Michael McCandless added a comment -

        Patch looks great!

        I wonder if we should name this module something more specific, eg docvalues? values?

        Should we also move over ValueSource, *DocValues, FieldCacheSource? I think, then, Solr 3.x grouping could cutover and then group by other field types.

        Show
        Michael McCandless added a comment - Patch looks great! I wonder if we should name this module something more specific, eg docvalues? values? Should we also move over ValueSource, *DocValues, FieldCacheSource? I think, then, Solr 3.x grouping could cutover and then group by other field types.
        Hide
        Chris Male added a comment -

        New patch which moves the MutableValue code to a Common module.

        Also makes a change to the configuration for the analysis common module in dev-tools, since calling this 'common' clashed.

        Compiles and test pass.

        Show
        Chris Male added a comment - New patch which moves the MutableValue code to a Common module. Also makes a change to the configuration for the analysis common module in dev-tools, since calling this 'common' clashed. Compiles and test pass.
        Hide
        Chris Male added a comment -

        New code to execute with patch:

        svn mkdir --parents modules/common/src/java/org/apache/lucene/common/mutable
        svn move solr/src/java/org/apache/solr/search/MutableValue.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValue.java
        svn move solr/src/java/org/apache/solr/search/MutableValueFloat.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueFloat.java
        svn move solr/src/java/org/apache/solr/search/MutableValueBool.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueBool.java
        svn move solr/src/java/org/apache/solr/search/MutableValueDate.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueDate.java
        svn move solr/src/java/org/apache/solr/search/MutableValueDouble.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueDouble.java
        svn move solr/src/java/org/apache/solr/search/MutableValueInt.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueInt.java
        svn move solr/src/java/org/apache/solr/search/MutableValueLong.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueLong.java
        svn move solr/src/java/org/apache/solr/search/MutableValueStr.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueStr.java
        svn move dev-tools/idea/modules/analysis/common/common.iml dev-tools/idea/modules/analysis/common/analysis-common.iml
        

        This code includes a change to the common.iml file used by IntelliJ for the Analysis Common module, since this name clashed.

        Show
        Chris Male added a comment - New code to execute with patch: svn mkdir --parents modules/common/src/java/org/apache/lucene/common/mutable svn move solr/src/java/org/apache/solr/search/MutableValue.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValue.java svn move solr/src/java/org/apache/solr/search/MutableValueFloat.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueFloat.java svn move solr/src/java/org/apache/solr/search/MutableValueBool.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueBool.java svn move solr/src/java/org/apache/solr/search/MutableValueDate.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueDate.java svn move solr/src/java/org/apache/solr/search/MutableValueDouble.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueDouble.java svn move solr/src/java/org/apache/solr/search/MutableValueInt.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueInt.java svn move solr/src/java/org/apache/solr/search/MutableValueLong.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueLong.java svn move solr/src/java/org/apache/solr/search/MutableValueStr.java modules/common/src/java/org/apache/lucene/common/mutable/MutableValueStr.java svn move dev-tools/idea/modules/analysis/common/common.iml dev-tools/idea/modules/analysis/common/analysis-common.iml This code includes a change to the common.iml file used by IntelliJ for the Analysis Common module, since this name clashed.
        Hide
        Chris Male added a comment -

        Actually scrap that question, I'll put them somewhere else immediately.

        Show
        Chris Male added a comment - Actually scrap that question, I'll put them somewhere else immediately.
        Hide
        Chris Male added a comment -

        I've debated this backwards and forwards. Do they have a use case out of function queries at the moment? If so then yeah I'll happily put them somewhere else. Otherwise I'll cross that bridge at the time.

        Show
        Chris Male added a comment - I've debated this backwards and forwards. Do they have a use case out of function queries at the moment? If so then yeah I'll happily put them somewhere else. Otherwise I'll cross that bridge at the time.
        Hide
        Yonik Seeley added a comment -

        These are useful beyond function queries... perhaps they should not be in the "function" module?

        Show
        Yonik Seeley added a comment - These are useful beyond function queries... perhaps they should not be in the "function" module?
        Hide
        Chris Male added a comment -

        Patch that establishes the Queries module and moves the MutableValue classes. Includes intellij, eclipse and maven work.

        Everything compiles and tests pass.

        It'd be great if someone could review. I'll commit in a few days.

        Show
        Chris Male added a comment - Patch that establishes the Queries module and moves the MutableValue classes. Includes intellij, eclipse and maven work. Everything compiles and tests pass. It'd be great if someone could review. I'll commit in a few days.
        Hide
        Chris Male added a comment -

        Code to execute before patch:

        svn mkdir --parents modules/queries/src/java/org/apache/lucene/queries/function
        svn move solr/src/java/org/apache/solr/search/MutableValue.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValue.java
        svn move solr/src/java/org/apache/solr/search/MutableValueFloat.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueFloat.java
        svn move solr/src/java/org/apache/solr/search/MutableValueBool.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueBool.java
        svn move solr/src/java/org/apache/solr/search/MutableValueDate.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueDate.java
        svn move solr/src/java/org/apache/solr/search/MutableValueDouble.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueDouble.java
        svn move solr/src/java/org/apache/solr/search/MutableValueInt.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueInt.java
        svn move solr/src/java/org/apache/solr/search/MutableValueLong.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueLong.java
        svn move solr/src/java/org/apache/solr/search/MutableValueStr.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueStr.java
        
        Show
        Chris Male added a comment - Code to execute before patch: svn mkdir --parents modules/queries/src/java/org/apache/lucene/queries/function svn move solr/src/java/org/apache/solr/search/MutableValue.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValue.java svn move solr/src/java/org/apache/solr/search/MutableValueFloat.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueFloat.java svn move solr/src/java/org/apache/solr/search/MutableValueBool.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueBool.java svn move solr/src/java/org/apache/solr/search/MutableValueDate.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueDate.java svn move solr/src/java/org/apache/solr/search/MutableValueDouble.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueDouble.java svn move solr/src/java/org/apache/solr/search/MutableValueInt.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueInt.java svn move solr/src/java/org/apache/solr/search/MutableValueLong.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueLong.java svn move solr/src/java/org/apache/solr/search/MutableValueStr.java modules/queries/src/java/org/apache/lucene/queries/function/MutableValueStr.java

          People

          • Assignee:
            Chris Male
            Reporter:
            Chris Male
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development