Repository navigation
feat: reject unknown query context parameters - #20647
FrankChen021 wants to merge 3 commits into
Conversation
Add druid.query.context.rejectUnknownParameters, off by default. When it is on, the Broker rejects queries that set a query context parameter that is not declared in QueryContextParameters, and suggests parameters with similar names. The Broker also validates the values of the parameters a query sets before running it: the top-level context of native queries, the context and SET statements of SQL queries, and the connection properties of JDBC connections when they open. Default contexts are validated where they are configured: at Broker startup for druid.query.default.context and druid.msq.dart.query.context, and by the Coordinator for Broker dynamic configuration updates.
FrankChen021
left a comment
There was a problem hiding this comment.
🟡 Changes recommended
Validate native gRPC query contexts in extensions-contrib/grpc-query/src/main/java/org/apache/druid/grpc/server/QueryDriver.java:326. The gRPC module runs on the Broker, but rejectUnknownParameters is applied only to SQL translation. The native runNativeQuery path deserializes the query and initializes its lifecycle without calling QueryContextParameterValidator, so native gRPC requests can bypass unknown-parameter and invalid-value validation when the feature is enabled. Validate the native query context before lifecycle initialization and return the corresponding request error.
Reviewed 73 of 73 changed files; finding in 1 file.
Validation: git diff --check passed. No build or tests were run; this was a static review.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 0 |
| P2 | 1 |
| P3 | 0 |
| Total | 1 |
This is an automated review by Codex GPT-5.6-Luna(max)
After addressing the findings or replying to the comments, you can request another review from me to trigger a new automated review.
FrankChen021
left a comment
There was a problem hiding this comment.
🟢 Approval recommended
No actionable findings. The previous native-gRPC context-validation finding is addressed in the current head.
Reviewed 73 of 73 changed files.
Validation: git diff --check passed. No build or tests were run; this was a static review.
This is an automated review by Codex GPT-5.6-Luna(max)
Description
Background
Since #20578 and #20579, every query context parameter is declared in
QueryContextParameters. Druid still silently ignores a parameter it doesn't recognize, such as a misspelleduseCahce, and only checks a parameter's value when it first reads it, so an invalid value can go unnoticed or fail late.New configuration and behavior
druid.query.context.rejectUnknownParametersfalseSETstatements of SQL queries.rejectUnknownParametersistrue, with suggestions for similar names:Unknown query context parameter [useCahce]. Did you mean [useCache]?query/time/sqlQuery/time, like a query that can't be parsed.druid.query.default.context.*ordruid.msq.dart.query.context.*is invalid, and the Coordinator rejects a Broker dynamic configuration update with an invalidqueryContext.Upgrade note
JDBC connections now fail to open if a connection property, other than
userandpassword, sets a query context parameter to an invalid value; for examplesqlTimeZone=Not/AZone. Previously such a property failed only when a query read it, or was ignored if no query did. This matches how databases such as MySQL and PostgreSQL reject invalid session settings at connect. Ifdruid.query.context.rejectUnknownParametersistrue, connections with a connection property that Druid doesn't recognize also fail to open. Check the connection properties of your JDBC clients before upgrading.Release note
New property
druid.query.context.rejectUnknownParameters, off by default: whentrue, Druid rejects queries that set a query context parameter it doesn't recognize, and suggests similar parameter names. Druid also checks the values of query context parameters before running a query, and the default query contexts where they're configured.This PR has: