diff --git a/changelog/unreleased/calcite-sql-params.yml b/changelog/unreleased/calcite-sql-params.yml new file mode 100644 index 000000000000..a178dbe64889 --- /dev/null +++ b/changelog/unreleased/calcite-sql-params.yml @@ -0,0 +1,7 @@ +title: The /sql handler request params forwarded to Calcite is now configurable +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..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 @@ -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; @@ -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; @@ -52,10 +53,27 @@ 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"; + /** 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; @Override @@ -64,7 +82,6 @@ public void inform(SolrCore core) { if (coreContainer.isZooKeeperAware()) { defaultZkhost = coreContainer.getZkController().getZkServerAddress(); - defaultWorkerCollection = core.getCoreDescriptor().getCollectionName(); isCloud = true; } @@ -85,8 +102,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,13 +118,18 @@ 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 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); @@ -203,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 94e997998ea1..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 @@ -452,6 +452,61 @@ public void testBasicSelect() throws Exception { tuples = getTuples(sParams, baseUrl); assertEquals(0, tuples.size()); + + // Verify that unrelated request parameters are ignored + sParams = + params( + CommonParams.QT, + "/sql", + "someOtherParam", + "someValue", + "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 + 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 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.