diff --git a/changelog/unreleased/calcite-sql-params.yml b/changelog/unreleased/calcite-sql-params.yml new file mode 100644 index 000000000000..1635159c1567 --- /dev/null +++ b/changelog/unreleased/calcite-sql-params.yml @@ -0,0 +1,7 @@ +title: Avoid forwarding unrelated request parameters to Calcite from the /sql handler +type: changed +authors: + - name: Jan Høydahl +links: + - name: PR#4607 + url: https://github.com/apache/solr/pull/4607 diff --git a/solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java b/solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java index 1f3cfe6b05d7..2295dc6269a4 100644 --- a/solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java +++ b/solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java @@ -22,10 +22,10 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import org.apache.calcite.config.Lex; import org.apache.solr.client.solrj.io.Tuple; import org.apache.solr.client.solrj.io.comp.StreamComparator; @@ -52,10 +52,11 @@ public class SQLHandler extends RequestHandlerBase private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static String defaultZkhost = null; - private static String defaultWorkerCollection = null; static final String sqlNonCloudErrorMsg = "/sql handler only works in Solr Cloud mode"; + static final Set CONNECTION_PARAMS = Set.of("aggregationMode", "numWorkers"); + private boolean isCloud = false; @Override @@ -64,7 +65,6 @@ public void inform(SolrCore core) { if (coreContainer.isZooKeeperAware()) { defaultZkhost = coreContainer.getZkController().getZkServerAddress(); - defaultWorkerCollection = core.getCoreDescriptor().getCollectionName(); isCloud = true; } @@ -85,8 +85,6 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw String sql = params.get("stmt"); // Set defaults for parameters params.set("numWorkers", params.getInt("numWorkers", 1)); - params.set("workerCollection", params.get("workerCollection", defaultWorkerCollection)); - params.set("workerZkhost", params.get("workerZkhost", defaultZkhost)); params.set("aggregationMode", params.get("aggregationMode", "facet")); TupleStream tupleStream = null; @@ -103,11 +101,12 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw String url = CalciteSolrDriver.CONNECT_STRING_PREFIX; Properties properties = new Properties(); - // Add all query parameters - Iterator parameterNamesIterator = params.getParameterNamesIterator(); - while (parameterNamesIterator.hasNext()) { - String param = parameterNamesIterator.next(); - properties.setProperty(param, params.get(param)); + // Forward only the parameters the handler supports as connection configuration. + for (String param : CONNECTION_PARAMS) { + String value = params.get(param); + if (value != null) { + properties.setProperty(param, value); + } } // Set these last to ensure that they are set properly diff --git a/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java b/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java index 94e997998ea1..131476c3031e 100644 --- a/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java +++ b/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java @@ -452,6 +452,24 @@ public void testBasicSelect() throws Exception { tuples = getTuples(sParams, baseUrl); assertEquals(0, tuples.size()); + + // Verify that we only forward parameters we care about + sParams = + params( + CommonParams.QT, + "/sql", + "typeSystem", + "does.not.Exist", + "stmt", + "select id, field_i, str_s from collection1 where text_t='XXXX' order by field_i desc limit 1"); + + tuples = getTuples(sParams, baseUrl); + + assertEquals(1, tuples.size()); + tuple = tuples.get(0); + assertEquals(8, tuple.getLong("id").longValue()); + assertEquals(60, tuple.getLong("field_i").longValue()); + assertEquals("c", tuple.get("str_s")); } @Test