Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley merged PR #3418: URL: https://github.com/apache/solr/pull/3418 -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3675296776 @dsmiley - I see it failed again after re-triggering the same workflows. Luckily, I found a tiny cleanup to commit. Can you help approve the workflow? -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3668594281 Workflow failures seem likely to be infrastructure-related and a flaky SolrCloud test case. Re-triggering the workflow may help it pass. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3666228103 Sounds good, Thank you @dsmiley - done with update from main. I would appreciate it if you could trigger the workflows in the meantime. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3650202448 #3931 should merge first (say in ~2 days), then this one will be simpler and it's ready to go. I expect I should merge this one by the end of next week.. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3649615238 @dsmiley - Thanks for the PR! It looks good on my end. What do you think the next step should be, considering the other PR has been open for quite a few days 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2596378463
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQuerySearchHandler.java:
##
@@ -86,4 +51,13 @@ protected void postPrepareComponents(ResponseBuilder rb) {
crb.propagate();
}
}
+
+ /** Overrides the default list to include {@link CombinedQueryComponent}. */
+ @Override
+ protected List getDefaultComponents() {
+List names = new ArrayList<>(super.getDefaultComponents());
+String replaced = names.set(0, CombinedQueryComponent.COMPONENT_NAME);
+assert replaced.equals(QueryComponent.COMPONENT_NAME);
Review Comment:
This is merely the *default* component list. It doesn't stop someone from
configuring a list in solrconfig.xml that is different.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2596284428
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQuerySearchHandler.java:
##
@@ -86,4 +51,13 @@ protected void postPrepareComponents(ResponseBuilder rb) {
crb.propagate();
}
}
+
+ /** Overrides the default list to include {@link CombinedQueryComponent}. */
+ @Override
+ protected List getDefaultComponents() {
+List names = new ArrayList<>(super.getDefaultComponents());
+String replaced = names.set(0, CombinedQueryComponent.COMPONENT_NAME);
+assert replaced.equals(QueryComponent.COMPONENT_NAME);
Review Comment:
Interesting to learn that this is possible, as I thought it would limit the
user from setting another SearchComponent as the first one.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3621986825 Thanks for all the changes! So, the change would limit, having a new component only can't enable force-distributed behaviour on the existing SearchHandler. > I'd like to merge the refactoring portions of this PR in one go, then the rest of this here. Sounds good to me. I am still not sure if shardinfo reflects on the `process` method being called. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3621680527 Please take a look Sonu. I also refactored SearchHandler a bit to have a clearer flow / readability and also extensibility. I'd like to merge the refactoring portions of this PR in one go, then the rest of this here. Like... I could extract all that to be under SOLR-17982. A TODO I didn't get to tonight is testing the different forceDistrib API with a custom SearchHandler subclass. Both testForceDistrib methods are there but need to be modified. I suspect shards.info may reflect what we want to see. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3617328838 BTW I'm busy tweaking some things and preparing a ~final state that's mergeable. One thing I'm doing is reducing the forcedDistrib API as it's rather internal. Also merging from main. Adding changelog. Updating javadocs; usually to simplify / remove boilerplate likely AI written stuff -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2591563892 ## solr/core/src/java/org/apache/solr/handler/component/combine/package-info.java: ## @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This contains the classes to combine the scores from search index results. Multiple + * implementation of algorithms can be added to support them. + */ +package org.apache.solr.handler.component.combine; Review Comment: The original intention was to group the *Handler and *Component classes within the same package, as done previously. It will affect [this](https://github.com/apache/solr/pull/3418/files#diff-c857e7327c8c35700144083caf8d079b87622fa67aabad3de51728e32287f16bR110) where we might deviate from usual convention of having `solr.` & `solr.` ## solr/core/src/java/org/apache/solr/handler/component/combine/package-info.java: ## @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This contains the classes to combine the scores from search index results. Multiple + * implementation of algorithms can be added to support them. + */ +package org.apache.solr.handler.component.combine; Review Comment: The original intention was to group the *Handler and *Component classes within the same package, as done previously. It will affect [this](https://github.com/apache/solr/pull/3418/files#diff-c857e7327c8c35700144083caf8d079b87622fa67aabad3de51728e32287f16bR110) where we might deviate from usual convention of having solr. & solr. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2591607613 ## solr/core/src/java/org/apache/solr/handler/component/combine/package-info.java: ## @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This contains the classes to combine the scores from search index results. Multiple + * implementation of algorithms can be added to support them. + */ +package org.apache.solr.handler.component.combine; Review Comment: The original intention was to group the *Handler and *Component classes within the same package, as done previously. It will affect [this](https://github.com/apache/solr/pull/3418/files#diff-c857e7327c8c35700144083caf8d079b87622fa67aabad3de51728e32287f16bR110) where we might deviate from usual convention of having `solr.` & `solr.` -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2591563892 ## solr/core/src/java/org/apache/solr/handler/component/combine/package-info.java: ## @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This contains the classes to combine the scores from search index results. Multiple + * implementation of algorithms can be added to support them. + */ +package org.apache.solr.handler.component.combine; Review Comment: The original intention was to group the *Handler and *Component classes within the same package, as done previously. It will affect [this](https://github.com/apache/solr/pull/3418/files#diff-c857e7327c8c35700144083caf8d079b87622fa67aabad3de51728e32287f16bR110) where we might deviate from usual convention of having `solr.` & `solr.` -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2591563892 ## solr/core/src/java/org/apache/solr/handler/component/combine/package-info.java: ## @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This contains the classes to combine the scores from search index results. Multiple + * implementation of algorithms can be added to support them. + */ +package org.apache.solr.handler.component.combine; Review Comment: The original intention was to group the *Handler and *Component classes within the same package, as done previously. However, there is no issue with placing them in the combine package instead. It will affect [this](https://github.com/apache/solr/pull/3418/files#diff-c857e7327c8c35700144083caf8d079b87622fa67aabad3de51728e32287f16bR110) where we might deviate from usual convention of having `solr.` & `solr.` -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2591563892 ## solr/core/src/java/org/apache/solr/handler/component/combine/package-info.java: ## @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This contains the classes to combine the scores from search index results. Multiple + * implementation of algorithms can be added to support them. + */ +package org.apache.solr.handler.component.combine; Review Comment: The original intention was to group the *Handler and *Component classes within the same package, as done previously. However, there is no issue with placing them in the combine package instead. It will affect [this](https://github.com/apache/solr/pull/3418/files#diff-c857e7327c8c35700144083caf8d079b87622fa67aabad3de51728e32287f16bR110) where we might deviate from usual convention of having `solr.` -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2591563892 ## solr/core/src/java/org/apache/solr/handler/component/combine/package-info.java: ## @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This contains the classes to combine the scores from search index results. Multiple + * implementation of algorithms can be added to support them. + */ +package org.apache.solr.handler.component.combine; Review Comment: The original intention was to group the *Handler and *Component classes within the same package, as done previously. However, there is no issue with placing them in the combine package instead. Please let me know if you would like me to move them there. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2591417528 ## solr/core/src/java/org/apache/solr/handler/component/combine/package-info.java: ## @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * This contains the classes to combine the scores from search index results. Multiple + * implementation of algorithms can be added to support them. + */ +package org.apache.solr.handler.component.combine; Review Comment: I was thinking the CombinedQuery* classes would come here too, thus all classes associated with this are together. WDYT? -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3574058453 @dsmiley - Just a gentle reminder to have a look at the above comments and provide approval if we are good to merge. Absolutely no rush at all, especially with the holiday season! :) -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3484388424 > I think my fundamental concerns are addressed. I left some comments. Thank you, @dsmiley for the extensive review. I have addressed those comments as well. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2489065508
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -0,0 +1,284 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.CommonParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * The DistributedCombinedQueryComponentTest class is a JUnit test suite that
evaluates the
+ * functionality of the CombinedQueryComponent in a Solr distributed search
environment. It focuses
+ * on testing the integration of combiner queries with different
configurations.
+ */
+public class DistributedCombinedQueryComponentTest extends
BaseDistributedSearchTestCase {
+
+ private static final int NUM_DOCS = 10;
+ private static final String vectorField = "vector";
+
+ /**
+ * Sets up the test class by initializing the core and setting system
properties. This method is
+ * executed before all test methods in the class.
+ *
+ * @throws Exception if any exception occurs during initialization
+ */
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+initCore("solrconfig-combined-query.xml", "schema-vector-catchall.xml");
+System.setProperty("validateAfterInactivity", "200");
+System.setProperty("solr.httpclient.retries", "0");
+System.setProperty("distribUpdateSoTimeout", "5000");
+ }
+
+ /**
+ * Prepares Solr input documents for indexing, including adding sample data
and vector fields.
+ * This method populates the Solr index with test data, including text,
title, and vector fields.
+ * The vector fields are used to calculate cosine distance for testing
purposes.
+ *
+ * @throws Exception if any error occurs during the indexing process.
+ */
+ private synchronized void prepareIndexDocs() throws Exception {
+List docs = new ArrayList<>();
+fixShardCount(2);
+for (int i = 1; i <= NUM_DOCS; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", Integer.toString(i));
+ doc.addField("text", "test text for doc " + i);
+ doc.addField("title", "title test for doc " + i);
+ doc.addField("mod3_idv", (i % 3));
+ docs.add(doc);
+}
+// cosine distance vector1= 1.0
+docs.get(0).addField(vectorField, Arrays.asList(1f, 2f, 3f, 4f));
+// cosine distance vector1= 0.998
+docs.get(1).addField(vectorField, Arrays.asList(1.5f, 2.5f, 3.5f, 4.5f));
+// cosine distance vector1= 0.992
+docs.get(2).addField(vectorField, Arrays.asList(7.5f, 15.5f, 17.5f,
22.5f));
+// cosine distance vector1= 0.999
+docs.get(3).addField(vectorField, Arrays.asList(1.4f, 2.4f, 3.4f, 4.4f));
+// cosine distance vector1= 0.862
+docs.get(4).addField(vectorField, Arrays.asList(30f, 22f, 35f, 20f));
+// cosine distance vector1= 0.756
+docs.get(5).addField(vectorField, Arrays.asList(40f, 1f, 1f, 200f));
+// cosine distance vector1= 0.970
+docs.get(6).addField(vectorField, Arrays.asList(5f, 10f, 20f, 40f));
+// cosine distance vector1= 0.515
+docs.get(7).addField(vectorField, Arrays.asList(120f, 60f, 30f, 15f));
+// cosine distance vector1= 0.554
+docs.get(8).addField(vectorField, Arrays.asList(200f, 50f, 100f, 25f));
+// cosine distance vector1= 0.997
+docs.get(9).addField(vectorField, Arrays.asList(1.8f, 2.5f, 3.7f, 4.9f));
+del("*:*");
+clients.sort(
+(client1, client2) -> {
+ try {
+if (client2 instanceof HttpSolrClient httpClient2
+&& client1 instanceof HttpSolrClient httpClient1)
+ return new URI(httpClient1.getBaseURL()).getPort()
+ - new URI(httpClient2.getBaseURL()).getPort();
+ }
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3483724418 I'll handle the forceDistrib matter; it was my idea after all so I'll see to creating a test. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2486972983
##
solr/core/src/java/org/apache/solr/search/combine/package-info.java:
##
Review Comment:
Arguably, this package should be under org.apache.solr.handler.component,
and then it would consolidate all the functionality added in this PR, not just
these 2 classes. I know the combiner thing here is sort of general but it's
only used by the new CombineQueryComponent, and I have no reason to believe
that it's going to be used by new things that aren't search components.
##
solr/core/src/test/org/apache/solr/handler/component/CombinedQuerySolrCloudTest.java:
##
@@ -0,0 +1,245 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.cloud.AbstractFullDistribZkTestBase;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.CommonParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * Contains integration tests for the combined query functionality in a
SolrCloud environment. This
+ * class extends AbstractFullDistribZkTestBase to leverage the distributed
testing framework. The
+ * tests cover various scenarios including single and multiple lexical
queries, sorting, pagination,
+ * faceting, and highlighting similar to {@link
DistributedCombinedQueryComponentTest}
+ */
+public class CombinedQuerySolrCloudTest extends AbstractFullDistribZkTestBase {
+
+ private static final int NUM_DOCS = 10;
+ private static final String vectorField = "vector";
+
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+initCore("solrconfig-combined-query.xml", "schema-vector-catchall.xml");
Review Comment:
`initCore` should not be called for these `...Distrib...TestBase` test
classes. Cores are managed by the base test infra.
##
solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java:
##
@@ -21,39 +21,59 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.SimpleOrderedMap;
import org.junit.BeforeClass;
import org.junit.Test;
/**
* The CombinedQueryComponentTest class is an integration test suite for the
CombinedQueryComponent
- * in Solr. It verifies the functionality of the component by performing few
basic queries and
- * validating the responses including limitations and combiner plugin.
+ * in Solr. It verifies the functionality of the component by performing few
basic queries in single
+ * sharded mode and validating the responses including limitations and
combiner plugin.
*/
-public class CombinedQueryComponentTest extends SolrTestCaseJ4 {
+public class CombinedQueryComponentTest extends BaseDistributedSearchTestCase {
Review Comment:
I don't think extending BaseDistributedSearchTestCase makes sense in this
scenario. BaseDistributedSearchTestCase compares a "control" client (single
shard) to a distributed setup of a configurable number of shards; this is its
fundamental purpose. Calling `fixShardCount(1)` misses the point. Can the
single-sharded case can be tested with STCJ4? If not for some weird reason,
surely SolrCloudTestCase.
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -0,0 +1,284 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this fi
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2486775121
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -0,0 +1,284 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.CommonParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * The DistributedCombinedQueryComponentTest class is a JUnit test suite that
evaluates the
+ * functionality of the CombinedQueryComponent in a Solr distributed search
environment. It focuses
+ * on testing the integration of combiner queries with different
configurations.
+ */
+public class DistributedCombinedQueryComponentTest extends
BaseDistributedSearchTestCase {
+
+ private static final int NUM_DOCS = 10;
+ private static final String vectorField = "vector";
+
+ /**
+ * Sets up the test class by initializing the core and setting system
properties. This method is
+ * executed before all test methods in the class.
+ *
+ * @throws Exception if any exception occurs during initialization
+ */
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+initCore("solrconfig-combined-query.xml", "schema-vector-catchall.xml");
+System.setProperty("validateAfterInactivity", "200");
+System.setProperty("solr.httpclient.retries", "0");
+System.setProperty("distribUpdateSoTimeout", "5000");
+ }
+
+ /**
+ * Prepares Solr input documents for indexing, including adding sample data
and vector fields.
+ * This method populates the Solr index with test data, including text,
title, and vector fields.
+ * The vector fields are used to calculate cosine distance for testing
purposes.
+ *
+ * @throws Exception if any error occurs during the indexing process.
+ */
+ private synchronized void prepareIndexDocs() throws Exception {
+List docs = new ArrayList<>();
+fixShardCount(2);
+for (int i = 1; i <= NUM_DOCS; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", Integer.toString(i));
+ doc.addField("text", "test text for doc " + i);
+ doc.addField("title", "title test for doc " + i);
+ doc.addField("mod3_idv", (i % 3));
+ docs.add(doc);
+}
+// cosine distance vector1= 1.0
+docs.get(0).addField(vectorField, Arrays.asList(1f, 2f, 3f, 4f));
+// cosine distance vector1= 0.998
+docs.get(1).addField(vectorField, Arrays.asList(1.5f, 2.5f, 3.5f, 4.5f));
+// cosine distance vector1= 0.992
+docs.get(2).addField(vectorField, Arrays.asList(7.5f, 15.5f, 17.5f,
22.5f));
+// cosine distance vector1= 0.999
+docs.get(3).addField(vectorField, Arrays.asList(1.4f, 2.4f, 3.4f, 4.4f));
+// cosine distance vector1= 0.862
+docs.get(4).addField(vectorField, Arrays.asList(30f, 22f, 35f, 20f));
+// cosine distance vector1= 0.756
+docs.get(5).addField(vectorField, Arrays.asList(40f, 1f, 1f, 200f));
+// cosine distance vector1= 0.970
+docs.get(6).addField(vectorField, Arrays.asList(5f, 10f, 20f, 40f));
+// cosine distance vector1= 0.515
+docs.get(7).addField(vectorField, Arrays.asList(120f, 60f, 30f, 15f));
+// cosine distance vector1= 0.554
+docs.get(8).addField(vectorField, Arrays.asList(200f, 50f, 100f, 25f));
+// cosine distance vector1= 0.997
+docs.get(9).addField(vectorField, Arrays.asList(1.8f, 2.5f, 3.7f, 4.9f));
+del("*:*");
+clients.sort(
+(client1, client2) -> {
+ try {
+if (client2 instanceof HttpSolrClient httpClient2
+&& client1 instanceof HttpSolrClient httpClient1)
+ return new URI(httpClient1.getBaseURL()).getPort()
+ - new URI(httpClient2.getBaseURL()).getPort();
+ } catch
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3476411171 @dsmiley Was I able to address your concern through above changes and responses? Please let me know if there is more. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2453916641
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,629 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import static java.lang.Math.max;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+ private static final String RESPONSE_PER_QUERY_KEY = "response_per_query";
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseIn
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2453918348
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
Review Comment:
Yeah, definitely - A SolrCloud test can be introduced by extending
`AbstractFullDistribZkTestBase`. Will send it out shortly.
Update: Added a test for SolrCloud.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2453918348
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
Review Comment:
Yeah, definitely - A SolrCloud test can be introduced by extending
`AbstractFullDistribZkTestBase`. Will send it out shortly.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2453916641
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,629 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import static java.lang.Math.max;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+ private static final String RESPONSE_PER_QUERY_KEY = "response_per_query";
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseIn
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2449617288
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,629 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import static java.lang.Math.max;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+ private static final String RESPONSE_PER_QUERY_KEY = "response_per_query";
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseInt(maxQ
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2436004520
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,629 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import static java.lang.Math.max;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+ private static final String RESPONSE_PER_QUERY_KEY = "response_per_query";
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseInt(maxQ
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2434634317
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,629 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import static java.lang.Math.max;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+ private static final String RESPONSE_PER_QUERY_KEY = "response_per_query";
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseIn
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2434417321
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl =
+ String.format(Locale.ROOT, "%s://%s:%d%s/%s", scheme, host, port,
context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
+ return;
+}
Review Comment:
Also, the URL core construction here seems like it should be a utility
method on HttpSolrCall
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2434411532
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl =
+ String.format(Locale.ROOT, "%s://%s:%d%s/%s", scheme, host, port,
context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
+ return;
+}
Review Comment:
Ah. I see so it needs to happen _somewhere_, and you've put it here. It
feels out of place in this method, however. Based on what you showed me, it
could be in HttpShardHandler where StandaloneReplicaSource.Builder is used.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2434634317
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,629 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import static java.lang.Math.max;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+ private static final String RESPONSE_PER_QUERY_KEY = "response_per_query";
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseIn
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2434418768
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
Review Comment:
It appears this would activate for SolrCloud... yet we wouldn't want that.
As I suggest below, by moving this logic to the one spot where we need it,
then this whole portion of forceDistributed is removed.
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,629 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import static java.lang.Math.max;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+ private static final String RESPONSE_PER_QUERY_KEY = "response_per_query";
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String classNam
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2434677644
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
Review Comment:
This would activate for both - SolrCloud and non-SolrCloud as we don't
intend to go through the process() method of SearchComponent directly through
`shortCircuit` rather we want it to happen through creating a HttpShardHandler
which would query all the shards parallely and merge the result at the
coordinator (even in case of one shard).
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3402817819 @dsmiley - Just checking if you have any concerns apart from a few mentioned above, which I have resolved recently. Not sure if you had a chance to review it completely yet. Would you be able to advise on the best path forward? -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2436044532
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
Review Comment:
It seems we have no SolrCloud test introduced here... I was hoping to run in
a debugger to see for myself. H. I could imagine a dummy
ForceDistribSearchComponent test designed only to test that forceDistrib works.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3406714433 Sorry; this PR deserves more attention but I've been prioritizing my limited time to Solr 10. I'll return to this as soon as I can... perhaps tonight. I said that to myself yesterday so it's at least at the top of my discretionary list. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2384826680
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -0,0 +1,284 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.CommonParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * The DistributedCombinedQueryComponentTest class is a JUnit test suite that
evaluates the
+ * functionality of the CombinedQueryComponent in a Solr distributed search
environment. It focuses
+ * on testing the integration of combiner queries with different
configurations.
+ */
+public class DistributedCombinedQueryComponentTest extends
BaseDistributedSearchTestCase {
+
+ private static final int NUM_DOCS = 10;
+ private static final String vectorField = "vector";
+
+ /**
+ * Sets up the test class by initializing the core and setting system
properties. This method is
+ * executed before all test methods in the class.
+ *
+ * @throws Exception if any exception occurs during initialization
+ */
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+initCore("solrconfig-combined-query.xml", "schema-vector-catchall.xml");
+System.setProperty("validateAfterInactivity", "200");
+System.setProperty("solr.httpclient.retries", "0");
+System.setProperty("distribUpdateSoTimeout", "5000");
+ }
+
+ /**
+ * Prepares Solr input documents for indexing, including adding sample data
and vector fields.
+ * This method populates the Solr index with test data, including text,
title, and vector fields.
+ * The vector fields are used to calculate cosine distance for testing
purposes.
+ *
+ * @throws Exception if any error occurs during the indexing process.
+ */
+ private synchronized void prepareIndexDocs() throws Exception {
+List docs = new ArrayList<>();
+fixShardCount(2);
+for (int i = 1; i <= NUM_DOCS; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", Integer.toString(i));
+ doc.addField("text", "test text for doc " + i);
+ doc.addField("title", "title test for doc " + i);
+ doc.addField("mod3_idv", (i % 3));
+ docs.add(doc);
+}
+// cosine distance vector1= 1.0
+docs.get(0).addField(vectorField, Arrays.asList(1f, 2f, 3f, 4f));
+// cosine distance vector1= 0.998
+docs.get(1).addField(vectorField, Arrays.asList(1.5f, 2.5f, 3.5f, 4.5f));
+// cosine distance vector1= 0.992
+docs.get(2).addField(vectorField, Arrays.asList(7.5f, 15.5f, 17.5f,
22.5f));
+// cosine distance vector1= 0.999
+docs.get(3).addField(vectorField, Arrays.asList(1.4f, 2.4f, 3.4f, 4.4f));
+// cosine distance vector1= 0.862
+docs.get(4).addField(vectorField, Arrays.asList(30f, 22f, 35f, 20f));
+// cosine distance vector1= 0.756
+docs.get(5).addField(vectorField, Arrays.asList(40f, 1f, 1f, 200f));
+// cosine distance vector1= 0.970
+docs.get(6).addField(vectorField, Arrays.asList(5f, 10f, 20f, 40f));
+// cosine distance vector1= 0.515
+docs.get(7).addField(vectorField, Arrays.asList(120f, 60f, 30f, 15f));
+// cosine distance vector1= 0.554
+docs.get(8).addField(vectorField, Arrays.asList(200f, 50f, 100f, 25f));
+// cosine distance vector1= 0.997
+docs.get(9).addField(vectorField, Arrays.asList(1.8f, 2.5f, 3.7f, 4.9f));
+del("*:*");
+clients.sort(
+(client1, client2) -> {
+ try {
+if (client2 instanceof HttpSolrClient httpClient2
+&& client1 instanceof HttpSolrClient httpClient1)
+ return new URI(httpClient1.getBaseURL()).getPort()
+ - new URI(httpClient2.getBaseURL()).getPort();
+ }
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2384806103
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -0,0 +1,284 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.CommonParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * The DistributedCombinedQueryComponentTest class is a JUnit test suite that
evaluates the
+ * functionality of the CombinedQueryComponent in a Solr distributed search
environment. It focuses
+ * on testing the integration of combiner queries with different
configurations.
+ */
+public class DistributedCombinedQueryComponentTest extends
BaseDistributedSearchTestCase {
+
+ private static final int NUM_DOCS = 10;
+ private static final String vectorField = "vector";
+
+ /**
+ * Sets up the test class by initializing the core and setting system
properties. This method is
+ * executed before all test methods in the class.
+ *
+ * @throws Exception if any exception occurs during initialization
+ */
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+initCore("solrconfig-combined-query.xml", "schema-vector-catchall.xml");
+System.setProperty("validateAfterInactivity", "200");
+System.setProperty("solr.httpclient.retries", "0");
+System.setProperty("distribUpdateSoTimeout", "5000");
+ }
+
+ /**
+ * Prepares Solr input documents for indexing, including adding sample data
and vector fields.
+ * This method populates the Solr index with test data, including text,
title, and vector fields.
+ * The vector fields are used to calculate cosine distance for testing
purposes.
+ *
+ * @throws Exception if any error occurs during the indexing process.
+ */
+ private synchronized void prepareIndexDocs() throws Exception {
+List docs = new ArrayList<>();
+fixShardCount(2);
+for (int i = 1; i <= NUM_DOCS; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", Integer.toString(i));
+ doc.addField("text", "test text for doc " + i);
+ doc.addField("title", "title test for doc " + i);
+ doc.addField("mod3_idv", (i % 3));
+ docs.add(doc);
+}
+// cosine distance vector1= 1.0
+docs.get(0).addField(vectorField, Arrays.asList(1f, 2f, 3f, 4f));
+// cosine distance vector1= 0.998
+docs.get(1).addField(vectorField, Arrays.asList(1.5f, 2.5f, 3.5f, 4.5f));
+// cosine distance vector1= 0.992
+docs.get(2).addField(vectorField, Arrays.asList(7.5f, 15.5f, 17.5f,
22.5f));
+// cosine distance vector1= 0.999
+docs.get(3).addField(vectorField, Arrays.asList(1.4f, 2.4f, 3.4f, 4.4f));
+// cosine distance vector1= 0.862
+docs.get(4).addField(vectorField, Arrays.asList(30f, 22f, 35f, 20f));
+// cosine distance vector1= 0.756
+docs.get(5).addField(vectorField, Arrays.asList(40f, 1f, 1f, 200f));
+// cosine distance vector1= 0.970
+docs.get(6).addField(vectorField, Arrays.asList(5f, 10f, 20f, 40f));
+// cosine distance vector1= 0.515
+docs.get(7).addField(vectorField, Arrays.asList(120f, 60f, 30f, 15f));
+// cosine distance vector1= 0.554
+docs.get(8).addField(vectorField, Arrays.asList(200f, 50f, 100f, 25f));
+// cosine distance vector1= 0.997
+docs.get(9).addField(vectorField, Arrays.asList(1.8f, 2.5f, 3.7f, 4.9f));
+del("*:*");
+clients.sort(
+(client1, client2) -> {
+ try {
+if (client2 instanceof HttpSolrClient httpClient2
+&& client1 instanceof HttpSolrClient httpClient1)
+ return new URI(httpClient1.getBaseURL()).getPort()
+ - new URI(httpClient2.getBaseURL()).getPort();
+ }
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2384510445
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -0,0 +1,284 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.CommonParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * The DistributedCombinedQueryComponentTest class is a JUnit test suite that
evaluates the
+ * functionality of the CombinedQueryComponent in a Solr distributed search
environment. It focuses
+ * on testing the integration of combiner queries with different
configurations.
+ */
+public class DistributedCombinedQueryComponentTest extends
BaseDistributedSearchTestCase {
+
+ private static final int NUM_DOCS = 10;
+ private static final String vectorField = "vector";
+
+ /**
+ * Sets up the test class by initializing the core and setting system
properties. This method is
+ * executed before all test methods in the class.
+ *
+ * @throws Exception if any exception occurs during initialization
+ */
+ @BeforeClass
+ public static void setUpClass() throws Exception {
+initCore("solrconfig-combined-query.xml", "schema-vector-catchall.xml");
+System.setProperty("validateAfterInactivity", "200");
+System.setProperty("solr.httpclient.retries", "0");
+System.setProperty("distribUpdateSoTimeout", "5000");
+ }
+
+ /**
+ * Prepares Solr input documents for indexing, including adding sample data
and vector fields.
+ * This method populates the Solr index with test data, including text,
title, and vector fields.
+ * The vector fields are used to calculate cosine distance for testing
purposes.
+ *
+ * @throws Exception if any error occurs during the indexing process.
+ */
+ private synchronized void prepareIndexDocs() throws Exception {
+List docs = new ArrayList<>();
+fixShardCount(2);
+for (int i = 1; i <= NUM_DOCS; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+ doc.addField("id", Integer.toString(i));
+ doc.addField("text", "test text for doc " + i);
+ doc.addField("title", "title test for doc " + i);
+ doc.addField("mod3_idv", (i % 3));
+ docs.add(doc);
+}
+// cosine distance vector1= 1.0
+docs.get(0).addField(vectorField, Arrays.asList(1f, 2f, 3f, 4f));
+// cosine distance vector1= 0.998
+docs.get(1).addField(vectorField, Arrays.asList(1.5f, 2.5f, 3.5f, 4.5f));
+// cosine distance vector1= 0.992
+docs.get(2).addField(vectorField, Arrays.asList(7.5f, 15.5f, 17.5f,
22.5f));
+// cosine distance vector1= 0.999
+docs.get(3).addField(vectorField, Arrays.asList(1.4f, 2.4f, 3.4f, 4.4f));
+// cosine distance vector1= 0.862
+docs.get(4).addField(vectorField, Arrays.asList(30f, 22f, 35f, 20f));
+// cosine distance vector1= 0.756
+docs.get(5).addField(vectorField, Arrays.asList(40f, 1f, 1f, 200f));
+// cosine distance vector1= 0.970
+docs.get(6).addField(vectorField, Arrays.asList(5f, 10f, 20f, 40f));
+// cosine distance vector1= 0.515
+docs.get(7).addField(vectorField, Arrays.asList(120f, 60f, 30f, 15f));
+// cosine distance vector1= 0.554
+docs.get(8).addField(vectorField, Arrays.asList(200f, 50f, 100f, 25f));
+// cosine distance vector1= 0.997
+docs.get(9).addField(vectorField, Arrays.asList(1.8f, 2.5f, 3.7f, 4.9f));
+del("*:*");
+clients.sort(
+(client1, client2) -> {
+ try {
+if (client2 instanceof HttpSolrClient httpClient2
+&& client1 instanceof HttpSolrClient httpClient1)
+ return new URI(httpClient1.getBaseURL()).getPort()
+ - new URI(httpClient2.getBaseURL()).getPort();
+ } catch
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3341358574 @dsmiley I had a chance to take a look at Christine's POC changes and have updated my findings there. >I'll need to come at this afresh (not just looking at deltas from previous review) Just checking in—have you had a chance to take a look at this yet? -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3301699447 okay @dsmiley . So I have kept only the Way 2 for now, removing Way 1 completely. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2353470681 ## solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java: ## Review Comment: I concur that in a realistic setting, sharding will yield score variances or ties that could change the final ordering. But why test under that circumstance? It only makes testing harder and for a reader of the tests to understand that the assertions are valid. _We won't have less test coverage by fixing the scores somehow._ I think the vast majority of tests here should operate in this circumstance, which makes them clear to understand / validate. This should save you time (less tests to maintain). If the way RRF tie brakes matters (I doubt it), I could understand adding such a test to ensure we don't introduce a regression. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2332148796
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQuerySearchHandler.java:
##
@@ -81,4 +88,33 @@ protected void postPrepareComponents(ResponseBuilder rb) {
crb.propagate();
}
}
+
+ /**
+ * rb.distrib {@link ResponseBuilder} must be set for the combined query to
work in case of single
+ * core standalone mode. This method set the parameter explicitly with other
required solr param
+ * i.e. shards
+ *
+ * @param req the SolrQueryRequest
+ * @return boolean denoting whether the request can be marked as distributed.
+ */
+ @Override
+ protected boolean isDistrib(SolrQueryRequest req) {
+boolean isDistrib = super.isDistrib(req);
+if (!isDistrib
+&& !req.getParams().getBool(ShardParams.IS_SHARD, false)
+&& req.getHttpSolrCall() != null) {
+ log.info("Configuring distributed mode to enable Combined Query.");
+ ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl = String.format("%s://%s:%d%s/%s", scheme, host,
port, context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
Review Comment:
I followed what you said
[here](https://github.com/apache/solr/pull/3418#discussion_r2322678396):
I agree it's a bit sneaky. However, Apologies, but yet I didn't follow the
part below:
> for a component or handler to communicate, we need the distributed search
algorithm (no so-called short-circuit).
afaik, in standalone distributed search, we need the shards value to be
passed by the user.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2354613676 ## solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java: ## Review Comment: Alright, I have removed the Non-distributed test case separately using `queryServer` and now, using `query` directly with constant score queries. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2352867177
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl =
+ String.format(Locale.ROOT, "%s://%s:%d%s/%s", scheme, host, port,
context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
+ return;
+}
Review Comment:
No, shortCircuit=false is not enough as ShardHandler also [needs the
`shards`
parameter](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java#L483)
which comes directly from SolrParams. While, making shortCircuit=true makes
[the rb.isDistrib=false
](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java#L449)
and force the method to not go through the shardHandler requests, rather stick
to [node local
indexes](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java#L493)
through SearchComponent.process method.
I couldn't find this code anywhere but created myself.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3300226161 > IMO, If the data is independently sharded and shard-level scoring doesn't matter in overall query relevance, it may not make sense to first combine shard results based on their original scores and then apply RRF per query (as done in Way 2). In such cases, users may prefer Way 1. I disagree. Only having per-shard RRF quickly devolves to shard interleaving as the shard count increases, since at the coordination/aggregation, there's no global overall ranking measure left anymore. The results from each shard are ultimately treated as equivalent across the shards. Consequently, if say the _real_ RRF best result pointed clearly to one document (best score of both sub-queries, lets say), then in the per-shard RRF it'd merely be _arbitrarily_ somewhere in the top-20 if say there are 20 shards. Nobody would want that. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2352932992
##
solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java:
##
@@ -21,39 +21,59 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.SimpleOrderedMap;
import org.junit.BeforeClass;
import org.junit.Test;
/**
* The CombinedQueryComponentTest class is an integration test suite for the
CombinedQueryComponent
- * in Solr. It verifies the functionality of the component by performing few
basic queries and
- * validating the responses including limitations and combiner plugin.
+ * in Solr. It verifies the functionality of the component by performing few
basic queries in single
+ * sharded mode and validating the responses including limitations and
combiner plugin.
*/
-public class CombinedQueryComponentTest extends SolrTestCaseJ4 {
+public class CombinedQueryComponentTest extends BaseDistributedSearchTestCase {
Review Comment:
Since, the algorithm doesn't support non-distributed request anymore, it
didn't make sense to add test cases by extending SolrTestCaseJ4 so I have
changed it. However, this class can be merged with other CombinedQueryTest but
the key idea was: `The class verifies the functionality of the component by
performing few basic queries in single sharded mode and validating the
responses including limitations and combiner plugin.`
If you feel it should be merged to other test classes, I am open to making
those changes when we finalise [this
one](https://github.com/apache/solr/pull/3418#discussion_r2331983004)
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3299328167 > I have doubts that it makes sense to include "Way 1" if, I imagine, it increases the complexity/documentation matters and more fundamentally... why would someone choose it. I don't think it introduces any additional complexity in terms of code or documentation (adding a parameter combiner.method), rather it reuses several piece of code. IMO, If the data is independently sharded and shard-level scoring doesn't matter in overall query relevance, it may not make sense to first combine shard results based on their original scores and then apply RRF per query (as done in Way 2). In such cases, users may prefer Way 1. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2352867177
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl =
+ String.format(Locale.ROOT, "%s://%s:%d%s/%s", scheme, host, port,
context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
+ return;
+}
Review Comment:
No, shortCircuit=false is not enough as it helps make [the
rb.isDistrib=false
](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java#L449)
and force the method to not go through the shardHandler requests, rather stick
to [node local
indexes](https://github.com/apache/solr/blob/main/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java#L493)
through SearchComponent.process method.
I couldn't find this code anywhere but created myself.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2352636387
##
solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java:
##
@@ -21,39 +21,59 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.response.QueryResponse;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.SimpleOrderedMap;
import org.junit.BeforeClass;
import org.junit.Test;
/**
* The CombinedQueryComponentTest class is an integration test suite for the
CombinedQueryComponent
- * in Solr. It verifies the functionality of the component by performing few
basic queries and
- * validating the responses including limitations and combiner plugin.
+ * in Solr. It verifies the functionality of the component by performing few
basic queries in single
+ * sharded mode and validating the responses including limitations and
combiner plugin.
*/
-public class CombinedQueryComponentTest extends SolrTestCaseJ4 {
+public class CombinedQueryComponentTest extends BaseDistributedSearchTestCase {
Review Comment:
I'm now confused on how to differentiate the testing approach between this
class and DistributedCombinedQueryComponentTest. _That_ one is named
appropriately (consistently with other distributed tests) and extends
BaseDistributedSearchTestCase. I'm not sure *this* test has a role/purpose if
that one can be comprehensive.
##
solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java:
##
@@ -500,6 +505,28 @@ public void prepDistributed(ResponseBuilder rb) {
}
}
+ private static void forceDistributed(ResponseBuilder rb) {
+SolrQueryRequest req = rb.req;
+ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+solrParams.set("shortCircuit", false);
+req.setParams(solrParams);
+if (req.getHttpSolrCall() != null
+&& StringUtils.isEmpty(req.getParams().get(ShardParams.SHARDS))) {
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl =
+ String.format(Locale.ROOT, "%s://%s:%d%s/%s", scheme, host, port,
context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
+ return;
+}
Review Comment:
This part looks suspicious to me; so shortCircuit=false isn't enough? Did
you construct this based on seeing similar code elsewhere (where?) to create a
shards URL?
##
solr/core/src/java/org/apache/solr/handler/component/ResponseBuilder.java:
##
@@ -141,6 +141,15 @@ public ResponseBuilder(
public List outgoing; // requests to be sent
public List finished; // requests that have received responses
from all shards
public String shortCircuitedURL;
+ private boolean forcedDistrib = false;
Review Comment:
I thought of this as well, yet it feels weird to put a request aspect into
the response data holder. It doesn't affect the response. Any way... it's
really minor. This is pragmatic.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2341551234 ## solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java: ## Review Comment: True, but I also wanted to show the results are not the same as what was expected in a single shard, as I described in the jira issue. Asserting only ideal queries (like constant/fixed score) or a type of queries (lexical with ExactStatsCache) may not give a clear picture to the algorithm IMHO. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2337697778
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQuerySearchHandler.java:
##
@@ -81,4 +88,33 @@ protected void postPrepareComponents(ResponseBuilder rb) {
crb.propagate();
}
}
+
+ /**
+ * rb.distrib {@link ResponseBuilder} must be set for the combined query to
work in case of single
+ * core standalone mode. This method set the parameter explicitly with other
required solr param
+ * i.e. shards
+ *
+ * @param req the SolrQueryRequest
+ * @return boolean denoting whether the request can be marked as distributed.
+ */
+ @Override
+ protected boolean isDistrib(SolrQueryRequest req) {
+boolean isDistrib = super.isDistrib(req);
+if (!isDistrib
+&& !req.getParams().getBool(ShardParams.IS_SHARD, false)
+&& req.getHttpSolrCall() != null) {
+ log.info("Configuring distributed mode to enable Combined Query.");
+ ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl = String.format("%s://%s:%d%s/%s", scheme, host,
port, context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
Review Comment:
Makes sense. Have changed the logic in such a way that it can be useful for
other search components as well where a distributed request can be forced
depending on the component requirement.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2332148796
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQuerySearchHandler.java:
##
@@ -81,4 +88,33 @@ protected void postPrepareComponents(ResponseBuilder rb) {
crb.propagate();
}
}
+
+ /**
+ * rb.distrib {@link ResponseBuilder} must be set for the combined query to
work in case of single
+ * core standalone mode. This method set the parameter explicitly with other
required solr param
+ * i.e. shards
+ *
+ * @param req the SolrQueryRequest
+ * @return boolean denoting whether the request can be marked as distributed.
+ */
+ @Override
+ protected boolean isDistrib(SolrQueryRequest req) {
+boolean isDistrib = super.isDistrib(req);
+if (!isDistrib
+&& !req.getParams().getBool(ShardParams.IS_SHARD, false)
+&& req.getHttpSolrCall() != null) {
+ log.info("Configuring distributed mode to enable Combined Query.");
+ ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl = String.format("%s://%s:%d%s/%s", scheme, host,
port, context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
Review Comment:
I followed what you said
[here](https://github.com/apache/solr/pull/3418#discussion_r2322678396):
I agree it's a bit sneaky. However, Apologies, but yet I didn't follow the
part below:
> for a component or handler to communicate, we need the distributed search
algorithm (no so-called short-circuit).
afaik, in standalone distributed search, we need the shards value to be
passed by the user.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2334303365
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQuerySearchHandler.java:
##
@@ -81,4 +88,33 @@ protected void postPrepareComponents(ResponseBuilder rb) {
crb.propagate();
}
}
+
+ /**
+ * rb.distrib {@link ResponseBuilder} must be set for the combined query to
work in case of single
+ * core standalone mode. This method set the parameter explicitly with other
required solr param
+ * i.e. shards
+ *
+ * @param req the SolrQueryRequest
+ * @return boolean denoting whether the request can be marked as distributed.
+ */
+ @Override
+ protected boolean isDistrib(SolrQueryRequest req) {
+boolean isDistrib = super.isDistrib(req);
+if (!isDistrib
+&& !req.getParams().getBool(ShardParams.IS_SHARD, false)
+&& req.getHttpSolrCall() != null) {
+ log.info("Configuring distributed mode to enable Combined Query.");
+ ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl = String.format("%s://%s:%d%s/%s", scheme, host,
port, context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
Review Comment:
It's not just standalone; a single shard collection will "short-circuit". I
was going to link to my same comment. I understand the need/desire.
Interestingly, this is the first component to want to prevent the short
circuit, but I could see it being useful for any search component author who
doesn't want the extra development cost of an optimized single-shard algorithm.
Perhaps if SearchHandler called SearchComponent.prepare before it
initialized ShardHandler (that latter part needs to know if short-circuit),
then a component could add the short-circuit param.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2331961665
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQuerySearchHandler.java:
##
@@ -81,4 +88,33 @@ protected void postPrepareComponents(ResponseBuilder rb) {
crb.propagate();
}
}
+
+ /**
+ * rb.distrib {@link ResponseBuilder} must be set for the combined query to
work in case of single
+ * core standalone mode. This method set the parameter explicitly with other
required solr param
+ * i.e. shards
+ *
+ * @param req the SolrQueryRequest
+ * @return boolean denoting whether the request can be marked as distributed.
+ */
+ @Override
+ protected boolean isDistrib(SolrQueryRequest req) {
+boolean isDistrib = super.isDistrib(req);
+if (!isDistrib
+&& !req.getParams().getBool(ShardParams.IS_SHARD, false)
+&& req.getHttpSolrCall() != null) {
+ log.info("Configuring distributed mode to enable Combined Query.");
Review Comment:
not worth logging; or maybe trace
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQuerySearchHandler.java:
##
@@ -81,4 +88,33 @@ protected void postPrepareComponents(ResponseBuilder rb) {
crb.propagate();
}
}
+
+ /**
+ * rb.distrib {@link ResponseBuilder} must be set for the combined query to
work in case of single
+ * core standalone mode. This method set the parameter explicitly with other
required solr param
+ * i.e. shards
+ *
+ * @param req the SolrQueryRequest
+ * @return boolean denoting whether the request can be marked as distributed.
+ */
+ @Override
+ protected boolean isDistrib(SolrQueryRequest req) {
+boolean isDistrib = super.isDistrib(req);
+if (!isDistrib
+&& !req.getParams().getBool(ShardParams.IS_SHARD, false)
+&& req.getHttpSolrCall() != null) {
+ log.info("Configuring distributed mode to enable Combined Query.");
+ ModifiableSolrParams solrParams = new
ModifiableSolrParams(req.getParams());
+ String scheme = req.getHttpSolrCall().getReq().getScheme();
+ String host = req.getHttpSolrCall().getReq().getServerName();
+ int port = req.getHttpSolrCall().getReq().getServerPort();
+ String context = req.getHttpSolrCall().getReq().getContextPath();
+ String core = req.getCore().getName();
+ String localShardUrl = String.format("%s://%s:%d%s/%s", scheme, host,
port, context, core);
+ solrParams.set(ShardParams.SHARDS, localShardUrl);
+ req.setParams(solrParams);
Review Comment:
I think it's a bit sneaky that a predicate looking method has a side-effect.
This is a hack to work around a need for something proper -- for a component
or handler to communicate we need the distributed search algorithm (no
so-called short-circuit).
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
Review Comment:
Please update this test to not call `queryServer` since it doesn't compare
against the control client. I wish that method came with a big fat disclaimer
and was named differently that didn't look so normal. There are probably more
callers of it than there should be. I did
https://github.com/apache/solr/pull/3639 just now.
In order for this test to compare both single shard & multi-shard with score
relevancy, you could do a couple different things. One is to use the
ExactStatsCache for distributed-IDF. *Or*, write queries that set a
constant/fixed score. That would actually be most clear.
With such a test, you don't need to test non-distributed since the test
infrastructure here allows you to do both as one.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3264406707 >Yet this PR/approach will not be able to comply since unlike most (all?) components, its results are affected substantially by distributed-search. The (unsaid?) vision of sharding / distributed-search was getting the same results as a single shard, and Solr does the work to pull off that trick, with plenty of tests demonstrating it does. In fact I'd say, with great disappointment, that the observed (by a user) results of this component will not be RRF when there's distributed search over shards. pushed a change to the PR that adds an option for the user to choose which Combiner method to use — Way 1 (pre) or Way 2 (post). -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2322092316 ## solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java: ## @@ -0,0 +1,251 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.handler.component; + +import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_START; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.params.CommonParams; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * The CombinedQueryComponentTest class is a unit test suite for the CombinedQueryComponent in Solr. + * It verifies the functionality of the component by performing various queries and validating the + * responses. + */ +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) Review Comment: Feel free to at-mention me to look into a failure. Solr tests are rather stringent to enforce best practices like ensuring threads aren't running when a test finishes. When this is violated, usually it means the test hasn't cleaned up (closed) something. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2320851066
##
solr/core/src/java/org/apache/solr/search/combine/ReciprocalRankFusion.java:
##
@@ -0,0 +1,262 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.combine;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TotalHits;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.component.ShardDoc;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.DocSet;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrDocumentFetcher;
+import org.apache.solr.search.SolrIndexSearcher;
+
+/**
+ * This class implements a query and response combiner that uses the
Reciprocal Rank Fusion (RRF)
+ * algorithm to combine multiple ranked lists into a single ranked list.
+ */
+public class ReciprocalRankFusion extends QueryAndResponseCombiner {
+
+ private int k;
+
+ public ReciprocalRankFusion() {
+this.k = CombinerParams.DEFAULT_COMBINER_RRF_K;
+ }
+
+ @Override
+ public void init(NamedList args) {
+Object kParam = args.get("k");
+if (kParam != null) {
+ this.k = Integer.parseInt(kParam.toString());
+}
+ }
+
+ public int getK() {
+return k;
+ }
+
+ @Override
+ public QueryResult combine(List rankedLists, SolrParams
solrParams) {
+int kVal = solrParams.getInt(CombinerParams.COMBINER_RRF_K, this.k);
+List docListAndSet =
getDocListsAndSetFromQueryResults(rankedLists);
+QueryResult combinedResult = new QueryResult();
+combineResults(combinedResult, docListAndSet, false, kVal);
+return combinedResult;
+ }
+
+ private static List getDocListsAndSetFromQueryResults(
Review Comment:
minor: IMO this method isn't worthwhile. Instead, just pass the
`List` to combineResults. That method need not be separated
either; I think just inline it.
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentLis
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322141633
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -205,7 +203,7 @@ public void testMultipleQueryWithSort() throws Exception {
}
/**
- * Tests the hybrid query functionality of the system.
+ * Tests the hybrid query functionality of the system with various setting
of pagination.
Review Comment:
thanks for improving this test :-)
##
solr/core/src/java/org/apache/solr/search/combine/ReciprocalRankFusion.java:
##
@@ -69,21 +68,24 @@ public int getK() {
@Override
public QueryResult combine(List rankedLists, SolrParams
solrParams) {
int kVal = solrParams.getInt(CombinerParams.COMBINER_RRF_K, this.k);
-List docListAndSet =
getDocListsAndSetFromQueryResults(rankedLists);
QueryResult combinedResult = new QueryResult();
-combineResults(combinedResult, docListAndSet, false, kVal);
+combineResults(combinedResult, rankedLists, false, kVal);
return combinedResult;
}
- private static List getDocListsAndSetFromQueryResults(
- List rankedLists) {
-List docLists = new ArrayList<>(rankedLists.size());
-for (QueryResult rankedList : rankedLists) {
- docLists.add(rankedList.getDocListAndSet());
-}
-return docLists;
- }
-
+ /**
+ * Merges per-shard ranked results using Reciprocal Rank Fusion (RRF).
Review Comment:
Reading this sentence, I think okay this is a typical application of RRF.
And the code does that, so it's technically not wrong. But lets be real with
the reader; in practice this is going to interleave the results because docs
uniquely only appear in one shard. Say that. And say that you change the
score to the RRF score.
As an aside, it'd probably be ideal to secondarily sort based on doc ID so
we have more reproducibility. WDYT?
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322105169
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -213,7 +214,7 @@ private void prepareCombinedResponseBuilder(
combinedQueryResult.setSegmentTerminatedEarly(segmentTerminatedEarly);
combinedQueryResult.setMaxHitsTerminatedEarly(setMaxHitsTerminatedEarly);
crb.setResult(combinedQueryResult);
-if (rb.isDebug()) {
+if (rb.isDebugQuery()) {
String[] queryKeys =
rb.req.getParams().getParams(CombinerParams.COMBINER_QUERY);
List queries =
crb.responseBuilders.stream().map(ResponseBuilder::getQuery).toList();
NamedList explanations =
Review Comment:
the explainations are for "isDebugResults". If you want to add the queries,
then _that_ is "isDebugQuery"
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322075842
##
solr/core/src/java/org/apache/solr/handler/component/FacetComponent.java:
##
@@ -82,6 +82,14 @@ public void prepare(ResponseBuilder rb) throws IOException {
rb.setNeedDocSet(true);
rb.doFacets = true;
+ if (rb instanceof CombinedQueryResponseBuilder crb) {
+crb.responseBuilders.forEach(
+thisRb -> {
+ thisRb.setNeedDocSet(true);
+ thisRb.doFacets = true;
+});
+ }
+
Review Comment:
well done
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322678396
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseInt(maxQueries.toString());
+}
+combiners.computeIfAbsent(
+CombinerParams.RECIPROCAL_RANK_FUSION,
+key -> {
+
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on PR #3418: URL: https://github.com/apache/solr/pull/3418#issuecomment-3254540627 > The beauty/wisdom of BaseDistributedSearchTestCase is that it tests consistency between single shard and multi-shard. I think it's brilliant; that is the point of this base class. Yet this PR/approach will not be able to comply since unlike most (all?) components, its results are affected substantially by distributed-search. The (unsaid?) vision of sharding / distributed-search was getting the same results as a single shard, and Solr does the work to pull off that trick, with plenty of tests demonstrating it does. In fact I'd say, with great disappointment, that the observed (by a user) results of this component will _not_ be RRF when there's distributed search over shards. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322683455
##
solr/core/src/java/org/apache/solr/search/combine/QueryAndResponseCombiner.java:
##
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.combine;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.handler.component.ShardDoc;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.util.plugin.NamedListInitializedPlugin;
+
+/**
+ * The QueryAndResponseCombiner class is an abstract base class for combining
query results and
+ * shard documents. It provides a framework for different algorithms to be
implemented for merging
+ * ranked lists and shard documents.
+ */
+public abstract class QueryAndResponseCombiner implements
NamedListInitializedPlugin {
+ /**
+ * Combines multiple ranked lists into a single QueryResult.
+ *
+ * @param rankedLists a list of ranked lists to be combined
+ * @param solrParams params to be used when provided at query time
+ * @return a new QueryResult containing the combined results
+ * @throws IllegalArgumentException if the input list is empty
+ */
+ public abstract QueryResult combine(List rankedLists,
SolrParams solrParams);
+
+ /**
+ * Combines shard documents based on the provided map.
+ *
+ * @param shardDocMap a map where keys represent shard IDs and values are
lists of ShardDocs for
+ * each shard
+ * @param solrParams params to be used when provided at query time
+ * @return a combined list of ShardDocs from all shards
+ */
+ public abstract List combine(
+ Map> shardDocMap, SolrParams solrParams);
+
+ /**
+ * Retrieves a list of explanations for the given queries and results.
+ *
+ * @param queryKeys the keys associated with the queries
+ * @param queries the list of queries for which explanations are requested
+ * @param queryResult the list of QueryResult corresponding to each query
+ * @param searcher the SolrIndexSearcher used to perform the search
+ * @param schema the IndexSchema used to interpret the search results
+ * @param solrParams params to be used when provided at query time
+ * @return a list of explanations for the given queries and results
+ * @throws IOException if an I/O error occurs during the explanation
retrieval process
+ */
+ public abstract NamedList getExplanations(
Review Comment:
I'm suggest insisting that in the interface. In Solr I think we want to get
away from NamedList usages that are not SimpleOrderedMap. Non-SOM is only for
when keys may repeat.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2321569807
##
solr/core/src/java/org/apache/solr/search/combine/ReciprocalRankFusion.java:
##
@@ -0,0 +1,262 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.combine;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TotalHits;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.component.ShardDoc;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.DocSet;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrDocumentFetcher;
+import org.apache.solr.search.SolrIndexSearcher;
+
+/**
+ * This class implements a query and response combiner that uses the
Reciprocal Rank Fusion (RRF)
+ * algorithm to combine multiple ranked lists into a single ranked list.
+ */
+public class ReciprocalRankFusion extends QueryAndResponseCombiner {
+
+ private int k;
+
+ public ReciprocalRankFusion() {
+this.k = CombinerParams.DEFAULT_COMBINER_RRF_K;
+ }
+
+ @Override
+ public void init(NamedList args) {
+Object kParam = args.get("k");
+if (kParam != null) {
+ this.k = Integer.parseInt(kParam.toString());
+}
+ }
+
+ public int getK() {
+return k;
+ }
+
+ @Override
+ public QueryResult combine(List rankedLists, SolrParams
solrParams) {
+int kVal = solrParams.getInt(CombinerParams.COMBINER_RRF_K, this.k);
+List docListAndSet =
getDocListsAndSetFromQueryResults(rankedLists);
+QueryResult combinedResult = new QueryResult();
+combineResults(combinedResult, docListAndSet, false, kVal);
+return combinedResult;
+ }
+
+ private static List getDocListsAndSetFromQueryResults(
+ List rankedLists) {
+List docLists = new ArrayList<>(rankedLists.size());
+for (QueryResult rankedList : rankedLists) {
+ docLists.add(rankedList.getDocListAndSet());
+}
+return docLists;
+ }
+
+ @Override
+ public List combine(Map> shardDocMap,
SolrParams solrParams) {
Review Comment:
added java-docs. Yes, it would be interleaved in this case as the each shard
has unique docs.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322185934
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseInt(maxQueries.toString());
+}
+combiners.computeIfAbsent(
+CombinerParams.RECIPROCAL_RANK_FUSION,
+key -> {
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322185934
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseInt(maxQueries.toString());
+}
+combiners.computeIfAbsent(
+CombinerParams.RECIPROCAL_RANK_FUSION,
+key -> {
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2322081128
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,598 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+for (Map.Entry initEntry : initParams) {
+ if ("combiners".equals(initEntry.getKey())
+ && initEntry.getValue() instanceof NamedList all) {
+for (int i = 0; i < all.size(); i++) {
+ String name = all.getName(i);
+ NamedList combinerConfig = (NamedList) all.getVal(i);
+ String className = (String) combinerConfig.get("class");
+ QueryAndResponseCombiner combiner =
+ core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+ combiner.init(combinerConfig);
+ combiners.compute(
+ name,
+ (k, existingCombiner) -> {
+if (existingCombiner == null) {
+ return combiner;
+}
+throw new SolrException(
+SolrException.ErrorCode.BAD_REQUEST,
+"Found more than one combiner with same name");
+ });
+}
+ }
+}
+Object maxQueries = initParams.get("maxCombinerQueries");
+if (maxQueries != null) {
+ this.maxCombinerQueries = Integer.parseInt(maxQueries.toString());
+}
+combiners.computeIfAbsent(
+CombinerParams.RECIPROCAL_RANK_FUSION,
+key -> {
+
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2321606729 ## solr/core/src/test/org/apache/solr/handler/component/CombinedQueryComponentTest.java: ## @@ -0,0 +1,251 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.handler.component; + +import static org.apache.solr.common.params.CursorMarkParams.CURSOR_MARK_START; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.apache.solr.SolrTestCaseJ4; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.params.CommonParams; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * The CombinedQueryComponentTest class is a unit test suite for the CombinedQueryComponent in Solr. + * It verifies the functionality of the component by performing various queries and validating the + * responses. + */ +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) Review Comment: Somehow tests were failing and I see this as a fix referring other tests. But, have removed now and see if test fails. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2321569807
##
solr/core/src/java/org/apache/solr/search/combine/ReciprocalRankFusion.java:
##
@@ -0,0 +1,262 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.combine;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TotalHits;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.component.ShardDoc;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.DocSet;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrDocumentFetcher;
+import org.apache.solr.search.SolrIndexSearcher;
+
+/**
+ * This class implements a query and response combiner that uses the
Reciprocal Rank Fusion (RRF)
+ * algorithm to combine multiple ranked lists into a single ranked list.
+ */
+public class ReciprocalRankFusion extends QueryAndResponseCombiner {
+
+ private int k;
+
+ public ReciprocalRankFusion() {
+this.k = CombinerParams.DEFAULT_COMBINER_RRF_K;
+ }
+
+ @Override
+ public void init(NamedList args) {
+Object kParam = args.get("k");
+if (kParam != null) {
+ this.k = Integer.parseInt(kParam.toString());
+}
+ }
+
+ public int getK() {
+return k;
+ }
+
+ @Override
+ public QueryResult combine(List rankedLists, SolrParams
solrParams) {
+int kVal = solrParams.getInt(CombinerParams.COMBINER_RRF_K, this.k);
+List docListAndSet =
getDocListsAndSetFromQueryResults(rankedLists);
+QueryResult combinedResult = new QueryResult();
+combineResults(combinedResult, docListAndSet, false, kVal);
+return combinedResult;
+ }
+
+ private static List getDocListsAndSetFromQueryResults(
+ List rankedLists) {
+List docLists = new ArrayList<>(rankedLists.size());
+for (QueryResult rankedList : rankedLists) {
+ docLists.add(rankedList.getDocListAndSet());
+}
+return docLists;
+ }
+
+ @Override
+ public List combine(Map> shardDocMap,
SolrParams solrParams) {
Review Comment:
Yes, it would be interleaved in this case as the each shard has unique docs.
##
solr/core/src/java/org/apache/solr/search/combine/QueryAndResponseCombiner.java:
##
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.combine;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.handler.component.ShardDoc;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.QueryResult;
+
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2319184435 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -0,0 +1,112 @@ += JSON Combined Query DSL +:tabs-sync-option: +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The Combined Query feature aims to execute multiple queries of multiple kinds across multiple shards of a collection and combine their result basis an algorithm (like Reciprocal Rank Fusion). +It is extending JSON Query DSL ultimately enabling Hybrid Search. + +[NOTE] + +This feature is currently unsupported for grouping and Cursors. + + +== Query DSL Structure +The query structure is similar to JSON Query DSL except for how multiple queries are defined along with their parameters. + +* Multiple queries can be defined under the `queries` key by providing their name with the same syntax as a single query is defined with the key `query`. +* In addition to the other supported parameters, there are several parameters which can be defined under `params` key as below: +`combiner` | Default: `false`:: + Enables the combined query mode when set to `true`. +`combiner.query`:: + The list of queries to be executed as defined in the `queries` key. Example: `["query1", "query2"]` +`combiner.algorithm` | Default: `rrf`:: + The algorithm to be used for combining the results. Reciprocal Rank Fusion (RRF) is the in-built fusion algorithm. + Any other algorithm can be configured using xref:json-combined-query-dsl.adoc#combiner-algorithm-plugin[plugin]. +`combiner.rrf.k` | Default: `60`:: + The k parameter in the RRF algorithm. + +=== Example + +Below is a sample JSON query payload: Review Comment: > and we can adjust the response to indicate the inexactness if more were underlying results were present. True, it's already being marked as part of this [code](https://github.com/apache/solr/pull/3418/files#diff-9ee2d3ca53340b37b597cf2666844ce73ea534416ead953dddc15c5399458282R173) `numFoundExact ` would be false. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2319184435 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -0,0 +1,112 @@ += JSON Combined Query DSL +:tabs-sync-option: +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The Combined Query feature aims to execute multiple queries of multiple kinds across multiple shards of a collection and combine their result basis an algorithm (like Reciprocal Rank Fusion). +It is extending JSON Query DSL ultimately enabling Hybrid Search. + +[NOTE] + +This feature is currently unsupported for grouping and Cursors. + + +== Query DSL Structure +The query structure is similar to JSON Query DSL except for how multiple queries are defined along with their parameters. + +* Multiple queries can be defined under the `queries` key by providing their name with the same syntax as a single query is defined with the key `query`. +* In addition to the other supported parameters, there are several parameters which can be defined under `params` key as below: +`combiner` | Default: `false`:: + Enables the combined query mode when set to `true`. +`combiner.query`:: + The list of queries to be executed as defined in the `queries` key. Example: `["query1", "query2"]` +`combiner.algorithm` | Default: `rrf`:: + The algorithm to be used for combining the results. Reciprocal Rank Fusion (RRF) is the in-built fusion algorithm. + Any other algorithm can be configured using xref:json-combined-query-dsl.adoc#combiner-algorithm-plugin[plugin]. +`combiner.rrf.k` | Default: `60`:: + The k parameter in the RRF algorithm. + +=== Example + +Below is a sample JSON query payload: Review Comment: > and we can adjust the response to indicate the inexactness if more were underlying results were present. True, it's already being marked as part of this [code](https://github.com/apache/solr/pull/3418/files#diff-9ee2d3ca53340b37b597cf2666844ce73ea534416ead953dddc15c5399458282R173) `numExactCount` would be false. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2319184435 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -0,0 +1,112 @@ += JSON Combined Query DSL +:tabs-sync-option: +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The Combined Query feature aims to execute multiple queries of multiple kinds across multiple shards of a collection and combine their result basis an algorithm (like Reciprocal Rank Fusion). +It is extending JSON Query DSL ultimately enabling Hybrid Search. + +[NOTE] + +This feature is currently unsupported for grouping and Cursors. + + +== Query DSL Structure +The query structure is similar to JSON Query DSL except for how multiple queries are defined along with their parameters. + +* Multiple queries can be defined under the `queries` key by providing their name with the same syntax as a single query is defined with the key `query`. +* In addition to the other supported parameters, there are several parameters which can be defined under `params` key as below: +`combiner` | Default: `false`:: + Enables the combined query mode when set to `true`. +`combiner.query`:: + The list of queries to be executed as defined in the `queries` key. Example: `["query1", "query2"]` +`combiner.algorithm` | Default: `rrf`:: + The algorithm to be used for combining the results. Reciprocal Rank Fusion (RRF) is the in-built fusion algorithm. + Any other algorithm can be configured using xref:json-combined-query-dsl.adoc#combiner-algorithm-plugin[plugin]. +`combiner.rrf.k` | Default: `60`:: + The k parameter in the RRF algorithm. + +=== Example + +Below is a sample JSON query payload: Review Comment: > and we can adjust the response to indicate the inexactness if more were underlying results were present. True, it's already being marked as part of this [code](https://github.com/apache/solr/pull/3418/files#diff-9ee2d3ca53340b37b597cf2666844ce73ea534416ead953dddc15c5399458282R173) -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r231912
##
solr/core/src/java/org/apache/solr/handler/component/FacetComponent.java:
##
@@ -82,6 +82,14 @@ public void prepare(ResponseBuilder rb) throws IOException {
rb.setNeedDocSet(true);
rb.doFacets = true;
+ if (rb instanceof CombinedQueryResponseBuilder crb) {
+crb.responseBuilders.forEach(
+thisRb -> {
+ thisRb.setNeedDocSet(true);
+ thisRb.doFacets = true;
+});
+ }
+
Review Comment:
Please see a method created `propagate` to take care of this.
- Please see a test for highlighter where we need to provide a query in hl.
- We are combining the docSet using union which would help coombining the
facets.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
cpoerschke commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2319049507
##
solr/core/src/java/org/apache/solr/handler/component/FacetComponent.java:
##
@@ -82,6 +82,14 @@ public void prepare(ResponseBuilder rb) throws IOException {
rb.setNeedDocSet(true);
rb.doFacets = true;
+ if (rb instanceof CombinedQueryResponseBuilder crb) {
+crb.responseBuilders.forEach(
+thisRb -> {
+ thisRb.setNeedDocSet(true);
+ thisRb.doFacets = true;
+});
+ }
+
Review Comment:
> ... Maybe CombinedQueryComponent could at some point after the other
components to their preparation, it could propagate settings from parent to
children using new code to do so on CombinedQueryResponseBuilder. ...
I had a similar thought.
Thinking specifically about the faceting component though, I'm curious how
that interacts with combined queries:
* in the case of the highlighting component, if we have (say) `title:sales`
and `title:report` as the two lexical queries being combined then we wish for
the matching term to be highlighted in each document.
* in the case of the faceting component, faceting by (say) some language id
field that the document has, how would the facet counts from the two queries be
combined?
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
cpoerschke commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2318978519
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,572 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private Map combiners = new
ConcurrentHashMap<>();
+ private int maxCombinerQueries;
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+if (initParams != null && initParams.size() > 0) {
+ log.info("Initializing CombinedQueryComponent");
+ NamedList all = (NamedList) initParams.get("combiners");
+ for (int i = 0; i < all.size(); i++) {
+String name = all.getName(i);
+NamedList combinerConfig = (NamedList) all.getVal(i);
+String className = (String) combinerConfig.get("class");
+QueryAndResponseCombiner combiner =
+core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+combiner.init(combinerConfig);
+combiners.computeIfAbsent(name, combinerName -> combiner);
+ }
+ Object maxQueries = initParams.get("maxCombinerQueries");
+ if (maxQueries != null) {
+this.maxCombinerQueries = Integer.parseInt(maxQueries.toString());
+ }
+}
+combiners.computeIfAbsent(
+CombinerParams.RECIPROCAL_RANK_FUSION,
+key -> {
+ ReciprocalRankFusion reciprocalRankFusion = new
ReciprocalRankFusion();
+ reciprocalRankFusion.init(initParams);
+ return reciprocalRankFusion;
+});
+ }
+
+ /**
+ * Overrides the prepare method to handle combined queries.
+ *
+ * @param rb the ResponseBuilder to prepare
+
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2318929806
##
solr/core/src/java/org/apache/solr/handler/component/FacetComponent.java:
##
@@ -82,6 +82,14 @@ public void prepare(ResponseBuilder rb) throws IOException {
rb.setNeedDocSet(true);
rb.doFacets = true;
+ if (rb instanceof CombinedQueryResponseBuilder crb) {
+crb.responseBuilders.forEach(
+thisRb -> {
+ thisRb.setNeedDocSet(true);
+ thisRb.doFacets = true;
+});
+ }
+
Review Comment:
This is unfortunate.
Could `CombinedQueryResponseBuilder.setNeedDocSet` be overridden to
propagate to children? That leaves `doFacets`; maybe a setter cold be added
similarly.
OR,
Maybe CombinedQueryComponent could at some point after the other components
to their preparation, it could propagate settings from parent to children using
new code to do so on CombinedQueryResponseBuilder.
I recognize ResponseBuilder is a sloppy mess of entanglement. I don't think
it has ever been cleaned up and re-thought out.
##
solr/core/src/java/org/apache/solr/handler/component/FacetComponent.java:
##
@@ -82,6 +82,14 @@ public void prepare(ResponseBuilder rb) throws IOException {
rb.setNeedDocSet(true);
rb.doFacets = true;
+ if (rb instanceof CombinedQueryResponseBuilder crb) {
+crb.responseBuilders.forEach(
+thisRb -> {
+ thisRb.setNeedDocSet(true);
+ thisRb.doFacets = true;
+});
+ }
+
Review Comment:
I could imagine ResponseBuilder having a method like `copyTo(ResponseBuilder
other)`. Albeit certain fields relate to the query indirectly, and that's what
varies across the queries you are combining. The CombinedQueryResponseBuilder
could arrange for saving those, and reinstating them after copying them.
##
solr/core/src/java/org/apache/solr/handler/component/HighlightComponent.java:
##
@@ -97,6 +97,13 @@ public void prepare(ResponseBuilder rb) throws IOException {
rb.doHighlights = solrConfigHighlighter.isHighlightingEnabled(params);
if (rb.doHighlights) {
rb.setNeedDocList(true);
+ if (rb instanceof CombinedQueryResponseBuilder crb) {
Review Comment:
same feedback as for facets
##
solr/core/src/java/org/apache/solr/handler/component/HighlightComponent.java:
##
@@ -97,6 +97,13 @@ public void prepare(ResponseBuilder rb) throws IOException {
rb.doHighlights = solrConfigHighlighter.isHighlightingEnabled(params);
if (rb.doHighlights) {
rb.setNeedDocList(true);
+ if (rb instanceof CombinedQueryResponseBuilder crb) {
Review Comment:
And there are other components; for some reason these two alone have your
attention but a number of them use the ResponseBuilder.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2318914312
##
solr/core/src/test/org/apache/solr/search/combine/ReciprocalRankFusionTest.java:
##
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.combine;
+
+import static org.apache.solr.common.params.CombinerParams.COMBINER_RRF_K;
+import static
org.apache.solr.common.params.CombinerParams.RECIPROCAL_RANK_FUSION;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.search.TotalHits;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.handler.component.ShardDoc;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * The ReciprocalRankFusionTest class is a unit test suite for the {@link
ReciprocalRankFusion}
+ * class. It verifies the correctness of the fusion algorithm and its
supporting methods.
+ */
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public class ReciprocalRankFusionTest extends SolrTestCaseJ4 {
Review Comment:
I'm looking at that and I only see the DocList being examined.
ReciprocalRankFusionTest, being a unit test, ought to also test what it did
with DocSets since it combines the QueryResult, which is the combo of both.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2318895113 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -0,0 +1,112 @@ += JSON Combined Query DSL +:tabs-sync-option: +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The Combined Query feature aims to execute multiple queries of multiple kinds across multiple shards of a collection and combine their result basis an algorithm (like Reciprocal Rank Fusion). +It is extending JSON Query DSL ultimately enabling Hybrid Search. + +[NOTE] + +This feature is currently unsupported for grouping and Cursors. + + +== Query DSL Structure +The query structure is similar to JSON Query DSL except for how multiple queries are defined along with their parameters. + +* Multiple queries can be defined under the `queries` key by providing their name with the same syntax as a single query is defined with the key `query`. +* In addition to the other supported parameters, there are several parameters which can be defined under `params` key as below: +`combiner` | Default: `false`:: + Enables the combined query mode when set to `true`. +`combiner.query`:: + The list of queries to be executed as defined in the `queries` key. Example: `["query1", "query2"]` +`combiner.algorithm` | Default: `rrf`:: + The algorithm to be used for combining the results. Reciprocal Rank Fusion (RRF) is the in-built fusion algorithm. + Any other algorithm can be configured using xref:json-combined-query-dsl.adoc#combiner-algorithm-plugin[plugin]. +`combiner.rrf.k` | Default: `60`:: + The k parameter in the RRF algorithm. + +=== Example + +Below is a sample JSON query payload: Review Comment: I think this feature should work with the [minExactCount](https://solr.apache.org/guide/solr/latest/query-guide/common-query-parameters.html#minexactcount-parameter) param in this regard. If it's fully exact, then docSets need to be requested and we simply get the cardinality (size) of the final docSet. When docSets are not enabled, then we're only capable of accurately counting up to "rows", and we can adjust the response to indicate the inexactness if more were underlying results were present. > This is also true for a single query which are spread over multiple shards. No; a given doc (identified by its ID) may not be in more than one shard. If a user using implicit routing were to arrange for that to happen; it's _unsupported_ (the user _should not_ do that). There's some trivial dedupe logic you saw but there's no real support for it. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
cpoerschke commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r231403
##
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##
@@ -905,7 +906,84 @@ protected boolean addFL(StringBuilder fl, String field,
boolean additionalAdded)
return true;
}
+ protected abstract static class ShardDocQueue {
+public abstract boolean push(ShardDoc shardDoc);
+
+public abstract Map resultIds(int offset);
+ }
+ ;
+
+ protected static class ShardDocQueueFactory
+ implements BiFunction {
+
+private final SolrIndexSearcher searcher;
+
+public ShardDocQueueFactory(SolrIndexSearcher searcher) {
+ this.searcher = searcher;
+}
+
+@Override
+public ShardDocQueue apply(SortField[] sortFields, Integer size) {
+ return new ShardDocQueue() {
+
+// id to shard mapping, to eliminate any accidental dups
+private final HashMap uniqueDoc = new HashMap<>();
+
+private final ShardFieldSortedHitQueue queue =
+new ShardFieldSortedHitQueue(sortFields, size, searcher);
+
+@Override
+public boolean push(ShardDoc shardDoc) {
+ final String prevShard = uniqueDoc.put(shardDoc.id, shardDoc.shard);
+ if (prevShard != null) {
+// duplicate detected
+
+// For now, just always use the first encountered since we can't
currently
+// remove the previous one added to the priority queue. If we
switched
+// to the Java5 PriorityQueue, this would be easier.
+return false;
+// make which duplicate is used deterministic based on shard
+// if (prevShard.compareTo(shardDoc.shard) >= 0) {
+// TODO: remove previous from priority queue
+// return false;
+// }
+ }
+
+ queue.insertWithOverflow(shardDoc);
+ return true;
+}
+
+@Override
+public Map resultIds(int offset) {
+ final Map resultIds = new HashMap<>();
+
+ // The queue now has 0 -> queuesize docs, where queuesize <= start +
rows
+ // So we want to pop the last documents off the queue to get
+ // the docs offset -> queuesize
+ int resultSize = queue.size() - offset;
+ resultSize = Math.max(0, resultSize); // there may not be any docs
in range
+
+ for (int i = resultSize - 1; i >= 0; i--) {
+ShardDoc shardDoc = queue.pop();
+shardDoc.positionInResponse = i;
+// Need the toString() for correlation with other lists that must
+// be strings (like keys in highlighting, explain, etc)
+resultIds.put(shardDoc.id.toString(), shardDoc);
+ }
+
+ return resultIds;
+}
+ };
+}
+ }
+ ;
+
protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) {
+implementMergeIds(rb, sreq, new
ShardDocQueueFactory(rb.req.getSearcher()));
+ }
+
+ private void implementMergeIds(
Review Comment:
> > though in that case no need for a factory perhaps:
>
> Simple!
https://github.com/apache/solr/pull/3418/commits/a52dd22072b98a91d6e7d065c4df843e340549cd
for that.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2317761872
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,590 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+if (initParams != null && initParams.size() > 0) {
+ for (Map.Entry initEntry : initParams) {
+if ("combiners".equals(initEntry.getKey())
+&& initEntry.getValue() instanceof NamedList all) {
+ for (int i = 0; i < all.size(); i++) {
+String name = all.getName(i);
+NamedList combinerConfig = (NamedList) all.getVal(i);
+String className = (String) combinerConfig.get("class");
+QueryAndResponseCombiner combiner =
+core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+combiner.init(combinerConfig);
+combiners.compute(
+name,
+(k, existingCombiner) -> {
+ if (existingCombiner == null) {
+return combiner;
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Found more than one combiner with same name");
+});
+ }
+}
+ }
+ Object maxQueries = initParams.get("maxCombinerQueries");
+ if (maxQueries != null) {
+this.maxCombinerQueries = Integer.parseInt(maxQueries.toString()
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2318852609 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -0,0 +1,112 @@ += JSON Combined Query DSL +:tabs-sync-option: +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The Combined Query feature aims to execute multiple queries of multiple kinds across multiple shards of a collection and combine their result basis an algorithm (like Reciprocal Rank Fusion). +It is extending JSON Query DSL ultimately enabling Hybrid Search. + +[NOTE] + +This feature is currently unsupported for grouping and Cursors. + + +== Query DSL Structure +The query structure is similar to JSON Query DSL except for how multiple queries are defined along with their parameters. + +* Multiple queries can be defined under the `queries` key by providing their name with the same syntax as a single query is defined with the key `query`. +* In addition to the other supported parameters, there are several parameters which can be defined under `params` key as below: +`combiner` | Default: `false`:: + Enables the combined query mode when set to `true`. +`combiner.query`:: + The list of queries to be executed as defined in the `queries` key. Example: `["query1", "query2"]` +`combiner.algorithm` | Default: `rrf`:: + The algorithm to be used for combining the results. Reciprocal Rank Fusion (RRF) is the in-built fusion algorithm. + Any other algorithm can be configured using xref:json-combined-query-dsl.adoc#combiner-algorithm-plugin[plugin]. +`combiner.rrf.k` | Default: `60`:: + The k parameter in the RRF algorithm. + +=== Example + +Below is a sample JSON query payload: Review Comment: This is also true for a single query which are spread over multiple shards. But, yes we can add this as clarification. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
cpoerschke commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2318785260 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -0,0 +1,112 @@ += JSON Combined Query DSL +:tabs-sync-option: +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The Combined Query feature aims to execute multiple queries of multiple kinds across multiple shards of a collection and combine their result basis an algorithm (like Reciprocal Rank Fusion). +It is extending JSON Query DSL ultimately enabling Hybrid Search. + +[NOTE] + +This feature is currently unsupported for grouping and Cursors. + + +== Query DSL Structure +The query structure is similar to JSON Query DSL except for how multiple queries are defined along with their parameters. + +* Multiple queries can be defined under the `queries` key by providing their name with the same syntax as a single query is defined with the key `query`. +* In addition to the other supported parameters, there are several parameters which can be defined under `params` key as below: +`combiner` | Default: `false`:: + Enables the combined query mode when set to `true`. +`combiner.query`:: + The list of queries to be executed as defined in the `queries` key. Example: `["query1", "query2"]` +`combiner.algorithm` | Default: `rrf`:: + The algorithm to be used for combining the results. Reciprocal Rank Fusion (RRF) is the in-built fusion algorithm. + Any other algorithm can be configured using xref:json-combined-query-dsl.adoc#combiner-algorithm-plugin[plugin]. +`combiner.rrf.k` | Default: `60`:: + The k parameter in the RRF algorithm. + +=== Example + +Below is a sample JSON query payload: Review Comment: My understanding is that * if (say) `lexical1` is for `title:sales` and there are (say) `numFound=500` results and * if (say) `lexical2` is for `title:report` and there are (say) `numFound=300` results and * we combine the two queries and use (say) `rows=10` and * there are (say) 100 documents in common between the 500 and 300 * then we don't know what is in common i.e. we can only observe the overlap of (say) 3 documents amongst the 10 that each of `lexical1` and `lexical2` returned. * So then `500 + 300 - 3` would be the calculation whereas `500 + 300 - 100` is the precise number if one were to interpret it as _"number of documents that matched either or both queries"_ or something along those lines. This is based on code reading/interpretation only so far i.e. haven't yet tried it out locally and/or via a test. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2318605879
##
solr/core/src/test/org/apache/solr/search/combine/ReciprocalRankFusionTest.java:
##
@@ -0,0 +1,138 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.combine;
+
+import static org.apache.solr.common.params.CombinerParams.COMBINER_RRF_K;
+import static
org.apache.solr.common.params.CombinerParams.RECIPROCAL_RANK_FUSION;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.lucene.search.TotalHits;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.handler.component.ShardDoc;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * The ReciprocalRankFusionTest class is a unit test suite for the {@link
ReciprocalRankFusion}
+ * class. It verifies the correctness of the fusion algorithm and its
supporting methods.
+ */
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public class ReciprocalRankFusionTest extends SolrTestCaseJ4 {
Review Comment:
Isn't it being tested as part of
[this](https://github.com/apache/solr/pull/3418/files#diff-9f3a36d6ae6c7df2f303bdd1ec89f6fdc2d7187ee215ce08072ef277048b311cR64).
Do you want me to test the DocSet combining explicitly?
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2318603576
##
solr/core/src/java/org/apache/solr/search/combine/ReciprocalRankFusion.java:
##
@@ -0,0 +1,252 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search.combine;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.StringJoiner;
+import org.apache.lucene.document.Document;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.TotalHits;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.component.ShardDoc;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.DocIterator;
+import org.apache.solr.search.DocList;
+import org.apache.solr.search.DocSlice;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrDocumentFetcher;
+import org.apache.solr.search.SolrIndexSearcher;
+import org.apache.solr.search.SortedIntDocSet;
+
+/**
+ * This class implements a query and response combiner that uses the
Reciprocal Rank Fusion (RRF)
+ * algorithm to combine multiple ranked lists into a single ranked list.
+ */
+public class ReciprocalRankFusion extends QueryAndResponseCombiner {
+
+ private int k;
+
+ public ReciprocalRankFusion() {
+this.k = CombinerParams.DEFAULT_COMBINER_RRF_K;
+ }
+
+ @Override
+ public void init(NamedList args) {
+Object kParam = args.get("k");
+if (kParam != null) {
+ this.k = Integer.parseInt(kParam.toString());
+}
+ }
+
+ public int getK() {
+return k;
+ }
+
+ @Override
+ public QueryResult combine(List rankedLists, SolrParams
solrParams) {
+int kVal = solrParams.getInt(CombinerParams.COMBINER_RRF_K, this.k);
+List docLists = getDocListsFromQueryResults(rankedLists);
+QueryResult combinedResult = new QueryResult();
+combineResults(combinedResult, docLists, false, kVal);
Review Comment:
Thanks for this insight. I changed the method to accomodate above proposed
fix.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2318601809
##
solr/core/src/test/org/apache/solr/handler/component/DistributedCombinedQueryComponentTest.java:
##
@@ -0,0 +1,272 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.params.CommonParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * The DistributedCombinedQueryComponentTest class is a JUnit test suite that
evaluates the
+ * functionality of the CombinedQueryComponent in a Solr distributed search
environment. It focuses
+ * on testing the integration of lexical and vector queries using the combiner
component.
+ */
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public class DistributedCombinedQueryComponentTest extends
BaseDistributedSearchTestCase {
+
+ private static final int NUM_DOCS = 10;
Review Comment:
I added a
[testcase](https://github.com/apache/solr/pull/3418/files#diff-d18c0ebaeee21519f08c727c3202b4bc8d47a592e886102b07dc99af0a0b48b4R243)
to assert this behaviour.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2315324958 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -0,0 +1,113 @@ += JSON Combined Query DSL +:tabs-sync-option: +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The Combined Query feature aims to execute multiple queries of multiple kinds across multiple shards of a collection and combine their result basis an algorithm (like Reciprocal Rank Fusion). +It is extending JSON Query DSL ultimately enabling Hybrid Search. Review Comment: Yes, it's only using JSON Query Parser as the `queries` key in the request has to be in a format that can be parsed using a JSON parser. I dont' see any way to parse multiple Solr Queries of different type using different parsers like lucene, dismax etc. Also, it's a long text ideal to be passed as JSON. [Here](https://github.com/apache/solr/pull/3418/files#diff-1ca21c13d967412ea6227fe257616a9cb2ac45012b207373a526d3767a46b588R145) the parsed query basis the QueryParser are used directly which is not possible without JSON Query DSL. -- 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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2317761758 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -0,0 +1,112 @@ += JSON Combined Query DSL +:tabs-sync-option: +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +The Combined Query feature aims to execute multiple queries of multiple kinds across multiple shards of a collection and combine their result basis an algorithm (like Reciprocal Rank Fusion). +It is extending JSON Query DSL ultimately enabling Hybrid Search. + +[NOTE] + +This feature is currently unsupported for grouping and Cursors. + + +== Query DSL Structure +The query structure is similar to JSON Query DSL except for how multiple queries are defined along with their parameters. + +* Multiple queries can be defined under the `queries` key by providing their name with the same syntax as a single query is defined with the key `query`. +* In addition to the other supported parameters, there are several parameters which can be defined under `params` key as below: +`combiner` | Default: `false`:: + Enables the combined query mode when set to `true`. +`combiner.query`:: + The list of queries to be executed as defined in the `queries` key. Example: `["query1", "query2"]` +`combiner.algorithm` | Default: `rrf`:: + The algorithm to be used for combining the results. Reciprocal Rank Fusion (RRF) is the in-built fusion algorithm. + Any other algorithm can be configured using xref:json-combined-query-dsl.adoc#combiner-algorithm-plugin[plugin]. +`combiner.rrf.k` | Default: `60`:: + The k parameter in the RRF algorithm. + +=== Example + +Below is a sample JSON query payload: Review Comment: Response is standard and same as any other query. Don't think we need to illustrate any response. Is there any change in how `numFound` calculated earlier? If yes, it makes sense. ## solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java: ## @@ -0,0 +1,590 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.handler.component; + +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import org.apache.lucene.search.Explanation; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.Sort; +import org.apache.lucene.search.SortField; +import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.common.SolrDocument; +import org.apache.solr.common.SolrDocumentList; +import org.apache.solr.common.SolrException; +import org.apache.solr.common.params.CombinerParams; +import org.apache.solr.common.params.CursorMarkParams; +import org.apache.solr.common.params.GroupParams; +import org.apache.solr.common.params.ShardParams; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.NamedList; +import org.apache.solr.common.util.SimpleOrderedMap; +import org.apache.solr.common.util.StrUtils; +import org.apache.solr.core.SolrCore; +import org.apache.solr.response.BasicResultContext; +import org.apache.solr.response.ResultContext; +import org.apache.so
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2315036487
##
solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc:
##
@@ -0,0 +1,113 @@
+= JSON Combined Query DSL
+:tabs-sync-option:
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+The Combined Query feature aims to execute multiple queries of multiple kinds
across multiple shards of a collection and combine their result basis an
algorithm (like Reciprocal Rank Fusion).
+It is extending JSON Query DSL ultimately enabling Hybrid Search.
+
+[NOTE]
+
+This feature is currently unsupported for grouping and Cursors. In
User-Managed (aka Standalone) Mode, the `shards` parameter must be provided to
enable this feature.
+
+
+== Query DSL Structure
+The query structure is similar to JSON Query DSL except for how multiple
queries are defined along with their parameters.
+
+* Multiple queries can be defined under the `queries` key by providing their
name with the same syntax as a single query is defined with the key `query`.
+* In addition to the other supported parameters, there are several parameters
which can be defined under `params` key as below:
+`combiner` | Default: `false`::
+ Enables the combined query mode when set to `true`.
+`combiner.query`::
+ The list of queries to be executed as defined in the `queries` key.
Example: `["query1", "query2"]`
+`combiner.algorithm` | Default: `rrf`::
+ The algorithm to be used for combining the results. Reciprocal Rank Fusion
(RRF) is the in-built fusion algorithm.
+ Any other algorithm can be configured using
xref:json-combined-query-dsl.adoc#combiner-algorithm-plugin[plugin].
+`combiner.rrf.k` | Default: `60`::
+ The k parameter in the RRF algorithm.
+
+=== Example
+
+Below is a sample JSON query payload:
+
+```
+{
+"queries": {
+"lexical1": {
+"lucene": {
+"query": "title:sales"
+}
+},
+"lexical2": {
+"lucene": {
+"query": "title:report"
+}
+},
+"vector": {
+"knn": {
+"f": "vector",
+"topK" :5,
+"query": "[0.1,-0.34,0.89,0.02]"
+}
+}
+},
+"limit": 5,
+"fields": ["id", "score", "title"],
+"params": {
+"combiner": true,
+"combiner.query": ["lexical1", "vector"],
+"combiner.algorithm": "rrf",
+"combiner.rrf.k": "15"
+}
+}
+```
+
+== Search Handler Configuration
+
+Combined Query Feature has a separate handler with class
`solr.CombinedQuerySearchHandler` which can be configured as below:
+
+```
+
+true
Review Comment:
It's not needed. removed.
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
cpoerschke commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2313937099
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,572 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private Map combiners = new
ConcurrentHashMap<>();
+ private int maxCombinerQueries;
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+if (initParams != null && initParams.size() > 0) {
+ log.info("Initializing CombinedQueryComponent");
+ NamedList all = (NamedList) initParams.get("combiners");
+ for (int i = 0; i < all.size(); i++) {
+String name = all.getName(i);
+NamedList combinerConfig = (NamedList) all.getVal(i);
+String className = (String) combinerConfig.get("class");
+QueryAndResponseCombiner combiner =
+core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+combiner.init(combinerConfig);
+combiners.computeIfAbsent(name, combinerName -> combiner);
+ }
+ Object maxQueries = initParams.get("maxCombinerQueries");
+ if (maxQueries != null) {
+this.maxCombinerQueries = Integer.parseInt(maxQueries.toString());
+ }
+}
+combiners.computeIfAbsent(
+CombinerParams.RECIPROCAL_RANK_FUSION,
+key -> {
+ ReciprocalRankFusion reciprocalRankFusion = new
ReciprocalRankFusion();
+ reciprocalRankFusion.init(initParams);
+ return reciprocalRankFusion;
+});
+ }
+
+ /**
+ * Overrides the prepare method to handle combined queries.
+ *
+ * @param rb the ResponseBuilder to prepare
+
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
cpoerschke commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2316063996
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,572 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private Map combiners = new
ConcurrentHashMap<>();
+ private int maxCombinerQueries;
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+if (initParams != null && initParams.size() > 0) {
+ log.info("Initializing CombinedQueryComponent");
+ NamedList all = (NamedList) initParams.get("combiners");
+ for (int i = 0; i < all.size(); i++) {
+String name = all.getName(i);
+NamedList combinerConfig = (NamedList) all.getVal(i);
+String className = (String) combinerConfig.get("class");
+QueryAndResponseCombiner combiner =
+core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+combiner.init(combinerConfig);
+combiners.computeIfAbsent(name, combinerName -> combiner);
+ }
+ Object maxQueries = initParams.get("maxCombinerQueries");
+ if (maxQueries != null) {
+this.maxCombinerQueries = Integer.parseInt(maxQueries.toString());
+ }
+}
+combiners.computeIfAbsent(
+CombinerParams.RECIPROCAL_RANK_FUSION,
+key -> {
+ ReciprocalRankFusion reciprocalRankFusion = new
ReciprocalRankFusion();
+ reciprocalRankFusion.init(initParams);
+ return reciprocalRankFusion;
+});
+ }
+
+ /**
+ * Overrides the prepare method to handle combined queries.
+ *
+ * @param rb the ResponseBuilder to prepare
+
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2315316958
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,590 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+if (initParams != null && initParams.size() > 0) {
+ for (Map.Entry initEntry : initParams) {
+if ("combiners".equals(initEntry.getKey())
+&& initEntry.getValue() instanceof NamedList all) {
+ for (int i = 0; i < all.size(); i++) {
+String name = all.getName(i);
+NamedList combinerConfig = (NamedList) all.getVal(i);
+String className = (String) combinerConfig.get("class");
+QueryAndResponseCombiner combiner =
+core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+combiner.init(combinerConfig);
+combiners.compute(
+name,
+(k, existingCombiner) -> {
+ if (existingCombiner == null) {
+return combiner;
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Found more than one combiner with same name");
+});
+ }
+}
+ }
+ Object maxQueries = initParams.get("maxCombinerQueries");
+ if (maxQueries != null) {
+this.maxCombinerQueries = Integer.parseInt(maxQueries.toString()
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
dsmiley commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2317770682
##
solr/core/src/java/org/apache/solr/handler/component/QueryComponent.java:
##
@@ -905,7 +906,84 @@ protected boolean addFL(StringBuilder fl, String field,
boolean additionalAdded)
return true;
}
+ protected abstract static class ShardDocQueue {
+public abstract boolean push(ShardDoc shardDoc);
+
+public abstract Map resultIds(int offset);
+ }
+ ;
+
+ protected static class ShardDocQueueFactory
+ implements BiFunction {
+
+private final SolrIndexSearcher searcher;
+
+public ShardDocQueueFactory(SolrIndexSearcher searcher) {
+ this.searcher = searcher;
+}
+
+@Override
+public ShardDocQueue apply(SortField[] sortFields, Integer size) {
+ return new ShardDocQueue() {
+
+// id to shard mapping, to eliminate any accidental dups
+private final HashMap uniqueDoc = new HashMap<>();
+
+private final ShardFieldSortedHitQueue queue =
+new ShardFieldSortedHitQueue(sortFields, size, searcher);
+
+@Override
+public boolean push(ShardDoc shardDoc) {
+ final String prevShard = uniqueDoc.put(shardDoc.id, shardDoc.shard);
+ if (prevShard != null) {
+// duplicate detected
+
+// For now, just always use the first encountered since we can't
currently
+// remove the previous one added to the priority queue. If we
switched
+// to the Java5 PriorityQueue, this would be easier.
+return false;
+// make which duplicate is used deterministic based on shard
+// if (prevShard.compareTo(shardDoc.shard) >= 0) {
+// TODO: remove previous from priority queue
+// return false;
+// }
+ }
+
+ queue.insertWithOverflow(shardDoc);
+ return true;
+}
+
+@Override
+public Map resultIds(int offset) {
+ final Map resultIds = new HashMap<>();
+
+ // The queue now has 0 -> queuesize docs, where queuesize <= start +
rows
+ // So we want to pop the last documents off the queue to get
+ // the docs offset -> queuesize
+ int resultSize = queue.size() - offset;
+ resultSize = Math.max(0, resultSize); // there may not be any docs
in range
+
+ for (int i = resultSize - 1; i >= 0; i--) {
+ShardDoc shardDoc = queue.pop();
+shardDoc.positionInResponse = i;
+// Need the toString() for correlation with other lists that must
+// be strings (like keys in highlighting, explain, etc)
+resultIds.put(shardDoc.id.toString(), shardDoc);
+ }
+
+ return resultIds;
+}
+ };
+}
+ }
+ ;
+
protected void mergeIds(ResponseBuilder rb, ShardRequest sreq) {
+implementMergeIds(rb, sreq, new
ShardDocQueueFactory(rb.req.getSearcher()));
+ }
+
+ private void implementMergeIds(
Review Comment:
> though in that case no need for a factory perhaps:
Simple!
--
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-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2315946946
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,590 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+if (initParams != null && initParams.size() > 0) {
+ for (Map.Entry initEntry : initParams) {
+if ("combiners".equals(initEntry.getKey())
+&& initEntry.getValue() instanceof NamedList all) {
+ for (int i = 0; i < all.size(); i++) {
+String name = all.getName(i);
+NamedList combinerConfig = (NamedList) all.getVal(i);
+String className = (String) combinerConfig.get("class");
+QueryAndResponseCombiner combiner =
+core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+combiner.init(combinerConfig);
+combiners.compute(
+name,
+(k, existingCombiner) -> {
+ if (existingCombiner == null) {
+return combiner;
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Found more than one combiner with same name");
+});
+ }
+}
+ }
+ Object maxQueries = initParams.get("maxCombinerQueries");
+ if (maxQueries != null) {
+this.maxCombinerQueries = Integer.parseInt(maxQueries.toString()
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418:
URL: https://github.com/apache/solr/pull/3418#discussion_r2315942209
##
solr/core/src/java/org/apache/solr/handler/component/CombinedQueryComponent.java:
##
@@ -0,0 +1,590 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.component;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.lucene.search.Explanation;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.SortField;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.SolrDocument;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CombinerParams;
+import org.apache.solr.common.params.CursorMarkParams;
+import org.apache.solr.common.params.GroupParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.response.BasicResultContext;
+import org.apache.solr.response.ResultContext;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SchemaField;
+import org.apache.solr.search.DocListAndSet;
+import org.apache.solr.search.QueryResult;
+import org.apache.solr.search.SolrReturnFields;
+import org.apache.solr.search.SortSpec;
+import org.apache.solr.search.combine.QueryAndResponseCombiner;
+import org.apache.solr.search.combine.ReciprocalRankFusion;
+import org.apache.solr.util.SolrResponseUtil;
+import org.apache.solr.util.plugin.SolrCoreAware;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * The CombinedQueryComponent class extends QueryComponent and provides
support for executing
+ * multiple queries and combining their results.
+ */
+public class CombinedQueryComponent extends QueryComponent implements
SolrCoreAware {
+
+ private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ public static final String COMPONENT_NAME = "combined_query";
+ protected NamedList initParams;
+ private final Map combiners = new
HashMap<>();
+ private int maxCombinerQueries;
+
+ @Override
+ public void init(NamedList args) {
+super.init(args);
+this.initParams = args;
+this.maxCombinerQueries = CombinerParams.DEFAULT_MAX_COMBINER_QUERIES;
+ }
+
+ @Override
+ public void inform(SolrCore core) {
+if (initParams != null && initParams.size() > 0) {
+ for (Map.Entry initEntry : initParams) {
+if ("combiners".equals(initEntry.getKey())
+&& initEntry.getValue() instanceof NamedList all) {
+ for (int i = 0; i < all.size(); i++) {
+String name = all.getName(i);
+NamedList combinerConfig = (NamedList) all.getVal(i);
+String className = (String) combinerConfig.get("class");
+QueryAndResponseCombiner combiner =
+core.getResourceLoader().newInstance(className,
QueryAndResponseCombiner.class);
+combiner.init(combinerConfig);
+combiners.compute(
+name,
+(k, existingCombiner) -> {
+ if (existingCombiner == null) {
+return combiner;
+ }
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "Found more than one combiner with same name");
+});
+ }
+}
+ }
+ Object maxQueries = initParams.get("maxCombinerQueries");
+ if (maxQueries != null) {
+this.maxCombinerQueries = Integer.parseInt(maxQueries.toString()
Re: [PR] SOLR-17319 : Combined Query Feature for Multi Query Execution [solr]
ercsonusharma commented on code in PR #3418: URL: https://github.com/apache/solr/pull/3418#discussion_r2315899041 ## solr/solr-ref-guide/modules/query-guide/pages/json-combined-query-dsl.adoc: ## @@ -22,7 +22,7 @@ It is extending JSON Query DSL ultimately enabling Hybrid Search. [NOTE] -This feature is currently unsupported for non-distributed query, grouping and Cursors. +This feature is currently unsupported for grouping and Cursors. In User-Managed (aka Standalone) Mode, the `shards` parameter must be provided to enable this feature. Review Comment: okay. Will remove this statement from docs then. -- 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]
