Skip to content

fix(databricks): string escaper#34180

Merged
betodealmeida merged 1 commit intomasterfrom
databricks-string-escape
Jul 15, 2025
Merged

fix(databricks): string escaper#34180
betodealmeida merged 1 commit intomasterfrom
databricks-string-escape

Conversation

@betodealmeida
Copy link
Member

@betodealmeida betodealmeida commented Jul 15, 2025

SUMMARY

The Databricks SQLAlchemy dialect we use has a bug when escaping single quotes: it doubles the single quote, instead of using a backslash. For example, a value of O'Hara will be rendered in the SQL as O''Hara, which is correct for most databases, but for Databricks should be O\'Hara.

Upgrading to the latest databricks-sqlalchemy package fixes the issue, but requires SQLAlchemy 2.x. The 1.x line of databricks-sqlalchemy has the bug as well. Note, we're using the unnoficial sqlalchemy-databricks package instead, so we should consider changing to the 1.x version of the official one.

For now, this PR solves the problem by monkeypatching a custom string escaper into the the dialect.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

Screenshot 2025-07-15 at 6 16 23 PM

After:

Screenshot 2025-07-15 at 6 15 11 PM

TESTING INSTRUCTIONS

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

@dosubot dosubot bot added the data:connect:databricks Databricks DB Experience label Jul 15, 2025
@betodealmeida betodealmeida force-pushed the databricks-string-escape branch from 595ec93 to d0f275b Compare July 15, 2025 21:47
@korbit-ai
Copy link

korbit-ai bot commented Jul 15, 2025

I was unable to post the issues I found. This could be because a force push or squash has changed the commit history since I scanned this pull request. You can get another review by commenting /korbit-review.

Copy link
Contributor

@Vitor-Avila Vitor-Avila left a comment

Choose a reason for hiding this comment

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

Thanks for porting the fix!

@betodealmeida betodealmeida merged commit 83b6f67 into master Jul 15, 2025
50 checks passed
@betodealmeida betodealmeida deleted the databricks-string-escape branch July 15, 2025 23:27
ianngech pushed a commit to Pesapal-Ltd/superset that referenced this pull request Jul 17, 2025
dankor pushed a commit to dankor/superset that referenced this pull request Aug 18, 2025
@betodealmeida betodealmeida mentioned this pull request Aug 28, 2025
9 tasks
@github-actions github-actions bot added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 6.0.0 First shipped in 6.0.0 labels Dec 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels data:connect:databricks Databricks DB Experience preset-io size/M 🚢 6.0.0 First shipped in 6.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants