Vitor-Avila commented on code in PR #34991:
URL: https://github.com/apache/superset/pull/34991#discussion_r2317135390
##########
superset/db_engine_specs/databricks.py:
##########
@@ -72,19 +72,42 @@ def process(value: Any) -> str:
def monkeypatch_dialect() -> None:
"""
- Monkeypatch dialect to correctly escape single quotes.
-
- The Databricks SQLAlchemy dialect we currently use does not escape single
quotes
- correctly -- it doubles the single quotes, instead of adding a backslash.
The fixed
- version requires SQLAlchemy 2.0, which is not yet available in Superset.
+ Monkeypatch dialect to correctly escape single quotes for Databricks.
+
+ The Databricks SQLAlchemy dialect (<3.0) incorrectly escapes single quotes
by
+ doubling them ('O''Hara') instead of using backslash escaping ('O\'Hara').
The
+ fixed version requires SQLAlchemy>=2.0, which is not yet compatible with
Superset.
+
+ The challenge: DatabricksDialect inherits from HiveDialect and accesses the
+ `colspecs` type mapping through its parent class rather than its own. This
means:
+ - Direct assignment (`DatabricksDialect.colspecs[types.String] = ...`)
works because
+ DatabricksDialect.colspecs points to the same dict object as
HiveDialect.colspecs,
+ but then it affects all other dialects that inherit from HiveDialect,
causing
+ incorrect escaping for them.
+ - Shallow copying then modifying `DatabricksDialect.colspecs` doesn't work
+ because it's never accessed.
+
+ The solution: A context-aware approach by creating a custom String type
+ that checks which dialect is being used during SQL compilation, allowing
+ to apply Databricks-specific escaping only for Databricks connections.
Review Comment:
Thanks! done
##########
superset/db_engine_specs/databricks.py:
##########
@@ -72,19 +72,42 @@ def process(value: Any) -> str:
def monkeypatch_dialect() -> None:
"""
- Monkeypatch dialect to correctly escape single quotes.
-
- The Databricks SQLAlchemy dialect we currently use does not escape single
quotes
- correctly -- it doubles the single quotes, instead of adding a backslash.
The fixed
- version requires SQLAlchemy 2.0, which is not yet available in Superset.
+ Monkeypatch dialect to correctly escape single quotes for Databricks.
+
+ The Databricks SQLAlchemy dialect (<3.0) incorrectly escapes single quotes
by
+ doubling them ('O''Hara') instead of using backslash escaping ('O\'Hara').
The
+ fixed version requires SQLAlchemy>=2.0, which is not yet compatible with
Superset.
+
+ The challenge: DatabricksDialect inherits from HiveDialect and accesses the
+ `colspecs` type mapping through its parent class rather than its own. This
means:
+ - Direct assignment (`DatabricksDialect.colspecs[types.String] = ...`)
works because
+ DatabricksDialect.colspecs points to the same dict object as
HiveDialect.colspecs,
+ but then it affects all other dialects that inherit from HiveDialect,
causing
+ incorrect escaping for them.
+ - Shallow copying then modifying `DatabricksDialect.colspecs` doesn't work
+ because it's never accessed.
Review Comment:
Done too!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
