Solr
  1. Solr
  2. SOLR-2637

Solrj support for Field Collapsing / Grouping query results parsing

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 3.4, 4.0-ALPHA
    • Component/s: clients - java
    • Labels:

      Description

      Patch ready for Field Collapsing query results parsing.

      1. SOLR-2637.patch
        36 kB
        Martijn van Groningen
      2. SOLR-2637.patch
        35 kB
        Martijn van Groningen
      3. SOLR-2637.patch
        32 kB
        Martijn van Groningen
      4. SOLR-2637.patch
        12 kB
        Tao Cheng
      5. SOLR-2637.patch
        8 kB
        Tao Cheng

        Issue Links

          Activity

          Hide
          Robert Muir added a comment -

          bulk close for 3.4

          Show
          Robert Muir added a comment - bulk close for 3.4
          Hide
          Martijn van Groningen added a comment -

          Committed.
          branch3x: 1152567
          trunk: 1152568

          Show
          Martijn van Groningen added a comment - Committed. branch3x: 1152567 trunk: 1152568
          Hide
          Martijn van Groningen added a comment -

          Updated patch.

          • Updated the jdocs.
          • Made most fields final and removed the setters of these fields.

          I'll commit this in the coming day or so.

          Show
          Martijn van Groningen added a comment - Updated patch. Updated the jdocs. Made most fields final and removed the setters of these fields. I'll commit this in the coming day or so.
          Hide
          Martijn van Groningen added a comment -

          Updated patch.

          • Completed test.
          • Removed _maxScore in GroupCommand. The grouping inside Solr doesn't have this there, so there is no point adding this. The previous version set the maxScore based on the top n groups (inside a command), which in only one case can be correct (if sorting is score desc. As the comment specified in QueryResponse). If users still want this they can get the maxScore from the first score.

          I think this ready to be committed.

          Show
          Martijn van Groningen added a comment - Updated patch. Completed test. Removed _maxScore in GroupCommand. The grouping inside Solr doesn't have this there, so there is no point adding this. The previous version set the maxScore based on the top n groups (inside a command), which in only one case can be correct (if sorting is score desc. As the comment specified in QueryResponse). If users still want this they can get the maxScore from the first score. I think this ready to be committed.
          Hide
          Martijn van Groningen added a comment -

          Added a new patch, that builds further on Tao's work.

          • Grouped and GroupField don't extends from ArrayList any more. That have a list as field.
          • Renamed Grouped to GroupResponse
          • Renamed GroupField to GroupCommand
          • Added an initial test.
          • Fixed bug in XMLResponseParser. When a lst element appeared inside a arr element, the check at line 320 failed. This resulted in an exception.
          • Made the code work with group.query

          This is work in progress. I'll need to extend the test with more assertions. Also I might make GroupCommand abstract and add three concrete subclasses (GroupField, GroupFunction and GroupQuery). Seems like a good idea to me.

          Show
          Martijn van Groningen added a comment - Added a new patch, that builds further on Tao's work. Grouped and GroupField don't extends from ArrayList any more. That have a list as field. Renamed Grouped to GroupResponse Renamed GroupField to GroupCommand Added an initial test. Fixed bug in XMLResponseParser. When a lst element appeared inside a arr element, the check at line 320 failed. This resulted in an exception. Made the code work with group.query This is work in progress. I'll need to extend the test with more assertions. Also I might make GroupCommand abstract and add three concrete subclasses (GroupField, GroupFunction and GroupQuery). Seems like a good idea to me.
          Hide
          Martijn van Groningen added a comment -

          Thanks Tao. I looked at the patch and it looks good.
          They are three things that caught my attention:

          • The classes Grouped and GroupField now extend from ArrayList. I think it is more consistent with other classes (like FacetField, PivotField) if these classes have a List as a field.
          • More meaningful names for Grouped and GroupField classes. Maybe rename Grouped to GroupResponse and maybe rename GroupField to GroupCommand, since it might be grouped by function or query.
          • And as Ryan said some tests would be great.

          For the rest great work!

          Show
          Martijn van Groningen added a comment - Thanks Tao. I looked at the patch and it looks good. They are three things that caught my attention: The classes Grouped and GroupField now extend from ArrayList. I think it is more consistent with other classes (like FacetField, PivotField) if these classes have a List as a field. More meaningful names for Grouped and GroupField classes. Maybe rename Grouped to GroupResponse and maybe rename GroupField to GroupCommand, since it might be grouped by function or query. And as Ryan said some tests would be great. For the rest great work!
          Hide
          Martijn van Groningen added a comment -

          I created SOLR-1681 way back to have solrj support for SOLR-236. The patch in there was made for the old grouping / field collapse solution.
          I think we should continue development here, b/c that patch isn't based on the current solution.

          Show
          Martijn van Groningen added a comment - I created SOLR-1681 way back to have solrj support for SOLR-236 . The patch in there was made for the old grouping / field collapse solution. I think we should continue development here, b/c that patch isn't based on the current solution.
          Hide
          Koji Sekiguchi added a comment -

          Is this a duplicate of SOLR-1681? SOLR-1681 has a patch with tests.

          Show
          Koji Sekiguchi added a comment - Is this a duplicate of SOLR-1681 ? SOLR-1681 has a patch with tests.
          Hide
          Tao Cheng added a comment -

          1. fix 4 space tab to 2
          2. added doc comments
          3. extract "ngroups" when group.ngroups=true.

          Show
          Tao Cheng added a comment - 1. fix 4 space tab to 2 2. added doc comments 3. extract "ngroups" when group.ngroups=true.
          Hide
          Ryan McKinley added a comment -

          Thanks Tao - looks good....

          • can you add some tests?
          • can you change the 4 space indent to 2 spaces?
          Show
          Ryan McKinley added a comment - Thanks Tao - looks good.... can you add some tests? can you change the 4 space indent to 2 spaces?
          Hide
          Tao Cheng added a comment -

          1st draft; doc comments to be provided soon.

          Show
          Tao Cheng added a comment - 1st draft; doc comments to be provided soon.

            People

            • Assignee:
              Martijn van Groningen
              Reporter:
              Tao Cheng
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 24h
                24h
                Remaining:
                Remaining Estimate - 24h
                24h
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development