From e89773e19a85cdce0121f16bb2105602b0023ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Sat, 4 Jul 2026 01:08:18 +0200 Subject: [PATCH 1/4] Avoid forwarding unrelated request parameters to Calcite from the /sql handler --- changelog/unreleased/calcite-sql-params.yml | 7 +++++++ .../apache/solr/handler/sql/SQLHandler.java | 15 +++++++++------ .../solr/handler/sql/TestSQLHandler.java | 18 ++++++++++++++++++ 3 files changed, 34 insertions(+), 6 deletions(-) create mode 100644 changelog/unreleased/calcite-sql-params.yml diff --git a/changelog/unreleased/calcite-sql-params.yml b/changelog/unreleased/calcite-sql-params.yml new file mode 100644 index 000000000000..3f35c0a2a9fa --- /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: PRXXXX + url: https://github.com/apache/solr/pull/XXXX 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..51847676e3fb 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; @@ -56,6 +56,8 @@ public class SQLHandler extends RequestHandlerBase 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 @@ -103,11 +105,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 From ee97aaf224d7404fd38885d5a9a79dbe0d37638a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Sat, 4 Jul 2026 01:25:40 +0200 Subject: [PATCH 2/4] Put PR number in changelog --- changelog/unreleased/calcite-sql-params.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog/unreleased/calcite-sql-params.yml b/changelog/unreleased/calcite-sql-params.yml index 3f35c0a2a9fa..1635159c1567 100644 --- a/changelog/unreleased/calcite-sql-params.yml +++ b/changelog/unreleased/calcite-sql-params.yml @@ -3,5 +3,5 @@ type: changed authors: - name: Jan Høydahl links: - - name: PRXXXX - url: https://github.com/apache/solr/pull/XXXX + - name: PR#4607 + url: https://github.com/apache/solr/pull/4607 From 58127870a790dd86f69caf0afbc316feec7d1df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Mon, 6 Jul 2026 22:30:44 +0200 Subject: [PATCH 3/4] Remove dead code.defaultWorkerCollection, workerZkhost, workerCollection --- .../sql/src/java/org/apache/solr/handler/sql/SQLHandler.java | 4 ---- 1 file changed, 4 deletions(-) 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 51847676e3fb..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 @@ -52,7 +52,6 @@ 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"; @@ -66,7 +65,6 @@ public void inform(SolrCore core) { if (coreContainer.isZooKeeperAware()) { defaultZkhost = coreContainer.getZkController().getZkServerAddress(); - defaultWorkerCollection = core.getCoreDescriptor().getCollectionName(); isCloud = true; } @@ -87,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; From e8c2c486e3cfdd38df0fbbb781114d555f88e1b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20H=C3=B8ydahl?= Date: Mon, 6 Jul 2026 23:48:05 +0200 Subject: [PATCH 4/4] Make allowlist user configurable and with a larger set of default params --- changelog/unreleased/calcite-sql-params.yml | 2 +- .../apache/solr/handler/sql/SQLHandler.java | 32 ++++++++++++-- .../solr/handler/sql/TestSQLHandler.java | 43 +++++++++++++++++-- .../pages/solr-properties.adoc | 2 + 4 files changed, 72 insertions(+), 7 deletions(-) diff --git a/changelog/unreleased/calcite-sql-params.yml b/changelog/unreleased/calcite-sql-params.yml index 1635159c1567..a178dbe64889 100644 --- a/changelog/unreleased/calcite-sql-params.yml +++ b/changelog/unreleased/calcite-sql-params.yml @@ -1,4 +1,4 @@ -title: Avoid forwarding unrelated request parameters to Calcite from the /sql handler +title: The /sql handler request params forwarded to Calcite is now configurable type: changed authors: - name: Jan Høydahl 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 2295dc6269a4..1aa7ace83efa 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 @@ -34,6 +34,7 @@ import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.EnvUtils; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; import org.apache.solr.handler.RequestHandlerBase; @@ -55,7 +56,23 @@ public class SQLHandler extends RequestHandlerBase static final String sqlNonCloudErrorMsg = "/sql handler only works in Solr Cloud mode"; - static final Set CONNECTION_PARAMS = Set.of("aggregationMode", "numWorkers"); + /** System property to override the set of request parameters forwarded to Calcite. */ + static final String ALLOWED_CONNECTION_PARAMS_PROP = "solr.sql.connection.params.allowed"; + + /** Calcite configuration parameters forwarded as connection properties. */ + static final Set DEFAULT_CONNECTION_PARAMS = + Set.of( + "caseSensitive", + "quoting", + "quotedCasing", + "unquotedCasing", + "conformance", + "fun", + "typeCoercion", + "lenientOperatorLookup", + "defaultNullCollation", + "timeZone", + "locale"); private boolean isCloud = false; @@ -101,14 +118,18 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw String url = CalciteSolrDriver.CONNECT_STRING_PREFIX; Properties properties = new Properties(); - // Forward only the parameters the handler supports as connection configuration. - for (String param : CONNECTION_PARAMS) { + // Forward only the configured Calcite configuration parameters + for (String param : allowedConnectionParams()) { String value = params.get(param); if (value != null) { properties.setProperty(param, value); } } + // Solr-specific parameters + properties.setProperty("aggregationMode", params.get("aggregationMode")); + properties.setProperty("numWorkers", params.get("numWorkers")); + // Set these last to ensure that they are set properly properties.setProperty("lex", Lex.MYSQL.toString()); properties.setProperty("zk", defaultZkhost); @@ -202,6 +223,11 @@ public Tuple read() throws IOException { } } + static Set allowedConnectionParams() { + List override = EnvUtils.getPropertyAsList(ALLOWED_CONNECTION_PARAMS_PROP); + return override != null ? Set.copyOf(override) : DEFAULT_CONNECTION_PARAMS; + } + private ModifiableSolrParams adjustParams(SolrParams params) { ModifiableSolrParams adjustedParams = new ModifiableSolrParams(); adjustedParams.add(params); 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 131476c3031e..7cd3a2aa1219 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 @@ -453,13 +453,13 @@ public void testBasicSelect() throws Exception { assertEquals(0, tuples.size()); - // Verify that we only forward parameters we care about + // Verify that unrelated request parameters are ignored sParams = params( CommonParams.QT, "/sql", - "typeSystem", - "does.not.Exist", + "someOtherParam", + "someValue", "stmt", "select id, field_i, str_s from collection1 where text_t='XXXX' order by field_i desc limit 1"); @@ -472,6 +472,43 @@ public void testBasicSelect() throws Exception { assertEquals("c", tuple.get("str_s")); } + @Test + public void testConnectionParamsAllowlistOverride() throws Exception { + + new UpdateRequest() + .add("id", "1", "text_t", "XXXX XXXX", "str_s", "a", "field_i", "7") + .commit(cluster.getSolrClient(), COLLECTIONORALIAS); + + String baseUrl = + cluster.getJettySolrRunners().get(0).getBaseUrl().toString() + "/" + COLLECTIONORALIAS; + + SolrParams sParams = + params( + CommonParams.QT, + "/sql", + "caseSensitive", + "true", + "stmt", + "select id, FIELD_I from collection1 limit 1"); + + // caseSensitive is forwarded to Calcite by default, making column names case-sensitive, + // unlike the default MySQL dialect behavior + SolrStream solrStream = new SolrStream(baseUrl, sParams); + Tuple tuple = getTuple(new ExceptionStream(solrStream)); + assertTrue(tuple.EOF); + assertTrue(tuple.EXCEPTION); + assertTrue(tuple.getException().contains("Column 'FIELD_I' not found")); + + // Restrict the allowlist so that caseSensitive is no longer forwarded and the same query + // succeeds with case-insensitive column resolution + System.setProperty(SQLHandler.ALLOWED_CONNECTION_PARAMS_PROP, ""); + try { + assertEquals(1, getTuples(sParams, baseUrl).size()); + } finally { + System.clearProperty(SQLHandler.ALLOWED_CONNECTION_PARAMS_PROP); + } + } + @Test public void testWhere() throws Exception { diff --git a/solr/solr-ref-guide/modules/configuration-guide/pages/solr-properties.adoc b/solr/solr-ref-guide/modules/configuration-guide/pages/solr-properties.adoc index b9ecaddb1002..20fbfc09fb3a 100644 --- a/solr/solr-ref-guide/modules/configuration-guide/pages/solr-properties.adoc +++ b/solr/solr-ref-guide/modules/configuration-guide/pages/solr-properties.adoc @@ -110,6 +110,8 @@ NOTE: Properties marked with "!" indicate inverted meaning between pre Solr 10 a |solr.solrj.http.jetty.customizer|solr.httpclient.builder.factory||A class loaded to customize HttpJettySolrClient upon creation. +|solr.sql.connection.params.allowed||caseSensitive, quoting, quotedCasing, unquotedCasing, conformance, fun,typeCoercion, lenientOperatorLookup, defaultNullCollation, timeZone, locale|A comma separated list of request parameters to forward to Apache Calcite as connection properties. + |solr.streamingexpressions.facet.tiered.enabled|solr.facet.stream.tiered|true|Controls whether tiered faceting is enabled for streaming expressions. |solr.streamingexpressions.macros.enabled|StreamingExpressionMacros|false|Controls whether to expand URL parameters inside of the `expr` parameter.