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_r275043339
##########
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:
> bloomberg#231 sketches the idea, what do you think?
A pull request for a pull request, or a diff for a diff, at first glance
that spooked me a little :-)
And at second glance I skipped 'the middle bit' and looked end-to-end
https://github.com/apache/lucene-solr/compare/1071d093360b2c5869a918de743c7089952094f4...5a326a19eb7aa92754d6ccf7b321d3c04d3b9f50
with focus on the `SearchGroupsResultTransformer` class.
There seemed to be a relatively noticable amount of code similarity or
duplication between `DefaultSearchResultResultTransformer` and
`SkipSecondStepSearchResultResultTransformer` and so i took a step back and
considered what the _difference_ (conceptually) between the two transformers
is, with two aha! moments:
1. the `getConvertedSortValues` method used by
`SkipSecondStepSearchResultResultTransformer` is pretty similar to the
`ShardResultTransformerUtils.marshalSortValue` from
https://issues.apache.org/jira/browse/SOLR-9890 in Solr 6.5
2. the `SkipSecondStepSearchResultResultTransformer.serializeSearchGroup`
method appears to construct exactly what
`DefaultSearchResultResultTransformer.serializeSearchGroup` constructs and then
it 'wraps' it together with an id and a score in a 'group info' object
The
https://github.com/cpoerschke/lucene-solr/commit/10fbfd1dcf16065688c3610b26a55f2aa9c99f8a
commit sketches how a `SearchGroupsResultTransformer.serializeOneSearchGroup`
method could be factored out. What do you think?
(If factoring out such a method makes sense then I could (next week) attempt
a similar approach for the `transformToNative` method. And if that works out
good we could revisit how the
`[Default|SkipSecondStep]SearchResultResultTransformer` class trio is arranged
in terms of hierarchy? And if it does not work out good then that would be
insightful too in terms of why etc.)
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
