Skip to content

fix: OpenSearch dialect identifier delimiters#39953

Merged
Vitor-Avila merged 1 commit into
masterfrom
fix/opensearch-dialect
May 7, 2026
Merged

fix: OpenSearch dialect identifier delimiters#39953
Vitor-Avila merged 1 commit into
masterfrom
fix/opensearch-dialect

Conversation

@Vitor-Avila
Copy link
Copy Markdown
Contributor

@Vitor-Avila Vitor-Avila commented May 7, 2026

SUMMARY

This is a follow up to #39538. While the initial PR solved one issue for un-aggregated tables, further using the dialect exposed a bigger issue. The elasticsearch-dbapi driver was removing any " from the queries, causing issues for metric/column labels like "my metric".

I simplified the sanitize_query method for the elasticsearch-dbapi driver here, so this PR bumps the package version in Superset while also defaulting to backticks as the identifier delimiter (the official one according to OpenSearch/OpenDistro docs). Double quotes are still supported in the dialect since the DBs supports it as well.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

No UI changes.

TESTING INSTRUCTIONS

  1. Create a dataset powered by OpenSearch/OpenDistro.
  2. Create a metric in the dataset using count(*) and my metric as the key.
  3. Create a chart using this metric.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 7, 2026

Code Review Agent Run #1dea05

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 1c07879..1c07879
    • pyproject.toml
    • superset/sql/dialects/opensearch.py
    • tests/unit_tests/sql/dialects/opensearch_tests.py
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • MyPy (Static Code Analysis) - ✔︎ Successful
    • Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot Bot added the data:connect:elasticsearch Related to Elasticsearch label May 7, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented May 7, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 1c07879
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69fcd4eb3446880008d50f1c
😎 Deploy Preview https://deploy-preview-39953--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.87%. Comparing base (5b5dd01) to head (1c07879).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #39953      +/-   ##
==========================================
- Coverage   63.88%   63.87%   -0.01%     
==========================================
  Files        2583     2583              
  Lines      136604   136625      +21     
  Branches    31502    31504       +2     
==========================================
  Hits        87276    87276              
- Misses      47812    47833      +21     
  Partials     1516     1516              
Flag Coverage Δ
hive 39.37% <100.00%> (-0.02%) ⬇️
mysql 59.04% <100.00%> (-0.02%) ⬇️
postgres 59.11% <100.00%> (-0.02%) ⬇️
presto 41.07% <100.00%> (-0.02%) ⬇️
python 60.55% <100.00%> (-0.03%) ⬇️
sqlite 58.75% <100.00%> (-0.02%) ⬇️
unit 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Vitor-Avila Vitor-Avila merged commit ad5e317 into master May 7, 2026
70 of 71 checks passed
@Vitor-Avila Vitor-Avila deleted the fix/opensearch-dialect branch May 7, 2026 19:19
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data:connect:elasticsearch Related to Elasticsearch size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants