This is an automated email from the ASF dual-hosted git repository.
Jackie-Jiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 3214046a21a feat: reject unknown SQL query options on DQL SET/OPTION
path (#7207) (#19027)
3214046a21a is described below
commit 3214046a21a28729845a9905c2d03acdca98d01b
Author: deepinsight coder <[email protected]>
AuthorDate: Tue Aug 18 12:50:21 2026 -0700
feat: reject unknown SQL query options on DQL SET/OPTION path (#7207)
(#19027)
---
.../requesthandler/BaseBrokerRequestHandler.java | 8 +
.../common/utils/config/QueryOptionsUtils.java | 157 +++++++++++++++
.../apache/pinot/sql/parsers/CalciteSqlParser.java | 13 +-
.../sql/parsers/SqlQueryOptionValidationTest.java | 223 +++++++++++++++++++++
.../apache/pinot/spi/utils/CommonConstants.java | 9 +
5 files changed, 409 insertions(+), 1 deletion(-)
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
index 50ef569cf21..605fcda0ce8 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java
@@ -24,6 +24,7 @@ import com.google.common.collect.Maps;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.OptionalLong;
import java.util.Set;
@@ -49,6 +50,8 @@ import org.apache.pinot.common.metrics.BrokerQueryPhase;
import org.apache.pinot.common.response.BrokerResponse;
import org.apache.pinot.common.response.broker.BrokerResponseNative;
import org.apache.pinot.common.response.broker.QueryProcessingException;
+import org.apache.pinot.common.utils.config.QueryOptionsUtils;
+import
org.apache.pinot.common.utils.config.QueryOptionsUtils.SqlQueryOptionValidationMode;
import org.apache.pinot.common.utils.request.RequestUtils;
import org.apache.pinot.core.auth.Actions;
import org.apache.pinot.core.auth.TargetType;
@@ -130,6 +133,11 @@ public abstract class BaseBrokerRequestHandler implements
BrokerRequestHandler {
Broker.DEFAULT_BROKER_ENABLE_QUERY_CANCELLATION);
_enableAutoRewriteAggregationType =
config.getProperty(Broker.CONFIG_OF_BROKER_QUERY_ENABLE_AUTO_REWRITE_AGGREGATION_TYPE);
+ // Process-wide static because the SQL parser has no access to the broker
config. Set here rather
+ // than parsed per query; a broker restart is needed to pick up a config
change.
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.valueOf(
+
config.getProperty(Broker.CONFIG_OF_BROKER_QUERY_OPTION_VALIDATION_MODE,
+
Broker.DEFAULT_BROKER_QUERY_OPTION_VALIDATION_MODE).trim().toUpperCase(Locale.ROOT)));
if (_enableQueryCancellation) {
_queriesById = new ConcurrentHashMap<>();
_clientQueryIds = new ConcurrentHashMap<>();
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
index 8aeca449733..d4a31b6eac9 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/config/QueryOptionsUtils.java
@@ -27,6 +27,7 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;
import org.apache.commons.lang3.StringUtils;
import org.apache.pinot.spi.config.table.FieldConfig;
@@ -34,6 +35,8 @@ import org.apache.pinot.spi.utils.CommonConstants;
import
org.apache.pinot.spi.utils.CommonConstants.Broker.Request.QueryOptionKey;
import
org.apache.pinot.spi.utils.CommonConstants.MultiStageQueryRunner.JoinOverFlowMode;
import
org.apache.pinot.spi.utils.CommonConstants.MultiStageQueryRunner.WindowOverFlowMode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/// Utils to parse query options.
@@ -41,9 +44,46 @@ public class QueryOptionsUtils {
private QueryOptionsUtils() {
}
+ /// How SQL-supplied query option keys are validated, see
+ /// [CommonConstants.Broker#CONFIG_OF_BROKER_QUERY_OPTION_VALIDATION_MODE].
+ public enum SqlQueryOptionValidationMode {
+ /// Unknown keys are preserved as-is and nothing is logged. Default, and
the only mode that
+ /// leaves the parsed options identical to what Pinot produced before
validation existed.
+ NONE,
+ /// Unknown keys are preserved, but logged once per distinct key with a
typo suggestion.
+ WARN,
+ /// Unknown keys fail the query with a typo suggestion.
+ REJECT
+ }
+
+ private static final Logger LOGGER =
LoggerFactory.getLogger(QueryOptionsUtils.class);
+
private static final Map<String, String> CONFIG_RESOLVER;
private static final RuntimeException CLASS_LOAD_ERROR;
+ /// Lower-case keys accepted in SQL `SET`/`OPTION(...)` that are not
declared on [QueryOptionKey].
+ private static final Set<String> ADDITIONAL_SQL_OPTION_KEYS =
+ Set.of(CommonConstants.Broker.Request.TRACE.toLowerCase(),
CommonConstants.DATABASE.toLowerCase());
+
+ /// Row-level-security options are injected by the broker after parsing and
must never be settable
+ /// from user SQL, not even through [#registerSqlQueryOptionKey]. Matched as
a lower-case prefix so
+ /// case tricks (`RLSFilters-t`) cannot bypass the guard.
+ private static final String RLS_FILTERS_PREFIX =
CommonConstants.RLS_FILTERS.toLowerCase();
+
+ /// Lower-case option keys registered by plugins via
[#registerSqlQueryOptionKey].
+ private static final Set<String> REGISTERED_SQL_OPTION_KEYS =
ConcurrentHashMap.newKeySet();
+
+ /// Distinct unknown keys already logged in
[SqlQueryOptionValidationMode#WARN], so that a
+ /// high-QPS client sending one misspelled option cannot flood the broker
log.
+ private static final Set<String> WARNED_SQL_OPTION_KEYS =
ConcurrentHashMap.newKeySet();
+ private static final int MAX_WARNED_SQL_OPTION_KEYS = 1000;
+
+ /// Longest key for which a "did you mean" suggestion is computed, to bound
the Levenshtein cost.
+ private static final int MAX_SUGGESTION_KEY_LENGTH = 64;
+
+ private static volatile SqlQueryOptionValidationMode
_sqlQueryOptionValidationMode =
+ SqlQueryOptionValidationMode.NONE;
+
static {
// this is a bit hacky, but lots of the code depends directly on usage of
// Map<String, String> (JSON serialization/GRPC code) so we cannot just
@@ -75,6 +115,9 @@ public class QueryOptionsUtils {
: new RuntimeException("Failure to build case insensitive mapping.",
classLoadError);
}
+ /// Resolves known option keys case-insensitively to their canonical names.
Unknown keys are
+ /// preserved unchanged, including their key case. Used for every option
source: SQL, REST/JSON
+ /// `queryOptions` and broker-injected options, none of which reject unknown
keys here.
public static Map<String, String> resolveCaseInsensitiveOptions(Map<String,
String> queryOptions) {
if (CLASS_LOAD_ERROR != null) {
throw CLASS_LOAD_ERROR;
@@ -93,6 +136,120 @@ public class QueryOptionsUtils {
return resolved;
}
+ public static SqlQueryOptionValidationMode getSqlQueryOptionValidationMode()
{
+ return _sqlQueryOptionValidationMode;
+ }
+
+ /// Sets the validation mode applied to SQL-supplied query option keys.
Called once per process at
+ /// broker startup from
[CommonConstants.Broker#CONFIG_OF_BROKER_QUERY_OPTION_VALIDATION_MODE], and
+ /// by tests to restore [SqlQueryOptionValidationMode#NONE].
+ public static void
setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode mode) {
+ _sqlQueryOptionValidationMode = mode;
+ }
+
+ /// Registers an option key that [#validateSqlQueryOptions] accepts in
addition to the keys
+ /// declared on [QueryOptionKey]. Meant for plugins that read custom options
off the
+ /// `BrokerRequest`; call it from plugin init. Case-insensitive, thread safe
and idempotent.
+ public static void registerSqlQueryOptionKey(String key) {
+ REGISTERED_SQL_OPTION_KEYS.add(key.toLowerCase());
+ }
+
+ /// Validates SQL-supplied `SET` / `OPTION(...)` option keys according to
the configured
+ /// [SqlQueryOptionValidationMode]. Only meant for DQL: DML statements carry
free-form task and
+ /// filesystem properties, and REST/JSON `queryOptions` are free-form for
backward compatibility,
+ /// so neither is validated in any mode. Options the broker injects after
parsing are never routed
+ /// here either.
+ ///
+ /// Does not modify or copy the map; in the default `NONE` mode it returns
immediately, so the
+ /// parsed options stay exactly what the parser produced.
+ ///
+ /// @throws IllegalArgumentException in `REJECT` mode if an unsupported
option key is present
+ public static void validateSqlQueryOptions(Map<String, String> queryOptions)
{
+ SqlQueryOptionValidationMode mode = _sqlQueryOptionValidationMode;
+ if (mode == SqlQueryOptionValidationMode.NONE || queryOptions.isEmpty()) {
+ return;
+ }
+ if (CLASS_LOAD_ERROR != null) {
+ throw CLASS_LOAD_ERROR;
+ }
+ for (String key : queryOptions.keySet()) {
+ String lowerKey = key.toLowerCase();
+ if (CONFIG_RESOLVER.containsKey(lowerKey) ||
ADDITIONAL_SQL_OPTION_KEYS.contains(lowerKey)
+ || (REGISTERED_SQL_OPTION_KEYS.contains(lowerKey) &&
!lowerKey.startsWith(RLS_FILTERS_PREFIX))) {
+ continue;
+ }
+ if (mode == SqlQueryOptionValidationMode.REJECT) {
+ throw new IllegalArgumentException(buildUnsupportedOptionMessage(key));
+ }
+ // ponytail: bounded set, never evicted, so a key seen once is never
logged again. Swap for a
+ // rate limiter if the set of distinct unknown keys ever needs to be
unbounded.
+ if (WARNED_SQL_OPTION_KEYS.size() < MAX_WARNED_SQL_OPTION_KEYS &&
WARNED_SQL_OPTION_KEYS.add(lowerKey)) {
+ LOGGER.warn("{} (logged once per distinct key)",
buildUnsupportedOptionMessage(key));
+ }
+ }
+ }
+
+ private static String buildUnsupportedOptionMessage(String key) {
+ String suggestion = findClosestCanonicalOption(key);
+ if (suggestion != null) {
+ return "Unsupported query option '" + key + "'. Did you mean '" +
suggestion + "'?";
+ }
+ return "Unsupported query option '" + key + "'";
+ }
+
+ /// Only called on the unknown-key path, never while parsing a valid query.
+ @Nullable
+ private static String findClosestCanonicalOption(String key) {
+ if (key.length() > MAX_SUGGESTION_KEY_LENGTH) {
+ return null;
+ }
+ String lower = key.toLowerCase();
+ String best = null;
+ int bestDist = Integer.MAX_VALUE;
+ // CONFIG_RESOLVER is keyed by the lower-case name and valued by the
canonical name.
+ for (Map.Entry<String, String> entry : CONFIG_RESOLVER.entrySet()) {
+ int dist = levenshteinDistance(lower, entry.getKey());
+ if (dist < bestDist) {
+ bestDist = dist;
+ best = entry.getValue();
+ }
+ }
+ if (best == null) {
+ return null;
+ }
+ // Suggest only when reasonably close (pure case variants are not unknown
in the first place).
+ int threshold = Math.max(2, best.length() / 4);
+ return bestDist <= threshold ? best : null;
+ }
+
+ private static int levenshteinDistance(String a, String b) {
+ int n = a.length();
+ int m = b.length();
+ if (n == 0) {
+ return m;
+ }
+ if (m == 0) {
+ return n;
+ }
+ int[] prev = new int[m + 1];
+ int[] curr = new int[m + 1];
+ for (int j = 0; j <= m; j++) {
+ prev[j] = j;
+ }
+ for (int i = 1; i <= n; i++) {
+ curr[0] = i;
+ char ca = a.charAt(i - 1);
+ for (int j = 1; j <= m; j++) {
+ int cost = ca == b.charAt(j - 1) ? 0 : 1;
+ curr[j] = Math.min(Math.min(curr[j - 1] + 1, prev[j] + 1), prev[j - 1]
+ cost);
+ }
+ int[] tmp = prev;
+ prev = curr;
+ curr = tmp;
+ }
+ return prev[m];
+ }
+
@Nullable
public static String resolveCaseInsensitiveKey(Object property) {
if (property instanceof String) {
diff --git
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
index d9a4d418395..5a060850385 100644
---
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
+++
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
@@ -133,7 +133,13 @@ public class CalciteSqlParser {
SqlNodeAndOptions sqlNodeAndOptions =
extractSqlNodeAndOptions(sqlNodeList);
// add legacy OPTIONS keyword-based options
if (!options.isEmpty()) {
- sqlNodeAndOptions.setExtraOptions(extractOptionsMap(options));
+ Map<String, String> optionMap = extractOptionsMap(options);
+ if (sqlNodeAndOptions.getSqlType() == PinotSqlType.DQL) {
+ // No-op unless the broker enables query option validation. DML (e.g.
+ // INSERT INTO FILE OPTION(taskName=...)) carries free-form task/FS
properties, like DML SET.
+ QueryOptionsUtils.validateSqlQueryOptions(optionMap);
+ }
+ sqlNodeAndOptions.setExtraOptions(optionMap);
}
sqlNodeAndOptions.setParseTimeNs(System.nanoTime() - parseStartTimeNs);
return sqlNodeAndOptions;
@@ -191,6 +197,11 @@ public class CalciteSqlParser {
if (sqlType == null) {
throw new SqlCompilationException("SqlNode with executable statement not
found!");
}
+ if (sqlType == PinotSqlType.DQL) {
+ // No-op unless the broker enables query option validation. DML (e.g.
INSERT INTO FILE) carries
+ // free-form task/FS properties via SET, and REST/JSON queryOptions
never reach this path.
+ QueryOptionsUtils.validateSqlQueryOptions(options);
+ }
return new SqlNodeAndOptions(statementNode, sqlType,
QueryOptionsUtils.resolveCaseInsensitiveOptions(options));
}
diff --git
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/SqlQueryOptionValidationTest.java
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/SqlQueryOptionValidationTest.java
new file mode 100644
index 00000000000..a69160a31e6
--- /dev/null
+++
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/SqlQueryOptionValidationTest.java
@@ -0,0 +1,223 @@
+/**
+ * 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.pinot.sql.parsers;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.Logger;
+import org.apache.logging.log4j.core.LoggerContext;
+import org.apache.logging.log4j.core.appender.AbstractAppender;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.pinot.common.utils.config.QueryOptionsUtils;
+import
org.apache.pinot.common.utils.config.QueryOptionsUtils.SqlQueryOptionValidationMode;
+import org.testng.annotations.AfterMethod;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
+
+
+/// Covers the opt-in SQL query option validation modes. The pre-existing
default behavior is pinned
+/// by [CalciteSqlCompilerTest] and `QueryOptionsUtilsTest`, which are
deliberately untouched.
+public class SqlQueryOptionValidationTest {
+
+ @AfterMethod
+ public void resetValidationMode() {
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.NONE);
+ }
+
+ @Test
+ public void defaultModeIsNoneAndPreservesUnknownKeysVerbatim() {
+ assertEquals(QueryOptionsUtils.getSqlQueryOptionValidationMode(),
SqlQueryOptionValidationMode.NONE);
+
+ Map<String, String> setOptions = optionsOf("SET deliCious='yes'; select *
from vegetables");
+ assertEquals(setOptions.get("deliCious"), "yes");
+
+ Map<String, String> legacyOptions = optionsOf("select * from vegetables
OPTION(deliCious=yes)");
+ assertEquals(legacyOptions.get("deliCious"), "yes");
+ }
+
+ @Test
+ public void defaultModeKeepsLastWinsForDuplicateKeysInDifferentCases() {
+ Map<String, String> options = optionsOf("SET timeoutMs='1'; SET
TIMEOUTMS='2'; select * from vegetables");
+ // Both spellings resolve to the same canonical key; which value wins is
unspecified (as on master),
+ // but exactly one entry must survive.
+ assertEquals(options.size(), 1);
+ assertTrue(List.of("1", "2").contains(options.get("timeoutMs")),
options.toString());
+ }
+
+ @Test
+ public void rejectModeFailsUnknownSetOptionWithSuggestion() {
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.REJECT);
+
+ SqlCompilationException e = expectThrows(SqlCompilationException.class,
+ () -> optionsOf("SET timoutMs='100'; select * from vegetables"));
+ assertTrue(e.getMessage().contains("Unsupported query option 'timoutMs'"),
e.getMessage());
+ assertTrue(e.getMessage().contains("Did you mean 'timeoutMs'"),
e.getMessage());
+ }
+
+ @Test
+ public void rejectModeFailsUnknownLegacyOption() {
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.REJECT);
+
+ SqlCompilationException e = expectThrows(SqlCompilationException.class,
+ () -> optionsOf("select * from vegetables OPTION(delicious=yes)"));
+ assertTrue(e.getMessage().contains("Unsupported query option
'delicious'"), e.getMessage());
+ }
+
+ @Test
+ public void rejectModeOmitsSuggestionForKeysNowhereNearAKnownOption() {
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.REJECT);
+
+ SqlCompilationException e = expectThrows(SqlCompilationException.class,
+ () -> optionsOf("SET zzz='1'; select * from vegetables"));
+ assertTrue(e.getMessage().contains("Unsupported query option 'zzz'"),
e.getMessage());
+ assertFalse(e.getMessage().contains("Did you mean"), e.getMessage());
+ }
+
+ @Test
+ public void rejectModeAcceptsKnownKeysCaseInsensitivelyAndTraceAndDatabase()
{
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.REJECT);
+
+ Map<String, String> options =
+ optionsOf("SET timeoutMS='100'; SET TRACE='true'; SET Database='db1';
select * from vegetables");
+ assertEquals(options.get("timeoutMs"), "100");
+ assertEquals(options.get("TRACE"), "true");
+ assertEquals(options.get("Database"), "db1");
+ }
+
+ @Test
+ public void rejectModeLeavesDmlOptionsFreeForm() {
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.REJECT);
+
+ // DML carries free-form task and filesystem properties, via both SET and
legacy OPTION.
+ Map<String, String> legacyOptions =
+ optionsOf("INSERT INTO db.tbl FROM FILE 'file:///tmp/file1'
OPTION(taskName=myTask-1)");
+ assertEquals(legacyOptions.get("taskName"), "myTask-1");
+
+ Map<String, String> setOptions =
+ optionsOf("SET taskName='myTask-1'; INSERT INTO db.tbl FROM FILE
'file:///tmp/file1'");
+ assertEquals(setOptions.get("taskName"), "myTask-1");
+ }
+
+ @Test
+ public void rejectModeAcceptsRegisteredPluginKeys() {
+ QueryOptionsUtils.registerSqlQueryOptionKey("myPluginOption");
+ // Plugin init can run more than once; registration must be idempotent.
+ QueryOptionsUtils.registerSqlQueryOptionKey("myPluginOption");
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.REJECT);
+
+ // Registered case-insensitively, and the key case the user typed is
preserved as before.
+ assertEquals(optionsOf("SET MYPLUGINOPTION='x'; select * from
vegetables").get("MYPLUGINOPTION"), "x");
+ }
+
+ @Test
+ public void rejectModeNeverAcceptsUserSuppliedRlsFilters() {
+ // The broker injects rlsFilters* after parsing; registering the key must
not open a back door,
+ // and the prefix match is case-insensitive so case tricks cannot bypass
it either.
+ QueryOptionsUtils.registerSqlQueryOptionKey("rlsFiltersMyTable");
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.REJECT);
+
+ for (String key : List.of("rlsFiltersMyTable", "RLSFILTERSMyTable")) {
+ SqlCompilationException e = expectThrows(SqlCompilationException.class,
+ () -> optionsOf("SET " + key + "='col=1'; select * from
vegetables"));
+ assertTrue(e.getMessage().contains("Unsupported query option '" + key +
"'"), e.getMessage());
+ }
+ }
+
+ @Test
+ public void warnModePreservesUnknownKeysAndLogsOncePerDistinctKey() {
+ CapturingAppender appender =
CapturingAppender.attachTo(QueryOptionsUtils.class.getName());
+ try {
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.WARN);
+
+ // A misspelled option from a high-QPS client must be logged once, not
once per query.
+ for (int i = 0; i < 100; i++) {
+ assertEquals(optionsOf("SET warnOnceOption='v'; select * from
vegetables").get("warnOnceOption"), "v");
+ }
+ assertEquals(appender.messagesContaining("warnOnceOption").size(), 1);
+
+ // A different unknown key still gets its own line.
+ optionsOf("SET anotherWarnOnceOption='v'; select * from vegetables");
+
assertEquals(appender.messagesContaining("anotherWarnOnceOption").size(), 1);
+
+ // Known keys are never logged.
+ int loggedSoFar = appender.messagesContaining("Unsupported query
option").size();
+ optionsOf("SET timeoutMs='100'; select * from vegetables");
+ assertEquals(appender.messagesContaining("Unsupported query
option").size(), loggedSoFar);
+ } finally {
+ appender.detach();
+ }
+ }
+
+ @Test
+ public void restStyleOptionsStayFreeFormInEveryMode() {
+
QueryOptionsUtils.setSqlQueryOptionValidationMode(SqlQueryOptionValidationMode.REJECT);
+
+ // REST/JSON queryOptions and broker-injected options never go through the
SQL parser; they are
+ // merged through resolveCaseInsensitiveOptions, which validates nothing
in any mode.
+ Map<String, String> resolved =
QueryOptionsUtils.resolveCaseInsensitiveOptions(
+ Map.of("customFreeForm", "x", "timeoutMS", "5", "rlsFilters-tbl",
"col=1"));
+ assertEquals(resolved.get("customFreeForm"), "x");
+ assertEquals(resolved.get("timeoutMs"), "5");
+ assertEquals(resolved.get("rlsFilters-tbl"), "col=1");
+ }
+
+ private static Map<String, String> optionsOf(String sql) {
+ return CalciteSqlParser.compileToSqlNodeAndOptions(sql).getOptions();
+ }
+
+ private static final class CapturingAppender extends AbstractAppender {
+ private final List<String> _messages = new CopyOnWriteArrayList<>();
+ private final Logger _logger;
+
+ private CapturingAppender(Logger logger) {
+ super("SqlQueryOptionValidationCapture", null, null, true,
Property.EMPTY_ARRAY);
+ _logger = logger;
+ }
+
+ static CapturingAppender attachTo(String loggerName) {
+ LoggerContext context = (LoggerContext) LogManager.getContext(false);
+ CapturingAppender appender = new
CapturingAppender(context.getLogger(loggerName));
+ appender.start();
+ appender._logger.addAppender(appender);
+ return appender;
+ }
+
+ void detach() {
+ _logger.removeAppender(this);
+ stop();
+ }
+
+ List<String> messagesContaining(String substring) {
+ return _messages.stream().filter(message ->
message.contains(substring)).collect(Collectors.toList());
+ }
+
+ @Override
+ public void append(LogEvent event) {
+ _messages.add(event.getMessage().getFormattedMessage());
+ }
+ }
+}
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
index 3b6a9c195af..7357c7c4be5 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
@@ -383,6 +383,15 @@ public class CommonConstants {
"pinot.broker.query.log.sqlRedaction";
public static final String DEFAULT_BROKER_QUERY_LOG_SQL_REDACTION = "none";
public static final String CONFIG_OF_BROKER_QUERY_ENABLE_NULL_HANDLING =
"pinot.broker.query.enable.null.handling";
+ /// How query option keys supplied through SQL `SET` / `OPTION(...)` on
DQL queries are validated.
+ /// Broker config key: `pinot.broker.query.option.validationMode`.
+ /// One of `QueryOptionsUtils.SqlQueryOptionValidationMode`: `NONE`
(default, unknown keys are
+ /// preserved silently, as they always have been), `WARN` (preserved,
logged once per distinct
+ /// unknown key) or `REJECT` (query fails). Plugins can allowlist their
own keys for `REJECT` via
+ /// `QueryOptionsUtils.registerSqlQueryOptionKey`.
+ public static final String CONFIG_OF_BROKER_QUERY_OPTION_VALIDATION_MODE =
+ "pinot.broker.query.option.validationMode";
+ public static final String DEFAULT_BROKER_QUERY_OPTION_VALIDATION_MODE =
"NONE";
/// When true, the broker initializes the materialized view metadata cache
and query rewrite
/// engine. When false (default), MV rewrite is disabled regardless of
per-MV
/// `rewriteEnabled` setting.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
