Skip to content

Commit 0693e53

Browse files
committed
fix: close oracle round 16 gaps
- Detect unresolved bind parameters in subqueries and raise CompileError instead of silently rendering NULL with literal_binds=True - Handle single-quoted strings in _split_sql_list_quote_aware() - Strip quoted literals before constraint keyword detection in _guard_alter_add_column_constraints() to prevent false positives
1 parent 4afe32a commit 0693e53

5 files changed

Lines changed: 151 additions & 4 deletions

File tree

src/sqlalchemy_excel/compiler.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,10 @@ class ExcelCompiler(compiler.SQLCompiler):
9191
_in_in_clause: bool = False
9292
_subquery_depth: int = 0
9393
_has_join: bool = False
94+
_SUBQUERY_BINDPARAM_ERROR = (
95+
"Excel dialect does not support execution-time bind parameters in "
96+
"subqueries. Use literal values instead."
97+
)
9498

9599
@staticmethod
96100
def _raise_if_schema_qualified(table: Any) -> None:
@@ -284,6 +288,44 @@ def _reject_correlated_subquery(self, inner: Any) -> None:
284288
"Excel dialect does not support correlated subqueries"
285289
)
286290

291+
def _is_unresolved_bindparam(self, bindparam: elements.BindParameter[Any]) -> bool:
292+
return bool(getattr(bindparam, "required", False)) and (
293+
getattr(bindparam, "callable", None) is None
294+
)
295+
296+
def visit_bindparam(
297+
self,
298+
bindparam: Any,
299+
within_columns_clause: Any = False,
300+
literal_binds: Any = False,
301+
skip_bind_expression: Any = False,
302+
literal_execute: Any = False,
303+
render_postcompile: Any = False,
304+
is_upsert_set: Any = False,
305+
**kwargs: Any,
306+
) -> str:
307+
if (
308+
self._subquery_depth > 0
309+
and bool(literal_binds)
310+
and isinstance(bindparam, elements.BindParameter)
311+
and self._is_unresolved_bindparam(bindparam)
312+
):
313+
raise exc.CompileError(self._SUBQUERY_BINDPARAM_ERROR)
314+
315+
visit_bp = cast("Callable[..., str]", super().visit_bindparam)
316+
return str(
317+
visit_bp(
318+
bindparam,
319+
within_columns_clause=within_columns_clause,
320+
literal_binds=literal_binds,
321+
skip_bind_expression=skip_bind_expression,
322+
literal_execute=literal_execute,
323+
render_postcompile=render_postcompile,
324+
is_upsert_set=is_upsert_set,
325+
**kwargs,
326+
)
327+
)
328+
287329
def visit_function(
288330
self,
289331
func: Any,

src/sqlalchemy_excel/dialect.py

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,20 +143,22 @@ def _split_sql_list_quote_aware(sql: str) -> list[str]:
143143
start = 0
144144
depth = 0
145145
in_quote = False
146+
quote_char = ""
146147
i = 0
147148
while i < len(sql):
148149
ch = sql[i]
149150
if in_quote:
150-
if ch == '"':
151-
if i + 1 < len(sql) and sql[i + 1] == '"':
151+
if ch == quote_char:
152+
if i + 1 < len(sql) and sql[i + 1] == quote_char:
152153
i += 2
153154
continue
154155
in_quote = False
155156
i += 1
156157
continue
157158

158-
if ch == '"':
159+
if ch in ("'", '"'):
159160
in_quote = True
161+
quote_char = ch
160162
elif ch == "(":
161163
depth += 1
162164
elif ch == ")" and depth > 0:
@@ -195,6 +197,33 @@ def _coerce_bool_query_value(raw: Any) -> bool:
195197
return str(value).lower() in ("true", "1", "yes")
196198

197199

200+
def _strip_quoted_literals(sql: str) -> str:
201+
out: list[str] = []
202+
in_quote = False
203+
quote_char = ""
204+
i = 0
205+
while i < len(sql):
206+
ch = sql[i]
207+
if in_quote:
208+
if ch == quote_char:
209+
if i + 1 < len(sql) and sql[i + 1] == quote_char:
210+
i += 2
211+
continue
212+
in_quote = False
213+
i += 1
214+
continue
215+
216+
if ch in ("'", '"'):
217+
in_quote = True
218+
quote_char = ch
219+
i += 1
220+
continue
221+
222+
out.append(ch)
223+
i += 1
224+
return "".join(out)
225+
226+
198227
def _parse_alter_add_column(statement: str) -> tuple[str, str, str] | None:
199228
match = re.match(
200229
r'^ALTER TABLE\s+("[^"]+"|[A-Za-z_][A-Za-z0-9_]*)\s+ADD\s+COLUMN\s+("[^"]+"|[A-Za-z_][A-Za-z0-9_]*)\s+(.+?)\s*;?$',
@@ -420,7 +449,7 @@ def _guard_alter_add_column_constraints(self, cursor: Any, statement: str) -> No
420449
return
421450

422451
raw_table_name, _raw_col_name, remainder = add_match
423-
tail = remainder.upper()
452+
tail = _strip_quoted_literals(remainder).upper()
424453
requires_existing_rows_backfill = "NOT NULL" in tail or "PRIMARY KEY" in tail
425454
if not requires_existing_rows_backfill:
426455
return

tests/test_alter_table.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,39 @@ def test_alter_table_add_primary_key_column_to_non_empty_table_raises(engine) ->
120120
assert pk_columns == ["id"]
121121

122122

123+
def test_alter_table_add_column_with_primary_key_string_default_is_allowed(
124+
engine,
125+
) -> None:
126+
metadata = MetaData()
127+
users = _create_users_table(metadata)
128+
metadata.create_all(engine)
129+
130+
with engine.begin() as conn:
131+
conn.execute(insert(users).values(id=1, name="Alice", age=30))
132+
133+
with engine.begin() as conn:
134+
conn.exec_driver_sql(
135+
"ALTER TABLE users ADD COLUMN note TEXT DEFAULT 'PRIMARY KEY'"
136+
)
137+
138+
columns = {col["name"]: col for col in inspect(engine).get_columns("users")}
139+
assert "note" in columns
140+
# Guard should NOT reject this — 'PRIMARY KEY' is inside a string literal,
141+
# not an actual constraint. We only verify the column was added successfully.
142+
143+
144+
def test_alter_table_add_column_with_actual_not_null_still_rejected(engine) -> None:
145+
metadata = MetaData()
146+
users = _create_users_table(metadata)
147+
metadata.create_all(engine)
148+
149+
with engine.begin() as conn:
150+
conn.execute(insert(users).values(id=1, name="Alice", age=30))
151+
152+
with engine.begin() as conn, pytest.raises(exc.OperationalError, match="non-empty"):
153+
conn.exec_driver_sql("ALTER TABLE users ADD COLUMN note TEXT NOT NULL")
154+
155+
123156
def test_alter_table_add_column_warns_for_unsupported_unique(engine) -> None:
124157
ddl = AddColumn("users", Column("email", String, unique=True))
125158

tests/test_compiler_guards.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
String,
1414
Table,
1515
UniqueConstraint,
16+
bindparam,
1617
create_engine,
1718
distinct,
1819
exc,
@@ -394,6 +395,37 @@ def test_compiler_allows_not_in_subquery(tmp_xlsx: str) -> None:
394395
engine.dispose()
395396

396397

398+
def test_compiler_allows_subquery_with_inline_literal_predicate(tmp_xlsx: str) -> None:
399+
engine = create_engine(f"excel:///{tmp_xlsx}")
400+
metadata = MetaData()
401+
users, orders = _build_tables(metadata)
402+
403+
sub = select(orders.c.user_id).where(orders.c.user_id == 1)
404+
stmt = select(users).where(users.c.id.in_(sub))
405+
compiled = stmt.compile(dialect=engine.dialect)
406+
sql = str(compiled)
407+
assert "IN" in sql
408+
assert "= 1" in sql
409+
410+
engine.dispose()
411+
412+
413+
def test_compiler_rejects_unresolved_bindparam_in_subquery(tmp_xlsx: str) -> None:
414+
engine = create_engine(f"excel:///{tmp_xlsx}")
415+
metadata = MetaData()
416+
users, orders = _build_tables(metadata)
417+
418+
sub = select(orders.c.user_id).where(orders.c.user_id == bindparam("user_id"))
419+
stmt = select(users).where(users.c.id.in_(sub))
420+
with pytest.raises(
421+
exc.CompileError,
422+
match="execution-time bind parameters in subqueries",
423+
):
424+
stmt.compile(dialect=engine.dialect)
425+
426+
engine.dispose()
427+
428+
397429
def test_compiler_accepts_update_with_subquery(tmp_xlsx: str) -> None:
398430
"""UPDATE with IN subquery should compile successfully."""
399431
engine = create_engine(f"excel:///{tmp_xlsx}")

tests/test_ddl.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,17 @@ def test_drop_table_sql(self, engine, users_table):
148148
assert sql == "DROP TABLE users"
149149

150150

151+
def test_split_sql_list_quote_aware_handles_single_quoted_comma() -> None:
152+
"""Verify _split_sql_list_quote_aware keeps commas inside single quotes."""
153+
from sqlalchemy_excel.dialect import _split_sql_list_quote_aware
154+
155+
sql = "id INTEGER PRIMARY KEY, note TEXT DEFAULT 'a,b' NOT NULL"
156+
parts = _split_sql_list_quote_aware(sql)
157+
assert len(parts) == 2
158+
assert parts[0].strip() == "id INTEGER PRIMARY KEY"
159+
assert "DEFAULT 'a,b'" in parts[1]
160+
161+
151162
class TestDDLExecution:
152163
"""Test DDL statement execution (integration)."""
153164

0 commit comments

Comments
 (0)