cpoerschke commented on a change in pull request #300: SOLR-11831: Skip second
grouping step if group.limit is 1 (aka Las Vegas Patch)
URL: https://github.com/apache/lucene-solr/pull/300#discussion_r269179855
##########
File path:
solr/core/src/java/org/apache/solr/search/grouping/distributed/shardresultserializer/SearchGroupsResultTransformer.java
##########
@@ -34,17 +41,37 @@
/**
* Implementation for transforming {@link SearchGroup} into a {@link
NamedList} structure and visa versa.
*/
-public class SearchGroupsResultTransformer implements
ShardResultTransformer<List<Command>, Map<String,
SearchGroupsFieldCommandResult>> {
+public abstract class SearchGroupsResultTransformer implements
ShardResultTransformer<List<Command>, Map<String,
SearchGroupsFieldCommandResult>> {
Review comment:
An existing public class becoming `abstract` here 'jumped out' at me a
little since it could be considered to be breaking backwards compatibility?
The
https://github.com/cpoerschke/lucene-solr/commit/7e47e01ba27d3cd39daa288e60c61316b1738d67
commit explores removal/replacement of the
`SearchGroupsResultTransformer.getInstance` wrapper and a next step could be
for the proposed
`SearchGroupsResultTransformer.SkipSecondStepSearchResultResultTransformer`
inner class to become a `SkipSecondStepSearchResultResultTransformer.java`
class of its own and the proposed
`SearchGroupsResultTransformer.DefaultSearchResultResultTransformer` inner
class to _not_ be created i.e. `SearchGroupsResultTransformer` here to not
become abstract and its constructor to remain public. This of course assumes
that `SearchGroupsResultTransformer` would require only simple changes to make
it extensible and that then `SkipSecondStepSearchResultResultTransformer` could
extend it.
(Note that the
https://github.com/cpoerschke/lucene-solr/commit/7e47e01ba27d3cd39daa288e60c61316b1738d67
commit is on
https://github.com/cpoerschke/lucene-solr/tree/github-bloomberg-SOLR-11831-cpoerschke-5
branch and that branch includes
https://github.com/cpoerschke/lucene-solr/tree/github-bloomberg-SOLR-11831-cpoerschke-4
changes for convenience only i.e. not out of necessity.)
What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
