Skip to content

SQL: Parse queries immediately for non-JDBC endpoints.#18102

Merged
gianm merged 2 commits intoapache:masterfrom
gianm:fix-set-acd-false
Jun 10, 2025
Merged

SQL: Parse queries immediately for non-JDBC endpoints.#18102
gianm merged 2 commits intoapache:masterfrom
gianm:fix-set-acd-false

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Jun 9, 2025

SET statements (#17894) provide a new way to set query context. However, they are not available early enough to inform important operations like engine selection and planner rule construction. This patch moves parsing as early as possible: immediately on receiving the query from the user.

Behavior change: as a consequence of moving parsing prior to statement creation, SQL queries that cannot be parsed are no longer logged, nor do they have metrics emitted.

@github-actions github-actions Bot added Area - Batch Ingestion Area - Querying Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 labels Jun 9, 2025
SET statements (apache#17894) provide a new way to set query context. However,
they are not available early enough to inform important operations like
engine selection and planner rule construction. This patch moves parsing
as early as possible: immediately on receiving the query from the user.

As a consequence of moving parsing prior to statement creation, with
this change, SQL queries that cannot be parsed are no longer logged.
@gianm gianm force-pushed the fix-set-acd-false branch from 241046d to 1dd50ab Compare June 9, 2025 19:51
Copy link
Copy Markdown
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍

freshCopy feels kind of weird, but I can't really think of a cleaner way to do it, and making a factory thing or whatever seems like overkill, so 🤷

@gianm
Copy link
Copy Markdown
Contributor Author

gianm commented Jun 10, 2025

nice 👍

freshCopy feels kind of weird, but I can't really think of a cleaner way to do it, and making a factory thing or whatever seems like overkill, so 🤷

It is kind of weird, but that's what you get when you have mutable data structures. Btw, I did look for a way to do a deep clone of the SqlNode (rather than re-parsing) but didn't find a good one.

@gianm gianm merged commit 505f97b into apache:master Jun 10, 2025
74 checks passed
@capistrant capistrant added this to the 34.0.0 milestone Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area - Batch Ingestion Area - MSQ For multi stage queries - https://github.com/apache/druid/issues/12262 Area - Querying

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants