Skip to content

Commit 9ddd00d

Browse files
Apply row_factory before advancing _row_index in fetchone
Previously fetchone advanced _row_index BEFORE invoking the user's _row_factory. A factory that raises (custom factory hits a Unicode issue, dict-shaped factory hits a duplicate key, etc.) left the index advanced past the row that was never delivered. fetchmany's snapshot/restore at ``_row_index = snapshot + len(result)`` then underestimated the right index by 1 — silently REPLAYING the factory-failed row on the next fetchone() call instead of either retrying it or skipping cleanly. Reorder: read row, apply factory (may raise; index unchanged), then increment. The next fetchone() after a factory raise returns the same row, which is the desired retry semantic if the factory has been fixed; otherwise the same exception fires deterministically. Applied symmetrically to sync and async cursors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9b3f49b commit 9ddd00d

3 files changed

Lines changed: 148 additions & 2 deletions

File tree

src/dqlitedbapi/aio/cursor.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,10 +536,17 @@ async def fetchone(self) -> tuple[Any, ...] | None:
536536
return None
537537

538538
row = self._rows[self._row_index]
539-
self._row_index += 1
539+
# Apply row_factory BEFORE advancing ``_row_index`` so a raise
540+
# inside a custom factory leaves the index unchanged. Without
541+
# this ordering, ``fetchmany``'s snapshot/restore at
542+
# ``snapshot + len(result)`` underestimates by 1 for
543+
# factory-raised rows — silently REPLAYING a row on the next
544+
# call. Mirrors the sync sibling at ``cursor.py``'s fetchone.
540545
if self._row_factory is not None:
541546
transformed: tuple[Any, ...] = self._row_factory(self, row)
547+
self._row_index += 1
542548
return transformed
549+
self._row_index += 1
543550
return row
544551

545552
async def fetchmany(self, size: int | None = None) -> list[tuple[Any, ...]]:

src/dqlitedbapi/cursor.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1423,10 +1423,19 @@ def fetchone(self) -> tuple[Any, ...] | None:
14231423
return None
14241424

14251425
row = self._rows[self._row_index]
1426-
self._row_index += 1
1426+
# Apply row_factory BEFORE advancing ``_row_index`` so a raise
1427+
# inside a custom factory leaves the index unchanged. The next
1428+
# ``fetchone()`` call returns the same row (which is the
1429+
# desired retry semantic if the factory has been fixed).
1430+
# Without this ordering, ``fetchmany``'s snapshot/restore at
1431+
# ``snapshot + len(result)`` underestimates by 1 for
1432+
# factory-raised rows — silently REPLAYING a row on the next
1433+
# call instead of either retrying or skipping cleanly.
14271434
if self._row_factory is not None:
14281435
transformed: tuple[Any, ...] = self._row_factory(self, row)
1436+
self._row_index += 1
14291437
return transformed
1438+
self._row_index += 1
14301439
return row
14311440

14321441
def fetchmany(self, size: int | None = None) -> list[tuple[Any, ...]]:
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
"""Pin: ``fetchone`` / ``fetchmany`` must apply ``_row_factory`` BEFORE
2+
advancing ``_row_index``. A factory that raises must leave the index
3+
unchanged so the next ``fetchone()`` call returns the same row.
4+
5+
Without this ordering, ``fetchmany``'s snapshot/restore at
6+
``snapshot + len(result)`` underestimates by 1 for factory-raised
7+
rows — silently REPLAYING a row on the next call (skipping over it
8+
in `result` AND advancing past it in `_row_index`).
9+
10+
Tested across both sync and async cursors using direct attribute
11+
priming so we don't need a live connection.
12+
"""
13+
14+
from typing import Any
15+
from unittest.mock import MagicMock
16+
17+
import pytest
18+
19+
from dqlitedbapi.aio.cursor import AsyncCursor
20+
from dqlitedbapi.cursor import Cursor
21+
22+
23+
def _prime_sync_cursor(rows: list[tuple[Any, ...]]) -> Cursor:
24+
cur = Cursor.__new__(Cursor)
25+
cur._closed = False
26+
cur._rows = rows
27+
cur._row_index = 0
28+
cur._description = (("col0", None, None, None, None, None, None),)
29+
cur._row_factory = None
30+
cur._rowcount = -1
31+
cur._lastrowid = None
32+
cur._arraysize = 1
33+
cur.messages = []
34+
conn = MagicMock()
35+
conn._check_thread = MagicMock()
36+
cur._connection = conn
37+
return cur
38+
39+
40+
def _prime_async_cursor(rows: list[tuple[Any, ...]]) -> AsyncCursor:
41+
cur = AsyncCursor.__new__(AsyncCursor)
42+
cur._closed = False
43+
cur._rows = rows
44+
cur._row_index = 0
45+
cur._description = (("col0", None, None, None, None, None, None),)
46+
cur._row_factory = None
47+
cur._rowcount = -1
48+
cur._lastrowid = None
49+
cur._arraysize = 1
50+
cur.messages = []
51+
conn = MagicMock()
52+
conn._check_thread_for_async = MagicMock()
53+
cur._connection = conn
54+
return cur
55+
56+
57+
def test_sync_fetchone_factory_raise_does_not_advance_index() -> None:
58+
cur = _prime_sync_cursor([("a",), ("b",), ("c",)])
59+
60+
def boom(_c: object, _r: tuple[Any, ...]) -> tuple[Any, ...]:
61+
raise RuntimeError("simulated factory failure")
62+
63+
cur._row_factory = boom
64+
65+
with pytest.raises(RuntimeError, match="simulated factory failure"):
66+
cur.fetchone()
67+
# _row_index unchanged: a retry returns the SAME row, not the
68+
# next one (which would be a silent skip).
69+
assert cur._row_index == 0
70+
71+
72+
def test_sync_fetchmany_factory_raise_no_replay_no_skip() -> None:
73+
"""5 rows; factory raises on the 3rd call (rows[2]). After the
74+
fetchmany raises, the next fetchone must return rows[2] — not
75+
rows[1] (replay) and not rows[3] (skip)."""
76+
cur = _prime_sync_cursor([(0,), (1,), (2,), (3,), (4,)])
77+
78+
call_count = [0]
79+
80+
def factory(_c: object, r: tuple[Any, ...]) -> tuple[Any, ...]:
81+
call_count[0] += 1
82+
if call_count[0] == 3:
83+
raise RuntimeError("simulated factory failure")
84+
return r
85+
86+
cur._row_factory = factory
87+
88+
with pytest.raises(RuntimeError):
89+
cur.fetchmany(size=5)
90+
91+
# After the raise: _row_index points at rows[2] (the failed row).
92+
# Without the fix it would point at rows[3] (skip) — len(result)=2
93+
# but _row_index pre-advanced once more.
94+
cur._row_factory = None # neutralise so the next call returns raw.
95+
assert cur.fetchone() == (2,)
96+
97+
98+
@pytest.mark.asyncio
99+
async def test_async_fetchone_factory_raise_does_not_advance_index() -> None:
100+
cur = _prime_async_cursor([("a",), ("b",), ("c",)])
101+
102+
def boom(_c: object, _r: tuple[Any, ...]) -> tuple[Any, ...]:
103+
raise RuntimeError("simulated factory failure")
104+
105+
cur._row_factory = boom
106+
107+
with pytest.raises(RuntimeError, match="simulated factory failure"):
108+
await cur.fetchone()
109+
assert cur._row_index == 0
110+
111+
112+
@pytest.mark.asyncio
113+
async def test_async_fetchmany_factory_raise_no_replay_no_skip() -> None:
114+
cur = _prime_async_cursor([(0,), (1,), (2,), (3,), (4,)])
115+
116+
call_count = [0]
117+
118+
def factory(_c: object, r: tuple[Any, ...]) -> tuple[Any, ...]:
119+
call_count[0] += 1
120+
if call_count[0] == 3:
121+
raise RuntimeError("simulated factory failure")
122+
return r
123+
124+
cur._row_factory = factory
125+
126+
with pytest.raises(RuntimeError):
127+
await cur.fetchmany(size=5)
128+
129+
cur._row_factory = None
130+
assert await cur.fetchone() == (2,)

0 commit comments

Comments
 (0)