Skip to content

Commit 8006fa6

Browse files
committed
fix: close oracle round 17 gaps
- Strip quoted literals in DDL metadata sync before constraint keyword detection to prevent false PK/NOT NULL reflection - Reject unsupported subquery shapes at compile time: multi-column, compound (UNION/INTERSECT/EXCEPT), and ORDER BY/LIMIT/OFFSET - Harden _metadata_header_matches with ordered comparison and document stale metadata limitation
1 parent 0693e53 commit 8006fa6

6 files changed

Lines changed: 147 additions & 21 deletions

File tree

src/sqlalchemy_excel/compiler.py

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
visitors,
5050
)
5151
from sqlalchemy.sql.expression import Join
52+
from sqlalchemy.sql.selectable import CompoundSelect, Select
5253

5354
if TYPE_CHECKING:
5455
from collections.abc import Callable, MutableMapping
@@ -288,6 +289,28 @@ def _reject_correlated_subquery(self, inner: Any) -> None:
288289
"Excel dialect does not support correlated subqueries"
289290
)
290291

292+
@staticmethod
293+
def _guard_subquery_shape(inner: Select[Any] | CompoundSelect[Any]) -> None:
294+
if isinstance(inner, CompoundSelect):
295+
raise exc.CompileError(
296+
"Excel dialect does not support compound subqueries in WHERE ... IN"
297+
)
298+
299+
if len(inner.selected_columns) != 1:
300+
raise exc.CompileError(
301+
"Excel dialect only supports single-column subqueries in WHERE ... IN"
302+
)
303+
304+
if inner._order_by_clauses:
305+
raise exc.CompileError(
306+
"Excel dialect does not support ORDER BY in subqueries"
307+
)
308+
309+
if inner._limit_clause is not None or inner._offset_clause is not None:
310+
raise exc.CompileError(
311+
"Excel dialect does not support LIMIT/OFFSET in subqueries"
312+
)
313+
291314
def _is_unresolved_bindparam(self, bindparam: elements.BindParameter[Any]) -> bool:
292315
return bool(getattr(bindparam, "required", False)) and (
293316
getattr(bindparam, "callable", None) is None
@@ -580,13 +603,18 @@ def visit_subquery(self, subquery: Any, **kw: Any) -> str:
580603

581604
inner = getattr(subquery, "element", None)
582605
if inner is not None:
583-
# Reject subqueries that themselves contain a JOIN
584-
for from_clause in inner.get_final_froms():
585-
if isinstance(from_clause, Join):
586-
raise exc.CompileError(
587-
"Excel dialect does not support JOIN inside subqueries"
588-
)
589-
self._reject_correlated_subquery(inner)
606+
if isinstance(inner, CompoundSelect):
607+
self._guard_subquery_shape(inner)
608+
elif getattr(inner, "__visit_name__", None) == "select":
609+
select_inner = cast("Select[Any]", inner)
610+
self._guard_subquery_shape(select_inner)
611+
# Reject subqueries that themselves contain a JOIN
612+
for from_clause in select_inner.get_final_froms():
613+
if isinstance(from_clause, Join):
614+
raise exc.CompileError(
615+
"Excel dialect does not support JOIN inside subqueries"
616+
)
617+
self._reject_correlated_subquery(select_inner)
590618

591619
self._subquery_depth += 1
592620
try:
@@ -598,7 +626,8 @@ def visit_subquery(self, subquery: Any, **kw: Any) -> str:
598626

599627
def visit_grouping(self, grouping: Any, asfrom: bool = False, **kwargs: Any) -> str:
600628
element = getattr(grouping, "element", None)
601-
is_subquery_select = getattr(element, "__visit_name__", None) == "select"
629+
visit_name = getattr(element, "__visit_name__", None)
630+
is_subquery_select = visit_name in {"select", "compound_select"}
602631
if is_subquery_select:
603632
if not self._in_in_clause:
604633
raise exc.CompileError(
@@ -616,13 +645,18 @@ def visit_grouping(self, grouping: Any, asfrom: bool = False, **kwargs: Any) ->
616645
)
617646

618647
assert element is not None # guaranteed by is_subquery_select check
619-
# Reject subqueries that themselves contain a JOIN
620-
for from_clause in element.get_final_froms():
621-
if isinstance(from_clause, Join):
622-
raise exc.CompileError(
623-
"Excel dialect does not support JOIN inside subqueries"
624-
)
625-
self._reject_correlated_subquery(element)
648+
if visit_name == "compound_select":
649+
self._guard_subquery_shape(cast("CompoundSelect[Any]", element))
650+
elif visit_name == "select":
651+
select_element = cast("Select[Any]", element)
652+
self._guard_subquery_shape(select_element)
653+
# Reject subqueries that themselves contain a JOIN
654+
for from_clause in select_element.get_final_froms():
655+
if isinstance(from_clause, Join):
656+
raise exc.CompileError(
657+
"Excel dialect does not support JOIN inside subqueries"
658+
)
659+
self._reject_correlated_subquery(select_element)
626660
kwargs["literal_binds"] = True
627661

628662
if is_subquery_select:

src/sqlalchemy_excel/dialect.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -584,7 +584,7 @@ def _sync_create_table_metadata(self, cursor: Any, statement: str) -> None:
584584

585585
col_name = self._unquote_identifier(col_match.group(1))
586586
remainder = col_match.group(2)
587-
remainder_upper = remainder.upper()
587+
remainder_upper = _strip_quoted_literals(remainder).upper()
588588
extracted = self._extract_declared_type_name(remainder)
589589
if extracted is None:
590590
continue
@@ -672,7 +672,7 @@ def _sync_alter_table_metadata(
672672
type_map[col_name] = added_type
673673
# Preserve nullable/PK hints from trailing constraints:
674674
# ALTER TABLE t ADD COLUMN c TYPE NOT NULL PRIMARY KEY
675-
tail = remainder.upper()
675+
tail = _strip_quoted_literals(remainder).upper()
676676
is_pk = "PRIMARY KEY" in tail or "PRIMARY_KEY" in tail
677677
nullable_map[col_name] = "NOT NULL" not in tail and not is_pk
678678
pk_map[col_name] = is_pk

src/sqlalchemy_excel/reflection.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,9 @@ def _metadata_header_matches(
232232
) -> bool:
233233
if header_names is None:
234234
return False
235-
# Check that metadata column names match live headers
236-
# AND metadata column count matches live column count.
237-
# This guards against the edge case where a sheet is deleted and
238-
# recreated with the same header names but different types/nullability/PKs.
235+
# Header validation is intentionally strict on column order and count.
236+
# Limitation: if a worksheet is recreated with the exact same headers,
237+
# this check alone cannot detect drift in type/nullability/PK metadata.
239238
return [col["name"] for col in meta] == header_names and len(meta) == len(
240239
header_names
241240
)

tests/test_compiler_guards.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,60 @@ def test_compiler_allows_subquery_with_inline_literal_predicate(tmp_xlsx: str) -
410410
engine.dispose()
411411

412412

413+
def test_compiler_rejects_multi_column_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, orders.c.id)
419+
stmt = select(users).where(users.c.id.in_(sub))
420+
with pytest.raises(exc.CompileError, match="single-column subqueries"):
421+
stmt.compile(dialect=engine.dialect)
422+
423+
engine.dispose()
424+
425+
426+
def test_compiler_rejects_compound_in_subquery(tmp_xlsx: str) -> None:
427+
engine = create_engine(f"excel:///{tmp_xlsx}")
428+
metadata = MetaData()
429+
users, orders = _build_tables(metadata)
430+
431+
sub = sa.union(select(orders.c.user_id), select(orders.c.id))
432+
stmt = select(users).where(users.c.id.in_(sub))
433+
with pytest.raises(exc.CompileError, match="compound subqueries"):
434+
stmt.compile(dialect=engine.dialect)
435+
436+
engine.dispose()
437+
438+
439+
def test_compiler_rejects_ordered_limited_in_subquery(tmp_xlsx: str) -> None:
440+
engine = create_engine(f"excel:///{tmp_xlsx}")
441+
metadata = MetaData()
442+
users, orders = _build_tables(metadata)
443+
444+
sub = select(orders.c.user_id).order_by(orders.c.user_id).limit(1)
445+
stmt = select(users).where(users.c.id.in_(sub))
446+
with pytest.raises(exc.CompileError, match="ORDER BY"):
447+
stmt.compile(dialect=engine.dialect)
448+
449+
engine.dispose()
450+
451+
452+
def test_compiler_allows_simple_single_column_subquery(tmp_xlsx: str) -> None:
453+
engine = create_engine(f"excel:///{tmp_xlsx}")
454+
metadata = MetaData()
455+
users, orders = _build_tables(metadata)
456+
457+
sub = select(orders.c.user_id).where(orders.c.user_id > 0)
458+
stmt = select(users).where(users.c.id.in_(sub))
459+
compiled = stmt.compile(dialect=engine.dialect)
460+
sql = str(compiled)
461+
assert "IN" in sql
462+
assert "SELECT" in sql
463+
464+
engine.dispose()
465+
466+
413467
def test_compiler_rejects_unresolved_bindparam_in_subquery(tmp_xlsx: str) -> None:
414468
engine = create_engine(f"excel:///{tmp_xlsx}")
415469
metadata = MetaData()

tests/test_ddl.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,16 @@ def test_drop_table_removes_worksheet(self, engine, metadata, users_table):
187187
insp = inspect(engine)
188188
tables = insp.get_table_names()
189189
assert "users" not in tables
190+
191+
def test_create_table_default_primary_key_literal_does_not_mark_pk(
192+
self, engine
193+
) -> None:
194+
with engine.begin() as conn:
195+
conn.exec_driver_sql(
196+
"CREATE TABLE users (id INTEGER, note TEXT DEFAULT 'PRIMARY KEY')"
197+
)
198+
199+
inspector = inspect(engine)
200+
columns = {column["name"]: column for column in inspector.get_columns("users")}
201+
assert columns["note"]["nullable"] is True
202+
assert inspector.get_pk_constraint("users")["constrained_columns"] == []

tests/test_reflection_full.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,32 @@ def test_reflection_validates_metadata_with_cursor_when_headers_unavailable(
161161
engine.dispose()
162162

163163

164+
def test_reflection_same_headers_cannot_detect_metadata_drift(tmp_path) -> None:
165+
workbook_path = tmp_path / "reflection.xlsx"
166+
engine = create_engine(f"excel:///{workbook_path}")
167+
168+
with engine.begin() as conn:
169+
conn.execute(text("CREATE TABLE users (id INTEGER PRIMARY KEY, name TEXT)"))
170+
171+
workbook = load_workbook(workbook_path)
172+
stale_sheet = workbook["users"]
173+
workbook.remove(stale_sheet)
174+
recreated = workbook.create_sheet("users")
175+
recreated.cell(row=1, column=1, value="id")
176+
recreated.cell(row=1, column=2, value="name")
177+
workbook.save(workbook_path)
178+
179+
engine.dispose()
180+
engine = create_engine(f"excel:///{workbook_path}")
181+
182+
inspector = inspect(engine)
183+
columns = inspector.get_columns("users")
184+
assert [column["name"] for column in columns] == ["id", "name"]
185+
assert inspector.get_pk_constraint("users")["constrained_columns"] == ["id"]
186+
187+
engine.dispose()
188+
189+
164190
def test_table_scoped_reflection_methods_raise_for_missing_table(tmp_path) -> None:
165191
engine = _engine_for(tmp_path)
166192
inspector = inspect(engine)

0 commit comments

Comments
 (0)