Skip to content

Commit 3752729

Browse files
Read server version from the DBAPI module constant
_get_server_version_info ran a live SELECT sqlite_version() on every fresh engine connection and silently fell back to (3, 0, 0) on any transient error — that fallback disabled RETURNING, multi-values, and every other 3.35+ feature on the affected engine for its lifetime, and the round-trip was redundant because dqlitedbapi already pins sqlite_version_info as a module-level constant (matches the pysqlite override). Forward the constant directly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3474cce commit 3752729

2 files changed

Lines changed: 59 additions & 59 deletions

File tree

src/sqlalchemydqlite/base.py

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -345,35 +345,21 @@ def do_ping(self, dbapi_connection: Any) -> bool:
345345
cursor.close() # cursor close shouldn't crash the ping result
346346

347347
def _get_server_version_info(self, connection: Any) -> tuple[int, ...]:
348-
"""Return the server version as a tuple.
349-
350-
dqlite uses SQLite internally, so we return SQLite version. The
351-
parser is defensive: SQLite occasionally ships with suffixes
352-
(``3.46.0-alpha``, ``3.46.0rc1``) and a non-numeric part would
353-
otherwise raise ``ValueError`` on ``int()`` — we strip any
354-
trailing non-digit characters per component so the numeric
355-
prefix still parses.
348+
"""Return the server's SQLite version as a tuple.
349+
350+
Forwards ``dqlitedbapi.sqlite_version_info`` (a module-level
351+
constant pinning the minimum supported SQLite version). The
352+
previous implementation ran a live ``SELECT sqlite_version()``
353+
on every fresh engine connection and fell back to ``(3, 0, 0)``
354+
on any transient error — which silently disabled RETURNING /
355+
multi-values / all 3.35+ features on the affected engine. The
356+
DBAPI constant is authoritative and matches how pysqlite
357+
implements the same override.
356358
"""
357-
import re
358-
359-
try:
360-
result = connection.exec_driver_sql("SELECT sqlite_version()")
361-
version_str = result.scalar()
362-
if version_str:
363-
parts: list[int] = []
364-
for component in version_str.split("."):
365-
m = re.match(r"\d+", component)
366-
if not m:
367-
break
368-
parts.append(int(m.group(0)))
369-
if parts:
370-
return tuple(parts)
371-
except (
372-
_dbapi_exc.OperationalError,
373-
_client_exc.DqliteConnectionError,
374-
):
375-
pass
376-
return (3, 0, 0)
359+
info = getattr(self.dbapi, "sqlite_version_info", None)
360+
if info is not None:
361+
return tuple(info)
362+
return (3, 35, 0)
377363

378364

379365
# Register the dialect

tests/test_dialect.py

Lines changed: 45 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -184,51 +184,65 @@ def test_create_async_engine(self) -> None:
184184

185185

186186
class TestGetServerVersionInfo:
187-
def test_does_not_access_dbapi_connection_directly(self) -> None:
188-
"""_get_server_version_info should use exec_driver_sql, not internal attributes."""
189-
import ast
190-
import inspect
191-
import textwrap
187+
def test_reads_dbapi_module_constant(self) -> None:
188+
"""Forwards dqlitedbapi.sqlite_version_info — no live query."""
189+
from unittest.mock import MagicMock
192190

193-
source = textwrap.dedent(inspect.getsource(DqliteDialect._get_server_version_info))
194-
tree = ast.parse(source)
191+
import dqlitedbapi
195192

196-
# Check that it doesn't access .dbapi_connection
197-
for node in ast.walk(tree):
198-
if isinstance(node, ast.Attribute) and node.attr == "dbapi_connection":
199-
raise AssertionError(
200-
"_get_server_version_info accesses .dbapi_connection directly; "
201-
"should use connection.exec_driver_sql() instead"
202-
)
203-
204-
def test_returns_fallback_on_operational_error(self) -> None:
205-
"""Should return (3, 0, 0) if the server query fails with a
206-
connection-level error. Unrelated errors (bugs) propagate."""
193+
dialect = DqliteDialect()
194+
dialect.dbapi = dqlitedbapi # type: ignore[assignment]
195+
mock_conn = MagicMock()
196+
197+
result = dialect._get_server_version_info(mock_conn)
198+
assert result == tuple(dqlitedbapi.sqlite_version_info)
199+
# Critically, no wire round-trip.
200+
mock_conn.exec_driver_sql.assert_not_called()
201+
202+
def test_does_not_downgrade_on_transient_error(self) -> None:
203+
"""The previous fallback to (3, 0, 0) silently disabled 3.35+
204+
features on any transient error. The new implementation uses
205+
the DBAPI module constant directly, so there is no error path
206+
that could produce a stale tuple.
207+
"""
207208
from unittest.mock import MagicMock
208209

209-
import dqlitedbapi.exceptions
210+
import dqlitedbapi
210211

211212
dialect = DqliteDialect()
213+
dialect.dbapi = dqlitedbapi # type: ignore[assignment]
212214
mock_conn = MagicMock()
213-
mock_conn.exec_driver_sql.side_effect = dqlitedbapi.exceptions.OperationalError(
214-
"connection broken"
215-
)
215+
mock_conn.exec_driver_sql.side_effect = RuntimeError("should not be called")
216216

217217
result = dialect._get_server_version_info(mock_conn)
218-
assert result == (3, 0, 0)
218+
assert result == tuple(dqlitedbapi.sqlite_version_info)
219219

220-
def test_parses_version_string(self) -> None:
221-
"""Should parse a version string like '3.39.4' into a tuple."""
220+
def test_respects_dbapi_version_attribute(self) -> None:
221+
"""A hypothetical bump in the DBAPI's pinned SQLite version
222+
should flow through to the dialect without further wiring.
223+
"""
224+
from types import SimpleNamespace
222225
from unittest.mock import MagicMock
223226

224227
dialect = DqliteDialect()
225-
mock_conn = MagicMock()
226-
mock_result = MagicMock()
227-
mock_result.scalar.return_value = "3.39.4"
228-
mock_conn.exec_driver_sql.return_value = mock_result
228+
dialect.dbapi = SimpleNamespace(sqlite_version_info=(3, 46, 0)) # type: ignore[assignment]
229229

230-
result = dialect._get_server_version_info(mock_conn)
231-
assert result == (3, 39, 4)
230+
result = dialect._get_server_version_info(MagicMock())
231+
assert result == (3, 46, 0)
232+
233+
def test_falls_back_to_floor_when_dbapi_lacks_attribute(self) -> None:
234+
"""Defensive floor: returned only when the bound DBAPI module
235+
lacks ``sqlite_version_info`` entirely. Unreachable under the
236+
real dqlitedbapi module but documents intent.
237+
"""
238+
from types import SimpleNamespace
239+
from unittest.mock import MagicMock
240+
241+
dialect = DqliteDialect()
242+
dialect.dbapi = SimpleNamespace() # type: ignore[assignment]
243+
244+
result = dialect._get_server_version_info(MagicMock())
245+
assert result == (3, 35, 0)
232246

233247

234248
class TestGetDriverConnection:

0 commit comments

Comments
 (0)