Skip to content

Escape aliases in metrics view search fallback query (value is reserved in Druid SQL) - #9891

Merged
pjain1 merged 5 commits into
mainfrom
nishant/fix-druid-dimension-search-alias
Sep 17, 2026
Merged

pjain1 merged 5 commits into
mainfrom
nishant/fix-druid-dimension-search-alias

Conversation

@nishantmonu51

@nishantmonu51 nishantmonu51 commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator
  • The global dimension search (dimension_search feature flag) fails on Druid with Incorrect syntax near the keyword 'AS'. The SQL fallback for Executor.Search wraps each per-dimension query as SELECT '<dim>' AS dimension, "<dim>" AS value FROM (...), and VALUE is a reserved keyword in Druid's Calcite parser. The error columns reported in the thread (52 and 60) match the position of AS value for the account_name and line_item_alt_id dimensions exactly.
  • The fallback runs whenever the native Druid search cannot be used (filter not translatable to a native filter, missing time range, or unnested dimension), so it has to work on Druid too.
  • Escape both aliases via the dialect so Druid gets AS "dimension" and AS "value". No change in behavior for DuckDB or ClickHouse, which do not reserve value.
  • Adds executor_search_test.go, the first test covering the search fallback path (multiple dimensions, with and without a where clause and time range). Note it runs on DuckDB and so cannot reproduce the Druid keyword failure itself.

Closes APP-935

Slack: https://rilldata.slack.com/archives/C02T907FEUB/p1789483677280479

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@nishantmonu51 nishantmonu51 added Type:Bug Something isn't working Area:Druid Size:M Medium change: 100-499 lines labels Sep 16, 2026
@pjain1
pjain1 requested a balanced review from Copilot September 16, 2026 13:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Add a Druid-dialect regression assertion for fallback alias escaping.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread runtime/metricsview/executor/executor.go
@pjain1
pjain1 merged commit 49355b2 into main Sep 17, 2026
13 checks passed
@pjain1
pjain1 deleted the nishant/fix-druid-dimension-search-alias branch September 17, 2026 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area:Druid Size:M Medium change: 100-499 lines Type:Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants