Skip to content

Commit cd474a4

Browse files
committed
fix: harden SQL execution normalization and reflection consistency
1 parent 94ce146 commit cd474a4

9 files changed

Lines changed: 118 additions & 11 deletions

File tree

docs/COMPATIBILITY.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,10 @@ adaptation) and `excel-dbapi` (execution engine and workbook semantics).
4444
`sqlalchemy-excel` does not provide full ACID transactional guarantees.
4545

4646
- `Session.commit()` persists changes to the workbook as expected.
47-
- `Session.rollback()` is effectively a no-op for local workbook writes in
48-
standard usage; treat writes as durable once executed.
47+
- `Session.rollback()` for local workbooks (`engine=openpyxl` with
48+
`autocommit=False`) restores the latest committed snapshot.
49+
- Graph backend connections (`engine=graph`) do not support rollback and treat
50+
rollback calls as a no-op.
4951
- Use a single-writer model. Concurrent writes to the same workbook are not
5052
supported and can lead to conflicts/corruption.
5153
- If you need strict transaction isolation, rollback guarantees, and
@@ -101,11 +103,12 @@ Practical guidance:
101103

102104
- No CTE support.
103105
- No window functions (`OVER`).
104-
- No `ALTER TABLE` migration primitives.
106+
- `ALTER TABLE` supports `ADD COLUMN`, `DROP COLUMN`, and `RENAME COLUMN`.
105107
- No foreign key/index enforcement.
106108
- No correlated subqueries.
107109
- Constrained JOIN `ON` clauses (equality joins across sources).
108110
- No concurrent multi-writer guarantees.
109-
- `Session.rollback()` does not provide traditional database rollback semantics.
111+
- Rollback semantics depend on backend (`openpyxl` snapshot/restore;
112+
`graph` no rollback support).
110113

111114
When these limitations are blockers, prefer a traditional database backend.

src/sqlalchemy_excel/__init__.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@
22

33
from __future__ import annotations
44

5+
from importlib.metadata import PackageNotFoundError, version
6+
57
from .dialect import ExcelDialect, ExcelGraphDialect
68
from .dml import Insert, insert
79

8-
__version__ = "0.4.0"
10+
try:
11+
__version__ = version("sqlalchemy-excel")
12+
except PackageNotFoundError:
13+
__version__ = "0.5.4"
914

1015
__all__ = [
1116
"ExcelDialect",

src/sqlalchemy_excel/compiler.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747

4848

4949
_SUPPORTED_FUNCTIONS = {"count", "sum", "avg", "min", "max"}
50+
_BARE_IDENTIFIER_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*\Z")
5051

5152

5253
class ExcelIdentifierPreparer(compiler.IdentifierPreparer):
@@ -60,10 +61,19 @@ def __init__(self, dialect: Any) -> None:
6061
super().__init__(dialect, initial_quote="", final_quote="")
6162
self.reserved_words = set()
6263

64+
@staticmethod
65+
def _validate_identifier(ident: str) -> None:
66+
if not _BARE_IDENTIFIER_RE.fullmatch(ident):
67+
raise exc.CompileError(
68+
"Excel dialect identifiers must match [A-Za-z_][A-Za-z0-9_]*"
69+
)
70+
6371
def quote_identifier(self, value: str) -> str:
72+
self._validate_identifier(value)
6473
return value
6574

6675
def quote(self, ident: str, force: Any = None) -> str:
76+
self._validate_identifier(ident)
6777
return ident
6878

6979

src/sqlalchemy_excel/dialect.py

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from __future__ import annotations
44

5-
import re
65
import warnings
76
from typing import TYPE_CHECKING, Any, Literal, cast
87
from urllib.parse import unquote as _url_unquote
@@ -79,8 +78,48 @@ def _after_drop(
7978

8079

8180
# Register DDL events globally for all Table objects
82-
event.listen(Table, "after_create", _after_create)
83-
event.listen(Table, "after_drop", _after_drop)
81+
event.listen(Table, "after_create", _after_create, propagate=False)
82+
event.listen(Table, "after_drop", _after_drop, propagate=False)
83+
84+
85+
def _normalize_statement_whitespace_quote_aware(statement: str) -> str:
86+
out: list[str] = []
87+
in_quote = False
88+
quote_char = ""
89+
i = 0
90+
length = len(statement)
91+
92+
while i < length:
93+
ch = statement[i]
94+
95+
if in_quote:
96+
out.append(ch)
97+
if ch == quote_char:
98+
if i + 1 < length and statement[i + 1] == quote_char:
99+
out.append(statement[i + 1])
100+
i += 1
101+
else:
102+
in_quote = False
103+
i += 1
104+
continue
105+
106+
if ch in ("'", '"'):
107+
in_quote = True
108+
quote_char = ch
109+
out.append(ch)
110+
i += 1
111+
continue
112+
113+
if ch.isspace():
114+
if out and out[-1] != " ":
115+
out.append(" ")
116+
i += 1
117+
continue
118+
119+
out.append(ch)
120+
i += 1
121+
122+
return "".join(out).strip()
84123

85124

86125
class ExcelDialect( # type: ignore[misc] # pyright: ignore[reportIncompatibleMethodOverride]
@@ -153,7 +192,7 @@ def create_connect_args(self, url: URL) -> ConnectArgsType:
153192
excel:////absolute/path.xlsx → file_path="/absolute/path.xlsx"
154193
"""
155194
# url.database contains the path after the third slash
156-
file_path = url.database
195+
file_path = _url_unquote(url.database) if url.database else None
157196
if not file_path:
158197
raise ValueError("No file path in URL. Use excel:///path/to/file.xlsx")
159198

@@ -193,7 +232,7 @@ def do_execute(
193232
context: Any = None,
194233
) -> None:
195234
"""Execute a statement, normalizing whitespace for excel-dbapi."""
196-
normalized = re.sub(r"\s+", " ", statement).strip()
235+
normalized = _normalize_statement_whitespace_quote_aware(statement)
197236
cursor.execute(normalized, parameters)
198237
self._sync_alter_table_metadata(cursor, normalized)
199238

@@ -204,7 +243,7 @@ def do_execute_no_params(
204243
context: Any = None,
205244
) -> None:
206245
"""Execute a statement with no parameters."""
207-
normalized = re.sub(r"\s+", " ", statement).strip()
246+
normalized = _normalize_statement_whitespace_quote_aware(statement)
208247
cursor.execute(normalized, None)
209248
self._sync_alter_table_metadata(cursor, normalized)
210249

src/sqlalchemy_excel/reflection.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"TEXT": sa_types.String,
1515
"INTEGER": sa_types.Integer,
1616
"FLOAT": sa_types.Float,
17+
"REAL": sa_types.Float,
1718
"BOOLEAN": sa_types.Boolean,
1819
"DATE": sa_types.Date,
1920
"DATETIME": sa_types.DateTime,

tests/test_compiler_guards.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,8 @@ def test_compiler_visit_label_and_group_by_paths(tmp_xlsx: str) -> None:
447447
preparer = engine.dialect.identifier_preparer
448448
assert preparer.quote_identifier("users") == "users"
449449
assert preparer.quote("users") == "users"
450+
with pytest.raises(exc.CompileError, match=r"\[A-Za-z_\]\[A-Za-z0-9_\]\*"):
451+
preparer.quote_identifier("bad name")
450452

451453
engine.dispose()
452454

@@ -505,6 +507,23 @@ def execute(self, statement: str, parameters: object) -> None:
505507
dialect.do_execute_no_params(cursor, "SELECT 1\nFROM users")
506508
assert cursor.calls == [("SELECT 1 FROM users", None)]
507509

510+
cursor_quotes = CursorStub()
511+
dialect.do_execute_no_params(
512+
cursor_quotes,
513+
"SELECT 'a b', \"x y\" FROM\t users WHERE id = 1",
514+
)
515+
assert cursor_quotes.calls == [
516+
('SELECT \'a b\', "x y" FROM users WHERE id = 1', None)
517+
]
518+
519+
cursor_params = CursorStub()
520+
dialect.do_execute(
521+
cursor_params,
522+
"SELECT 'a b' FROM\t users WHERE id = ?",
523+
(1,),
524+
)
525+
assert cursor_params.calls == [("SELECT 'a b' FROM users WHERE id = ?", (1,))]
526+
508527
assert dialect._check_unicode_returns(connection=None) is True
509528
assert dialect._check_unicode_description(connection=None) is True
510529

tests/test_dialect.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,14 @@ def test_connect_args_file_path(self, tmp_xlsx):
8383
assert kwargs["autocommit"] is False
8484
engine.dispose()
8585

86+
def test_connect_args_percent_decodes_local_path(self):
87+
engine = create_engine("excel:///placeholder.xlsx")
88+
dialect = engine.dialect
89+
url = make_url("excel:///folder%20name/test%20file.xlsx")
90+
_, kwargs = dialect.create_connect_args(url)
91+
assert kwargs["file_path"] == "folder name/test file.xlsx"
92+
engine.dispose()
93+
8694

8795
class TestConnection:
8896
"""Test basic connection operations."""

tests/test_e2e.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3368,6 +3368,23 @@ def test_e2e_alter_table_add_column(tmp_path) -> None:
33683368
engine.dispose()
33693369

33703370

3371+
def test_e2e_alter_table_add_float_column_reflects_as_float(tmp_path) -> None:
3372+
engine = _engine_for(tmp_path)
3373+
metadata = MetaData()
3374+
_users_table(metadata)
3375+
metadata.create_all(engine)
3376+
3377+
with engine.connect() as conn:
3378+
conn.exec_driver_sql("ALTER TABLE users ADD COLUMN score FLOAT")
3379+
conn.commit()
3380+
3381+
columns = inspect(engine).get_columns("users")
3382+
score = next(col for col in columns if col["name"] == "score")
3383+
assert isinstance(score["type"], sa.Float)
3384+
3385+
engine.dispose()
3386+
3387+
33713388
def test_e2e_alter_table_drop_column(tmp_path) -> None:
33723389
engine = _engine_for(tmp_path)
33733390
metadata = MetaData()

tests/test_reflection.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
inspect,
1414
)
1515

16+
from sqlalchemy_excel.reflection import _sa_type_from_name
17+
1618

1719
@pytest.fixture
1820
def metadata():
@@ -92,6 +94,9 @@ def test_column_has_type(self, populated_engine):
9294
assert "type" in col
9395
assert col["type"] is not None
9496

97+
def test_real_type_name_maps_to_float(self):
98+
assert isinstance(_sa_type_from_name("REAL"), Float)
99+
95100

96101
class TestGetPKConstraint:
97102
"""Test get_pk_constraint — from metadata sheet."""

0 commit comments

Comments
 (0)