Skip to content

Commit e24b8c8

Browse files
committed
fix: close oracle round 10 reflection and ddl gaps
1 parent acaa1de commit e24b8c8

8 files changed

Lines changed: 156 additions & 25 deletions

File tree

README.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,9 @@ with Session(engine) as session:
8888
users = session.query(User).all()
8989
```
9090

91+
> **Note (development mode):** When using from source checkout (not pip-installed),
92+
> add `import sqlalchemy_excel` before `create_engine()` to register the dialect.
93+
9194
## URL Format
9295

9396
```python
@@ -249,7 +252,7 @@ with engine.connect() as conn:
249252

250253
Reflection verifies that worksheets still exist before trusting metadata.
251254
If metadata remains for a deleted worksheet, sqlalchemy-excel removes the stale
252-
entry and returns an empty inspection result for that table name.
255+
entry and raises `NoSuchTableError` for that table name.
253256

254257
Raw DDL executed through `text()` or `exec_driver_sql()` keeps metadata in sync
255258
for `CREATE TABLE`, `ALTER TABLE`, and `DROP TABLE`.
@@ -301,6 +304,9 @@ with engine.connect() as conn:
301304
print(row)
302305
```
303306

307+
> **Note (development mode):** When using from source checkout (not pip-installed),
308+
> add `import sqlalchemy_excel` before `create_engine()` to register the dialect.
309+
304310
URL format: `excel+graph:///drive_id/item_id` where `drive_id` and `item_id` are Microsoft Graph resource identifiers.
305311
Query parameters: `?readonly=false` to enable write operations.
306312

src/sqlalchemy_excel/ddl.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,19 @@ def _warn_unsupported_constraints(
5454
)
5555
seen_check.add(check_key)
5656

57+
@staticmethod
58+
def _warn_unsupported_generated_columns(column: Any) -> None:
59+
if getattr(column, "computed", None) is not None:
60+
warnings.warn(
61+
f"Column '{column.name}': Computed columns are not supported by excel dialect; the expression will be ignored",
62+
stacklevel=2,
63+
)
64+
if getattr(column, "identity", None) is not None:
65+
warnings.warn(
66+
f"Column '{column.name}': Identity columns are not supported by excel dialect; auto-increment will not be applied",
67+
stacklevel=2,
68+
)
69+
5770
def _format_alter_table_name(self, operation: Any) -> str:
5871
schema = getattr(operation, "schema", None)
5972
if schema is not None:
@@ -104,6 +117,7 @@ def visit_create_table(self, create: Any, **kw: Any) -> str:
104117
seen_unique=seen_unique,
105118
seen_check=seen_check,
106119
)
120+
self._warn_unsupported_generated_columns(col)
107121
suffix = f" {' '.join(constraints)}" if constraints else ""
108122
columns.append(f"{col_name} {col_type}{suffix}")
109123

@@ -155,6 +169,7 @@ def visit_add_column(self, create: Any, **kw: Any) -> str:
155169
seen_unique=set(),
156170
seen_check=set(),
157171
)
172+
self._warn_unsupported_generated_columns(column)
158173
suffix = f" {' '.join(constraints)}" if constraints else ""
159174
return f"ALTER TABLE {table_name} ADD COLUMN {col_name} {col_type}{suffix}"
160175

src/sqlalchemy_excel/dialect.py

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ def _after_create(
4343
"Excel dialect does not support autoincrement=True; value must be set explicitly",
4444
stacklevel=2,
4545
)
46+
if getattr(col, "computed", None) is not None:
47+
warnings.warn(
48+
f"Column '{col.name}': Computed columns are not supported by excel dialect; the expression will be ignored",
49+
stacklevel=2,
50+
)
51+
if getattr(col, "identity", None) is not None:
52+
warnings.warn(
53+
f"Column '{col.name}': Identity columns are not supported by excel dialect; auto-increment will not be applied",
54+
stacklevel=2,
55+
)
4656

4757
import excel_dbapi
4858

@@ -172,11 +182,20 @@ def _driver_type_from_declared(type_expr: str) -> str:
172182
token = match.group(1) if match is not None else "TEXT"
173183
if token in {"FLOAT", "REAL", "DECIMAL", "NUMERIC", "DOUBLE"}:
174184
return "FLOAT"
185+
if token in {"SMALLINT", "BIGINT"}:
186+
return "INTEGER"
187+
if token == "TIMESTAMP":
188+
return "DATETIME"
175189
if token in {"INTEGER", "BOOLEAN", "DATE", "DATETIME", "TEXT"}:
176190
return token
177191
return "TEXT"
178192

179193

194+
def _coerce_bool_query_value(raw: Any) -> bool:
195+
value = raw[0] if isinstance(raw, tuple) else raw
196+
return str(value).lower() in ("true", "1", "yes")
197+
198+
180199
def _parse_alter_add_column(statement: str) -> tuple[str, str, str] | None:
181200
match = re.match(
182201
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*;?$',
@@ -325,21 +344,15 @@ def create_connect_args(self, url: URL) -> ConnectArgsType:
325344
"create": True,
326345
}
327346

328-
# Forward query parameters
329347
query = dict(url.query)
330348
if "engine" in query:
331349
kwargs["engine"] = query.pop("engine")
332350
if "autocommit" in query:
333-
autocommit = query.pop("autocommit")
334-
if isinstance(autocommit, tuple):
335-
autocommit_text = autocommit[0] if autocommit else ""
336-
else:
337-
autocommit_text = autocommit
338-
kwargs["autocommit"] = autocommit_text.lower() in (
339-
"true",
340-
"1",
341-
"yes",
342-
)
351+
kwargs["autocommit"] = _coerce_bool_query_value(query.pop("autocommit"))
352+
353+
for key in ("data_only", "sanitize_formulas", "create", "file_locking"):
354+
if key in query:
355+
kwargs[key] = _coerce_bool_query_value(query[key])
343356

344357
return ([], kwargs)
345358

@@ -430,6 +443,10 @@ def _extract_declared_type_name(type_expr: str) -> str | None:
430443
@staticmethod
431444
def _normalize_metadata_type_name(type_name: str) -> str:
432445
normalized = type_name.upper()
446+
if normalized in {"SMALLINT", "BIGINT"}:
447+
return "INTEGER"
448+
if normalized == "TIMESTAMP":
449+
return "DATETIME"
433450
if normalized in {
434451
"FLOAT",
435452
"REAL",
@@ -736,8 +753,6 @@ def create_connect_args(self, url: URL) -> ConnectArgsType:
736753

737754
query = dict(url.query)
738755
if "readonly" in query:
739-
raw = query.pop("readonly")
740-
val = raw[0] if isinstance(raw, tuple) else raw
741-
kwargs["readonly"] = str(val).lower() in ("true", "1", "yes")
756+
kwargs["readonly"] = _coerce_bool_query_value(query.pop("readonly"))
742757

743758
return ([], kwargs)

src/sqlalchemy_excel/reflection.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,13 @@
99
from typing import Any, cast
1010

1111
from sqlalchemy import types as sa_types
12+
from sqlalchemy.exc import NoSuchTableError
1213

1314
_TYPE_MAP: dict[str, type[sa_types.TypeEngine[Any]]] = {
1415
"TEXT": sa_types.String,
1516
"INTEGER": sa_types.Integer,
17+
"SMALLINT": sa_types.Integer,
18+
"BIGINT": sa_types.Integer,
1619
"FLOAT": sa_types.Float,
1720
"REAL": sa_types.Float,
1821
"DECIMAL": sa_types.Float,
@@ -22,6 +25,7 @@
2225
"BOOLEAN": sa_types.Boolean,
2326
"DATE": sa_types.Date,
2427
"DATETIME": sa_types.DateTime,
28+
"TIMESTAMP": sa_types.DateTime,
2529
}
2630

2731

@@ -83,7 +87,7 @@ def get_columns(
8387
meta = None
8488

8589
if not table_exists:
86-
return []
90+
raise NoSuchTableError(table_name)
8791

8892
if meta is not None:
8993
return [
@@ -130,7 +134,7 @@ def get_pk_constraint(
130134
meta = None
131135

132136
if not table_exists:
133-
return {"constrained_columns": [], "name": None}
137+
raise NoSuchTableError(table_name)
134138

135139
if meta is not None:
136140
pk_cols = [col["name"] for col in meta if col.get("primary_key", False)]

tests/test_default_guards.py

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,17 @@
22

33
import warnings
44

5-
from sqlalchemy import Column, Integer, MetaData, String, Table, create_engine, text
5+
from sqlalchemy import (
6+
Column,
7+
Computed,
8+
Identity,
9+
Integer,
10+
MetaData,
11+
String,
12+
Table,
13+
create_engine,
14+
text,
15+
)
616

717

818
def test_server_default_emits_warning(tmp_xlsx: str) -> None:
@@ -68,3 +78,50 @@ def test_normal_columns_do_not_emit_default_warnings(tmp_xlsx: str) -> None:
6878
messages = [str(item.message) for item in caught]
6979
assert not any("Excel dialect does not support" in message for message in messages)
7080
engine.dispose()
81+
82+
83+
def test_computed_column_emits_warning(tmp_xlsx: str) -> None:
84+
engine = create_engine(f"excel:///{tmp_xlsx}")
85+
metadata = MetaData()
86+
_ = Table(
87+
"with_computed",
88+
metadata,
89+
Column("id", Integer, primary_key=True),
90+
Column("name", String),
91+
Column("name_upper", String, Computed("name")),
92+
)
93+
94+
with warnings.catch_warnings(record=True) as caught:
95+
warnings.simplefilter("always")
96+
metadata.create_all(engine)
97+
98+
messages = [str(item.message) for item in caught]
99+
assert any(
100+
"Computed columns are not supported by excel dialect; the expression will be ignored"
101+
in message
102+
for message in messages
103+
)
104+
engine.dispose()
105+
106+
107+
def test_identity_column_emits_warning(tmp_xlsx: str) -> None:
108+
engine = create_engine(f"excel:///{tmp_xlsx}")
109+
metadata = MetaData()
110+
_ = Table(
111+
"with_identity",
112+
metadata,
113+
Column("id", Integer, Identity(), primary_key=True),
114+
Column("name", String),
115+
)
116+
117+
with warnings.catch_warnings(record=True) as caught:
118+
warnings.simplefilter("always")
119+
metadata.create_all(engine)
120+
121+
messages = [str(item.message) for item in caught]
122+
assert any(
123+
"Identity columns are not supported by excel dialect; auto-increment will not be applied"
124+
in message
125+
for message in messages
126+
)
127+
engine.dispose()

tests/test_dialect.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,22 @@ def test_connect_args_percent_decodes_local_path(self):
9191
assert kwargs["file_path"] == "folder name/test file.xlsx"
9292
engine.dispose()
9393

94+
def test_connect_args_forwards_data_only_query_param(self):
95+
engine = create_engine("excel:///placeholder.xlsx")
96+
dialect = engine.dialect
97+
url = make_url("excel:///placeholder.xlsx?data_only=false")
98+
_, kwargs = dialect.create_connect_args(url)
99+
assert kwargs["data_only"] is False
100+
engine.dispose()
101+
102+
def test_connect_args_forwards_create_query_param(self):
103+
engine = create_engine("excel:///placeholder.xlsx")
104+
dialect = engine.dialect
105+
url = make_url("excel:///placeholder.xlsx?create=false")
106+
_, kwargs = dialect.create_connect_args(url)
107+
assert kwargs["create"] is False
108+
engine.dispose()
109+
94110

95111
class TestConnection:
96112
"""Test basic connection operations."""

tests/test_e2e.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3502,6 +3502,23 @@ def test_e2e_raw_create_table_numeric_aliases_reflect_as_float(tmp_path) -> None
35023502
engine.dispose()
35033503

35043504

3505+
def test_e2e_raw_create_table_integer_and_timestamp_aliases_reflect_correctly(
3506+
tmp_path,
3507+
) -> None:
3508+
engine = _engine_for(tmp_path)
3509+
3510+
with engine.begin() as conn:
3511+
conn.exec_driver_sql("CREATE TABLE t (a SMALLINT, ts TIMESTAMP, b BIGINT)")
3512+
3513+
inspector = inspect(engine)
3514+
columns = {column["name"]: column["type"] for column in inspector.get_columns("t")}
3515+
assert isinstance(columns["a"], sa.Integer)
3516+
assert isinstance(columns["b"], sa.Integer)
3517+
assert isinstance(columns["ts"], sa.DateTime)
3518+
3519+
engine.dispose()
3520+
3521+
35053522
def test_e2e_raw_drop_table_removes_metadata(tmp_path) -> None:
35063523
engine = _engine_for(tmp_path)
35073524

@@ -3519,7 +3536,8 @@ def test_e2e_raw_drop_table_removes_metadata(tmp_path) -> None:
35193536

35203537
inspector = inspect(engine)
35213538
assert inspector.has_table("users") is False
3522-
assert inspector.get_columns("users") == []
3539+
with pytest.raises(exc.NoSuchTableError):
3540+
inspector.get_columns("users")
35233541

35243542
with engine.connect() as conn:
35253543
import excel_dbapi

tests/test_reflection_full.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import annotations
22

3-
from sqlalchemy import create_engine, inspect, text
3+
import pytest
4+
from sqlalchemy import create_engine, exc, inspect, text
45

56

67
def _engine_for(tmp_path):
@@ -63,11 +64,10 @@ def test_reflection_cleans_stale_metadata_when_sheet_is_missing(tmp_path) -> Non
6364
assert excel_dbapi.read_table_metadata(raw_conn, "ghost") is not None
6465

6566
inspector = inspect(engine)
66-
assert inspector.get_columns("ghost") == []
67-
assert inspector.get_pk_constraint("ghost") == {
68-
"constrained_columns": [],
69-
"name": None,
70-
}
67+
with pytest.raises(exc.NoSuchTableError):
68+
inspector.get_columns("ghost")
69+
with pytest.raises(exc.NoSuchTableError):
70+
inspector.get_pk_constraint("ghost")
7171

7272
with engine.connect() as conn:
7373
import excel_dbapi

0 commit comments

Comments
 (0)