Skip to content

Commit 9742a30

Browse files
committed
Close combined-case gap: backtick AND default-escape char in same name
A column named e.g. `col`x.y` (literal backtick + dot) failed to round-trip because the two escape mechanisms interacted incorrectly: 1. Our override doubles the backtick: name -> `col``x.y` 2. Super's bindname_escape_characters then translates the dot: name -> `col``x_y`, escaped_from -> `col``x.y` 3. escaped_bind_names records {col``x.y: col``x_y} 4. bind_names[bindparam] is the original `col`x.y` (single backtick, set by _truncate_bindparam before our override runs) 5. construct_params looks up `col`x.y` in escaped_bind_names — misses (the key is doubled), so dict key stays `col`x.y` 6. Server parses marker name `col`x_y` (dot translated, backtick un-doubled), looks up `col`x_y` in the dict — UNBOUND. Fix: when our override doubles backticks, render the marker fully ourselves rather than delegating to super. Keeps escaped_bind_names empty for these names so construct_params passes the original key through unchanged. Super's escape map can no longer interact with our doubled name. The non-backtick path (e.g. plain `col.x`) is unchanged and still goes through super. Adds a unit test and an end-to-end audit case for `col`x.y`. Audit is now 43/43 (added 2 new assertions for the combined case). Vanishingly rare in practice but it's the last realistic edge case I could identify; closing it removes the only known correctness gap. Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 2c1bc55 commit 9742a30

3 files changed

Lines changed: 653 additions & 4 deletions

File tree

src/databricks/sqlalchemy/_ddl.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,31 @@ def bindparam_string(self, name, **kw):
138138
# ``BACKQUOTED_IDENTIFIER`` lexer rule. The doubling affects only
139139
# the rendered SQL; the params dict key sent to the driver stays
140140
# the single-backtick original (the server collapses `` -> `
141-
# when it parses the marker name), so we must not set
142-
# ``escaped_from`` — leaving ``escaped_bind_names`` empty keeps
143-
# the key translation in ``construct_params`` a no-op.
141+
# when it parses the marker name).
142+
#
143+
# When a backtick is present, render the marker ourselves rather
144+
# than delegating to super. Super would otherwise also apply
145+
# ``bindname_escape_characters`` translation (``.``->``_``,
146+
# ``[``->``_``, etc.) AND set ``escaped_from``, which together
147+
# would propagate into ``escaped_bind_names`` and rewrite the
148+
# params-dict key. The original dict key uses a single backtick
149+
# and the un-translated form, so the rewrite would create a
150+
# mismatch with what the server expects when it parses the
151+
# backtick-quoted marker name. By owning the rendering here we
152+
# keep ``escaped_bind_names`` empty for these names and the dict
153+
# key passes through unchanged.
144154
if (
145155
"`" in name
146156
and not kw.get("escaped_from")
147157
and not kw.get("post_compile", False)
148158
):
149-
name = name.replace("`", "``")
159+
accumulate = kw.get("accumulate_bind_names")
160+
if accumulate is not None:
161+
accumulate.add(name)
162+
visited = kw.get("visited_bindparam")
163+
if visited is not None:
164+
visited.append(name)
165+
return self._BIND_TEMPLATE % {"name": name.replace("`", "``")}
150166
return super().bindparam_string(name, **kw)
151167

152168
def limit_clause(self, select, **kw):

0 commit comments

Comments
 (0)