Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on PR #4277: URL: https://github.com/apache/solr/pull/4277#issuecomment-4319695437 @dsmiley Appreciate the backport to 10x. When you have a chance, could you also help backport this to 9x for the 9.11 release, along with https://github.com/apache/solr/pull/4276 ? -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
dsmiley merged PR #4277: URL: https://github.com/apache/solr/pull/4277 -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3095524525
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -296,6 +317,10 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest
sreq) {
long approximateTotalHits = 0;
Map> shardDocMap = new HashMap<>();
String[] queriesToCombineKeys =
rb.req.getParams().getParams(CombinerParams.COMBINER_QUERY);
+// Build per-shard set of doc IDs retained after collapse in simpleCombine.
Review Comment:
The doc to be returned from shard is docListAndSet, SolrDocument is not
available at that level where we can add internal fields. Individual shard only
[setResult](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java#L243)
to the ResponseBuilder which contains docListAndSet that is inherently lossy
in terms of metadata. IMO, fwiw that opens up a separate discussion like I
mentioned earlier.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
dsmiley commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3094676609
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -296,6 +317,10 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest
sreq) {
long approximateTotalHits = 0;
Map> shardDocMap = new HashMap<>();
String[] queriesToCombineKeys =
rb.req.getParams().getParams(CombinerParams.COMBINER_QUERY);
+// Build per-shard set of doc IDs retained after collapse in simpleCombine.
Review Comment:
I *definitely do not* suggest extending DocSlice/DocList. I mean adding
extra internal fields on the returned docs. e.g. a underscore prefix & suffix
field.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3094419351
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -296,6 +317,10 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest
sreq) {
long approximateTotalHits = 0;
Map> shardDocMap = new HashMap<>();
String[] queriesToCombineKeys =
rb.req.getParams().getParams(CombinerParams.COMBINER_QUERY);
+// Build per-shard set of doc IDs retained after collapse in simpleCombine.
Review Comment:
Added comment for future references. I thought about this
single-response-with-annotations approach, which looks cleaner and simpler at
first glance but is non-trivial and complex touching the core components. What
I found was:
[that](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java#L239-L243)
would require extending DocSlice/DocList (which has no per-doc metadata) and
the SOLRDOCLIST wire format, which would together a meaningful refactor across
per shard response, the response builder, and the transformer layer. I think
it's worth digging deeper as a follow up.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
dsmiley commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3093610784
##
changelog/unreleased/SOLR-18195-collapse-results-combined-query.yml:
##
@@ -0,0 +1,9 @@
+# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc
+title: Support for Collapse Results in Combined Query Component
Review Comment:
If we're searching for this, we'll likely search for
"CombinedQueryComponent" as that's its name -- a class. Maybe also add (RRF)
because that's the magical incantation to jog the memory of anyone reading this
```suggestion
title: Support for using {!collapse} with CombinedQueryComponent (RRF)
```
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -296,6 +317,10 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest
sreq) {
long approximateTotalHits = 0;
Map> shardDocMap = new HashMap<>();
String[] queriesToCombineKeys =
rb.req.getParams().getParams(CombinerParams.COMBINER_QUERY);
+// Build per-shard set of doc IDs retained after collapse in simpleCombine.
Review Comment:
That helps a lot!
It may be strange to suggest this but I think simply _removing_ the comment
words "retained after collapse in simpleCombine" would be slightly clearer, as
that references stuff happening at a shard level which confused me a little as
we're processing at the coordinator here.
What I find confusing from your explanation is that a shard returns *both*
`response_per_query` and the standard `response` key. I will suggest a comment
to put somewhere (not precisely at the line here). Not sure if it's actually
correct or useful but tell me:
"This component receives *both* "response" and "response_per_query". Only
the former is deduplicated, and thus most (all?) processing of the latter must
exclude docs not present in the former."
I wonder if we'd be better off with only "response" and then an additional
key to provide info on which queries matched.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3091417611
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -296,6 +317,10 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest
sreq) {
long approximateTotalHits = 0;
Map> shardDocMap = new HashMap<>();
String[] queriesToCombineKeys =
rb.req.getParams().getParams(CombinerParams.COMBINER_QUERY);
+// Build per-shard set of doc IDs retained after collapse in simpleCombine.
Review Comment:
Yes, Sure. Here is the overall flow:
Phase 1: Shard-level : Each shard runs the combined query request. In
process():
1. Per-query execution: Each sub-query executes and applies the
fq={!collapse} on its own, so each produces its own set of group heads. The
individual results are added to crb.rsp under "response_per_query" — this is
what the coordinator reads per-query later.
2. [Cross-query dedup via
simpleCombine](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java#L57):
All per-query results are merged and collapse is re-applied across the union.
The result is set as crb.rsp's main "response". This is the shard's
canonical post-collapse result — it contains only the docs that survived
collapse across all sub-queries.
So the shard response sent to the coordinator contains both:
- "response_per_query" — per-query doc lists (each collapsed individually,
may have cross-query duplicates)
- "response" — the simpleCombine output (deduplicated across queries)
Phase 2: [Coordinator-level
(mergeIds())](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java#L300)
The coordinator iterates per-query responses ("response_per_query") from
each shard to build shardDocMap (query -> list of ShardDocs), which is then
passed to the RRF combiner.
The problem is: per-query responses can contain docs that were eliminated by
simpleCombine on the shard. If we feed them into RRF, those eliminated docs get
reintroduced in the final result.
Example: On shard-1, `q1` picked `id=1` as group head for `mod3_idv=1`, and
`q2` picked `id=4`. simpleCombine re-collapsed and kept only one (say id=4).
But both `id=1` (from q1's per-query response) and `id=4` (from q2's) are still
in "response_per_query". Without filtering, RRF would see both.
[The fix
](https://github.com/apache/solr/pull/4277/changes#diff-1ca21c13d967412ea6227fe257616a9cb2ac45012b207373a526d3767a46b588R401-R654):
combinedDocIdsPerShard extracts the doc IDs from each shard's "response" (the
simpleCombine output). Then, while iterating per-query docs, any doc not in
that set is skipped. This ensures RRF/Combiner only operates on docs that
survived the shard-level cross-query collapse.
Tested in
[testCollapseWithCombinedQueryProducesDuplicates](https://github.com/apache/solr/pull/4277/changes#diff-d18c0ebaeee21519f08c727c3202b4bc8d47a592e886102b07dc99af0a0b48b4R321)
- sets up exactly this scenario with two queries scoring the same collapse
group differently, and asserts no duplicates in the final result.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
dsmiley commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3090778383
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -296,6 +317,10 @@ protected void mergeIds(ResponseBuilder rb, ShardRequest
sreq) {
long approximateTotalHits = 0;
Map> shardDocMap = new HashMap<>();
String[] queriesToCombineKeys =
rb.req.getParams().getParams(CombinerParams.COMBINER_QUERY);
+// Build per-shard set of doc IDs retained after collapse in simpleCombine.
+// Used to filter per-query docs so that RRF doesn't reintroduce docs
+// excluded by collapse at the shard level.
+Map> combinedDocIds = new HashMap<>();
Review Comment:
again; the map should clarify what the key is.
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -290,4 +290,114 @@ public void testForcedDistrib() throws Exception {
QueryResponse rsp = query("qt", "/forcedDistribTest", "q", "*:*", "rows",
"0");
// ForcedDistribSearchHandler would trigger a failure if this didn't work
}
+
+ /**
+ * Reproduces duplicate documents in combined query results when using
collapse. Two different
+ * sub-queries may select different group heads for the same collapse field
value, and
+ * simpleCombine() merges by Lucene doc ID — not by collapse field value —
so both survive.
+ *
+ * Data setup (mod3_idv is the collapse field, values cycle
1,2,0,1,2,0,...):
+ *
+ *
+ * id=1 mod3_idv=1 text="alpha bravo" title="alpha bravo"
+ * id=2 mod3_idv=2 text="alpha charlie"title="alpha charlie"
+ * id=3 mod3_idv=0 text="bravo delta" title="bravo delta"
+ * id=4 mod3_idv=1 text="charlie delta"title="charlie delta"
+ * id=5 mod3_idv=2 text="alpha delta" title="alpha delta"
+ * id=6 mod3_idv=0 text="bravo charlie"title="bravo charlie"
+ *
+ *
+ * Query 1: "alpha bravo" → matches docs 1,2,3,5,6 with varying scores.
After collapse on
+ * mod3_idv, picks one group head per value (0,1,2).
+ *
+ * Query 2: "charlie delta" → matches docs 2,3,4,5,6 with varying scores.
After collapse on
+ * mod3_idv, picks one group head per value (0,1,2).
+ *
+ * Because the queries score differently, they may pick DIFFERENT group
heads for the same
+ * mod3_idv value. After simpleCombine() merges by doc ID, both heads appear
→ duplicates on the
+ * collapse field.
+ */
+ @Test
+ public void testCollapseWithCombinedQueryProducesDuplicates() throws
Exception {
+del("*:*");
+
+// Index 6 docs where mod3_idv groups docs: {3,6}→0, {1,4}→1, {2,5}→2
+// All docs indexed to the same shard (first client) to ensure co-location,
+// which is a requirement for collapse in SolrCloud.
+List docs = new ArrayList<>();
+String[][] data = {
+ {"1", "alpha bravo", "alpha bravo"},
+ {"2", "alpha charlie", "alpha charlie"},
+ {"3", "bravo delta", "bravo delta"},
+ {"4", "charlie delta", "charlie delta"},
+ {"5", "alpha delta", "alpha delta"},
+ {"6", "bravo charlie", "bravo charlie"},
+};
+for (String[] row : data) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", row[0]);
+ doc.addField("text", row[1]);
+ doc.addField("title", row[2]);
+ int idVal = Integer.parseInt(row[0]);
+ doc.addField("mod3_idv", idVal % 3);
+ doc.addField("mod3_sdv", String.valueOf(idVal % 3));
+ docs.add(doc);
+}
+// Index all docs to the first shard (co-location for collapse).
+// indexDoc(client, doc) also indexes to controlClient automatically.
+for (SolrInputDocument doc : docs) {
+ indexDoc(clients.getFirst(), doc);
+}
+commit();
+
+// Two queries that score docs differently, both collapsed on mod3_idv.
+// Query 1 "alpha bravo" favours doc 1 (mod3_idv=1) and doc 3 (mod3_idv=0).
+// Query 2 "charlie delta" favours doc 4 (mod3_idv=1) and doc 3 or 6
(mod3_idv=0).
+// After collapse, each query returns 3 docs (one per mod3_idv value
0,1,2).
+// But the group heads for mod3_idv=1 differ: query1 picks id=1, query2
picks id=4.
+// simpleCombine() merges by doc ID, so both id=1 and id=4 survive →
duplicate on mod3_idv=1.
+// q1 -> 1,2,5,3,6q2 -> 2,4,6,3,5
+// q1 -> 1,2,2,0,0q2 -> 2,1,0,0,2
+// q1 -> 1 for sure, 2,3 => 4 for sure, 3 and 2
+String jsonQuery =
+"{\"queries\":"
Review Comment:
we are on Java 21 now; this really calls for a multi-line string.
##
solr/core/src/test/org/apache/solr/handler/component/CombinedQuerySolrCloudTest.java:
##
@@ -249,6 +269,41 @@ public void testQueriesWithFacetAndHighlights() throws
Exception {
rsp.getHighlighting().get("5").get("title").getFirst());
}
+ /**
+ * Tests the combined query feature with faceting, highlighting and collapse.
+ *
+
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3084423776
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -87,14 +122,53 @@ public static QueryResult simpleCombine(List
queryResults) {
combinedResultsLength,
combinedResultsDocIds,
combinedResultScores,
-Math.max(combinedResultsLength, totalMatches),
+Math.max(combinedResultsLength, totalMatches - removedByCollapse),
combinedResultScores.length > 0 ? combinedResultScores[0] : 0,
TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO);
combinedQueryResults.setDocList(combinedResultSlice);
combinedQueryResults.setDocSet(combinedDocSet);
return combinedQueryResults;
}
+ /**
+ * Removes collapsed duplicates by delegating to SolrIndexSearcher with the
CollapsingPostFilter.
+ * Uses a DocSetQuery wrapped with FunctionScoreQuery.boostByValue to
preserve original scores
+ * from combined sub-queries, then applies the collapse filter to determine
surviving doc IDs.
+ *
+ * This leverages Solr's native collapse infrastructure instead of
manually reading DocValues.
+ */
+ private static DocSet removeCollapsedDuplicatesViaSearcher(
+ CollapsingPostFilter collapseFilter,
+ SolrIndexSearcher searcher,
+ Map uniqueDocIds,
Review Comment:
ah, Thanks for catching this. It shouldn't be using the docSet from the
individual sub-queries rather it should be built on top of docList. I have made
this change now.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3084419954
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -49,12 +62,21 @@ public abstract List combine(
Map> queriesDocMap, SolrParams solrParams);
/**
- * Simple combine query result list as a union.
+ * Combine query result list as a union, optionally deduplicating by a
collapse field. When a
+ * collapse filter is provided, only one document per unique field value is
kept (based on the
+ * collapse sort/score selection). This ensures that collapse semantics are
preserved across
+ * combined queries.
*
* @param queryResults the query results to be combined
+ * @param collapseFilter the collapse post filter, or null if no collapse
dedup is needed
+ * @param searcher the searcher to read field values from, required when
collapseFilter is
+ * non-null
* @return the combined query result
*/
- public static QueryResult simpleCombine(List queryResults) {
+ public static QueryResult simpleCombine(
+ List queryResults,
+ CollapsingPostFilter collapseFilter,
+ SolrIndexSearcher searcher) {
QueryResult combinedQueryResults = new QueryResult();
DocSet combinedDocSet = null;
Review Comment:
it was added but after latest change, not needed.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
dsmiley commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3083783406
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -127,4 +201,72 @@ public static QueryAndResponseCombiner getImplementation(
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Unknown Combining algorithm: " +
algorithm);
}
+
+ /**
+ * A {@link DoubleValuesSource} that returns pre-computed scores for
specific document IDs. Used
+ * with {@link FunctionScoreQuery#boostByValue} to assign original combined
sub-query scores to a
+ * DocSetQuery, enabling the collapse filter to select group heads based on
these scores.
+ */
+ private static class PrecomputedScoreValuesSource extends DoubleValuesSource
{
+
+private final IntDoubleHashMap scoreMap;
+
+PrecomputedScoreValuesSource(IntDoubleHashMap scoreMap) {
+ this.scoreMap = scoreMap;
+}
+
+@Override
+public DoubleValues getValues(LeafReaderContext ctx, DoubleValues
existing) {
+ int base = ctx.docBase;
+ return new DoubleValues() {
+private double currentScore;
+
+@Override
+public double doubleValue() {
+ return currentScore;
+}
+
+@Override
+public boolean advanceExact(int doc) {
+ int globalDoc = base + doc;
+ double score = scoreMap.get(globalDoc);
+ if (score != 0) {
+currentScore = score;
+return true;
+ }
+ return false;
Review Comment:
note: by returning false here... docs in the DocSet but not in the score
will score as 0. I suppose that's fine.
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -49,12 +62,21 @@ public abstract List combine(
Map> queriesDocMap, SolrParams solrParams);
/**
- * Simple combine query result list as a union.
+ * Combine query result list as a union, optionally deduplicating by a
collapse field. When a
+ * collapse filter is provided, only one document per unique field value is
kept (based on the
+ * collapse sort/score selection). This ensures that collapse semantics are
preserved across
+ * combined queries.
*
* @param queryResults the query results to be combined
+ * @param collapseFilter the collapse post filter, or null if no collapse
dedup is needed
+ * @param searcher the searcher to read field values from, required when
collapseFilter is
+ * non-null
* @return the combined query result
*/
- public static QueryResult simpleCombine(List queryResults) {
+ public static QueryResult simpleCombine(
+ List queryResults,
+ CollapsingPostFilter collapseFilter,
+ SolrIndexSearcher searcher) {
QueryResult combinedQueryResults = new QueryResult();
DocSet combinedDocSet = null;
Review Comment:
I think combinedDocSet could still be null through this if the nature of the
requests don't require it (e.g. no faceting). The code you add add in this PR
assumes it's non-null to get a Query from it. Assuming you truly need it for
collapsing (?), you can tell Solr for each subquery to create it.
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -87,14 +122,53 @@ public static QueryResult simpleCombine(List
queryResults) {
combinedResultsLength,
combinedResultsDocIds,
combinedResultScores,
-Math.max(combinedResultsLength, totalMatches),
+Math.max(combinedResultsLength, totalMatches - removedByCollapse),
combinedResultScores.length > 0 ? combinedResultScores[0] : 0,
TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO);
combinedQueryResults.setDocList(combinedResultSlice);
combinedQueryResults.setDocSet(combinedDocSet);
return combinedQueryResults;
}
+ /**
+ * Removes collapsed duplicates by delegating to SolrIndexSearcher with the
CollapsingPostFilter.
+ * Uses a DocSetQuery wrapped with FunctionScoreQuery.boostByValue to
preserve original scores
+ * from combined sub-queries, then applies the collapse filter to determine
surviving doc IDs.
+ *
+ * This leverages Solr's native collapse infrastructure instead of
manually reading DocValues.
+ */
+ private static DocSet removeCollapsedDuplicatesViaSearcher(
+ CollapsingPostFilter collapseFilter,
+ SolrIndexSearcher searcher,
+ Map uniqueDocIds,
Review Comment:
Those are only top-X docs and scores, right?
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -87,14 +122,53 @@ public static QueryResult simpleCombine(List
queryResults) {
combinedResultsLength,
combinedResultsDocIds,
combinedResultScores,
-Math.max(combinedResultsLength,
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3079469742
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -127,4 +202,155 @@ public static QueryAndResponseCombiner getImplementation(
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Unknown Combining algorithm: " +
algorithm);
}
+
+ /**
+ * A query that returns pre-computed scores for specific doc IDs. Used to
preserve original scores
+ * from combined sub-queries when delegating collapse to the searcher.
+ */
+ private static class PrecomputedScoreQuery extends Query {
Review Comment:
makes sense. Thank you for the suggestion.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
dsmiley commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3077120573
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -87,14 +129,47 @@ public static QueryResult simpleCombine(List
queryResults) {
combinedResultsLength,
combinedResultsDocIds,
combinedResultScores,
-Math.max(combinedResultsLength, totalMatches),
+Math.max(combinedResultsLength, totalMatches - removedByCollapse),
combinedResultScores.length > 0 ? combinedResultScores[0] : 0,
TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO);
combinedQueryResults.setDocList(combinedResultSlice);
combinedQueryResults.setDocSet(combinedDocSet);
return combinedQueryResults;
}
+ /**
+ * Removes collapsed duplicates by delegating to SolrIndexSearcher with the
CollapsingPostFilter.
+ * Uses a PrecomputedScoreQuery to preserve original scores from combined
sub-queries, then
+ * applies the collapse filter to determine surviving doc IDs.
+ *
+ * This leverages Solr's native collapse infrastructure instead of
manually reading DocValues.
Review Comment:
very clever; well done!
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -127,4 +202,155 @@ public static QueryAndResponseCombiner getImplementation(
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Unknown Combining algorithm: " +
algorithm);
}
+
+ /**
+ * A query that returns pre-computed scores for specific doc IDs. Used to
preserve original scores
+ * from combined sub-queries when delegating collapse to the searcher.
+ */
+ private static class PrecomputedScoreQuery extends Query {
+private final Map docScores;
+
+PrecomputedScoreQuery(Map docScores) {
+ this.docScores = docScores;
+}
+
+@Override
+public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode,
float boost) {
+ return new PrecomputedScoreWeight(this, docScores, boost);
+}
+
+@Override
+public String toString(String field) {
+ return "PrecomputedScoreQuery(docs=" + docScores.size() + ")";
+}
+
+@Override
+public void visit(QueryVisitor visitor) {
+ visitor.visitLeaf(this);
+}
+
+@Override
+public boolean equals(Object other) {
+ return other instanceof PrecomputedScoreQuery otherQuery
+ && docScores.equals(otherQuery.docScores);
+}
+
+@Override
+public int hashCode() {
+ return classHash() * 31 + docScores.hashCode();
+}
+ }
+
+ private static class PrecomputedScoreWeight extends Weight {
+private final Map docScores;
+private final float boost;
+
+PrecomputedScoreWeight(Query query, Map docScores, float
boost) {
+ super(query);
+ this.docScores = docScores;
+ this.boost = boost;
+}
+
+@Override
+public Explanation explain(LeafReaderContext context, int doc) {
+ int globalDoc = context.docBase + doc;
+ Float score = docScores.get(globalDoc);
+ if (score != null) {
+return Explanation.match(score * boost, "precomputed score");
+ }
+ return Explanation.noMatch("no precomputed score");
+}
+
+@Override
+public ScorerSupplier scorerSupplier(LeafReaderContext context) {
+ List localDocs = getLocalDocs(context);
+ if (localDocs.isEmpty()) {
+return null;
+ }
+ Scorer scorer = new PrecomputedScoreScorer(localDocs, context.docBase,
docScores, boost);
+ return new SolrDefaultScorerSupplier(scorer);
+}
+
+private List getLocalDocs(LeafReaderContext context) {
Review Comment:
the word "local" is not a word I've seen to describe the docs for a segment.
I suggest you use the word "segment" or "leaf"
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -127,4 +202,155 @@ public static QueryAndResponseCombiner getImplementation(
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Unknown Combining algorithm: " +
algorithm);
}
+
+ /**
+ * A query that returns pre-computed scores for specific doc IDs. Used to
preserve original scores
+ * from combined sub-queries when delegating collapse to the searcher.
+ */
+ private static class PrecomputedScoreQuery extends Query {
+private final Map docScores;
+
+PrecomputedScoreQuery(Map docScores) {
+ this.docScores = docScores;
+}
+
+@Override
+public Weight createWeight(IndexSearcher searcher, ScoreMode scoreMode,
float boost) {
+ return new PrecomputedScoreWeight(this, docScores, boost);
+}
+
+@Override
+public String toString(String field) {
+ return "PrecomputedScoreQuery(docs=" + docScores.size() + ")";
+}
+
+@Override
+public void visit(QueryVisitor visitor) {
+ v
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on code in PR #4277:
URL: https://github.com/apache/solr/pull/4277#discussion_r3076631253
##
solr/core/src/java/org/apache/solr/handler/component/combine/QueryAndResponseCombiner.java:
##
@@ -127,4 +202,155 @@ public static QueryAndResponseCombiner getImplementation(
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Unknown Combining algorithm: " +
algorithm);
}
+
+ /**
+ * A query that returns pre-computed scores for specific doc IDs. Used to
preserve original scores
+ * from combined sub-queries when delegating collapse to the searcher.
+ */
+ private static class PrecomputedScoreQuery extends Query {
Review Comment:
I was unsure of the existence of any Query of such kind (?) , so created one
`private` but might move to another class if possibility of reuse.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] SOLR-18195: Support for Collapse Results in Combined Query Component [solr]
ercsonusharma commented on PR #4277: URL: https://github.com/apache/solr/pull/4277#issuecomment-4240625301 David, Could you please take a look when you have a moment? Your past context here on this feature would be super helpful. Thank you @dsmiley -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
