Skip to content

Commit 9f06dc3

Browse files
committed
fix: CROSS JOIN from_linter edge registration, add warning + rollback tests
- CROSS JOIN now registers edges with from_linter instead of suppressing it entirely — preserves cartesian-product warnings for genuinely disconnected FROM elements while suppressing false positives - Added CROSS JOIN warning contract regression test - Added graph rollback no-op test (do_rollback swallows NotSupportedError) - Added *.xlsx.lock to .gitignore - Removed stale lock file artifacts
1 parent 9dc4f19 commit 9f06dc3

4 files changed

Lines changed: 67 additions & 4 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,4 @@ Thumbs.db
4242
# Test artifacts
4343
*.xlsx
4444
!tests/fixtures/*.xlsx
45+
*.xlsx.lock

src/sqlalchemy_excel/compiler.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535

3636
from __future__ import annotations
3737

38+
import itertools
3839
import re
3940
from typing import TYPE_CHECKING, Any, Literal, cast
4041

@@ -383,21 +384,30 @@ def visit_join(
383384
and not join.full
384385
and not join.isouter
385386
):
386-
# Intentional cartesian product -- suppress from_linter to
387-
# avoid SAWarning about missing join conditions.
387+
# Intentional cartesian product — register edges so the
388+
# FROM-linter treats both sides as connected (no spurious
389+
# SAWarning) while still detecting genuinely disconnected
390+
# FROM elements elsewhere in the statement.
391+
if from_linter:
392+
from_linter.edges.update(
393+
itertools.product(
394+
join.left._from_objects,
395+
join.right._from_objects,
396+
)
397+
)
388398
left = str(
389399
join.left._compiler_dispatch(
390400
self,
391401
asfrom=True,
392-
from_linter=None,
402+
from_linter=from_linter,
393403
**kwargs,
394404
)
395405
)
396406
right = str(
397407
join.right._compiler_dispatch(
398408
self,
399409
asfrom=True,
400-
from_linter=None,
410+
from_linter=from_linter,
401411
**kwargs,
402412
)
403413
)

tests/test_compiler_guards.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1649,3 +1649,36 @@ def test_strip_compound_branch_parens_double_wrapped_grouped() -> None:
16491649
result = ExcelCompiler._strip_compound_branch_parens(double_wrapped)
16501650
# Passes through completely unchanged.
16511651
assert result == double_wrapped
1652+
1653+
1654+
def test_cross_join_no_sa_warning(tmp_xlsx: str) -> None:
1655+
"""CROSS JOIN must not emit SAWarning about cartesian products."""
1656+
import warnings
1657+
1658+
import sqlalchemy as sa
1659+
from sqlalchemy import Column, Integer, MetaData, String, Table, create_engine
1660+
1661+
engine = create_engine(f"excel:///{tmp_xlsx}")
1662+
meta = MetaData()
1663+
t1 = Table("a", meta, Column("id", Integer), Column("x", String))
1664+
t2 = Table("b", meta, Column("id", Integer), Column("y", String))
1665+
meta.create_all(engine)
1666+
with engine.connect() as conn:
1667+
conn.execute(t1.insert(), [{"id": 1, "x": "v"}])
1668+
conn.execute(t2.insert(), [{"id": 1, "y": "w"}])
1669+
conn.commit()
1670+
1671+
with warnings.catch_warnings(record=True) as caught:
1672+
warnings.simplefilter("always")
1673+
with engine.connect() as conn:
1674+
j = t1.join(t2, sa.true())
1675+
stmt = sa.select(t1.c.id, t2.c.y).select_from(j)
1676+
rows = conn.execute(stmt).fetchall()
1677+
assert rows == [(1, "w")]
1678+
1679+
cartesian_warnings = [
1680+
w for w in caught if "cartesian" in str(w.message).lower()
1681+
]
1682+
assert cartesian_warnings == [], (
1683+
f"CROSS JOIN should not emit cartesian warning: {cartesian_warnings}"
1684+
)

tests/test_graph_dialect.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,3 +169,22 @@ def test_select_with_where(self):
169169
assert len(rows) == 1
170170
assert rows[0] == ("Alice",)
171171
engine.dispose()
172+
173+
174+
class TestGraphRollbackNoOp:
175+
"""do_rollback should swallow NotSupportedError on graph connections."""
176+
177+
def test_rollback_is_noop_on_graph(self):
178+
"""Graph backend connections treat rollback as no-op."""
179+
transport = httpx.MockTransport(_graph_handler)
180+
engine = create_engine(
181+
"excel+graph:///drv-test/itm-test",
182+
connect_args={
183+
"credential": "test-token",
184+
"transport": transport,
185+
},
186+
)
187+
with engine.connect() as conn:
188+
# Rollback should not raise — dialect swallows NotSupportedError
189+
conn.rollback()
190+
engine.dispose()

0 commit comments

Comments
 (0)