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

Grouping.java: sort variable names confusion

    Details

    • Type: Wish
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.4, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      The undistributed case i.e. solr/Grouping.java's variable names confusingly differ from the names used by lucene (and by the distributed case).

      Specifically the name groupSort in lucene (and in the distributed case) means between-groups-sort but in the Grouping.java it means within-group-sort.

      lucene:

      TermFirstPassGroupingCollector(... Sort groupSort ...)
      TermSecondPassGroupingCollector(... Sort groupSort, Sort withinGroupSort ...)
      

      solr:

      SearchGroupsFieldCommand.java: firstPassGroupingCollector = new TermFirstPassGroupingCollector(field.getName(), groupSort, topNGroups);
      TopGroupsFieldCommand.java: secondPassCollector = new TermSecondPassGroupingCollector(... groupSort, sortWithinGroup ...);
      
      Grouping.java:    public Sort groupSort;   // the sort of the documents *within* a single group.
      Grouping.java:    public Sort sort;        // the sort between groups
      Grouping.java:  firstPass = new TermFirstPassGroupingCollector(groupBy, sort, actualGroupsToFind);
      Grouping.java: secondPass = new TermSecondPassGroupingCollector(... sort, groupSort ...);
      

      This JIRA proposes to rename the Grouping.java variables to remove the confusion:

      • part 1: in Grouping.java rename groupSort to withinGroupSort
      • part 2: in Grouping.java rename sort to groupSort

        Attachments

        1. SOLR-8114-part2of2.patch
          5 kB
          Christine Poerschke
        2. SOLR-8114-part1of2.patch
          7 kB
          Christine Poerschke

          Activity

            People

            • Assignee:
              cpoerschke Christine Poerschke
              Reporter:
              cpoerschke Christine Poerschke
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: