Skip to content

Commit 64a4564

Browse files
chore: remove dead decode_rows_continuation and stale references
Issue 066 changed decode_continuation() to call decode_body() instead of decode_rows_continuation(), making the latter dead code. The method implemented a wire format that does not match the C dqlite server (which always includes column headers in every ROWS frame). Keeping it available would invite future callers to use the wrong format. Delete decode_rows_continuation (~100 lines) and its 7 dedicated test methods. Update 4 stale comments/docstrings that still referenced the old method or the incorrect "no column header" continuation format. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3878c59 commit 64a4564

3 files changed

Lines changed: 14 additions & 261 deletions

File tree

src/dqlitewire/messages/responses.py

Lines changed: 7 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -226,25 +226,18 @@ class RowsResponse(Message):
226226
has_more: bool = False
227227

228228
def __post_init__(self) -> None:
229-
# Defensive copies (issue 042). Three sources of aliasing
229+
# Defensive copies (issue 042). Two sources of aliasing
230230
# motivate this:
231231
#
232-
# 1. ``decode_body`` / ``decode_rows_continuation`` store
233-
# ``column_types = types`` where ``types`` is also stored
234-
# as ``all_row_types[0]``, so without a copy
235-
# ``self.column_types is self.row_types[0]`` and mutating
236-
# one silently rewrites the other.
232+
# 1. ``decode_body`` stores ``column_types = types`` where
233+
# ``types`` is also stored as ``all_row_types[0]``, so
234+
# without a copy ``self.column_types is self.row_types[0]``
235+
# and mutating one silently rewrites the other.
237236
#
238-
# 2. ``MessageDecoder.decode_continuation`` and
239-
# ``decode_rows_continuation`` pass the caller's
240-
# ``column_names`` list by reference into every
241-
# continuation response; any mutation on one response
242-
# propagates to every sibling.
243-
#
244-
# 3. User code constructing ``RowsResponse`` directly with a
237+
# 2. User code constructing ``RowsResponse`` directly with a
245238
# list they intend to keep mutating elsewhere.
246239
#
247-
# Copying here catches all three sites uniformly and survives
240+
# Copying here catches both sites uniformly and survives
248241
# future construction sites. The cost is two list allocations
249242
# per response — negligible compared to the row payload.
250243
self.column_names = list(self.column_names)
@@ -396,105 +389,6 @@ def decode_body(
396389
f"(decoded {len(rows)} rows, consumed {offset} of {len(data)} bytes)"
397390
)
398391

399-
@classmethod
400-
def decode_rows_continuation(
401-
cls,
402-
data: bytes,
403-
column_names: list[str],
404-
column_count: int,
405-
max_rows: int = DEFAULT_MAX_ROWS,
406-
) -> "RowsResponse":
407-
"""Decode a continuation message (rows without column header prefix).
408-
409-
After receiving a RowsResponse with has_more=True (PART marker), the
410-
server sends additional ROWS messages that contain only row data and a
411-
trailing marker — no column_count or column_names prefix. Use this
412-
method to decode those continuation messages, passing the column_names
413-
and column_count from the initial response.
414-
"""
415-
if column_count > _MAX_COLUMN_COUNT:
416-
raise DecodeError(f"Column count {column_count} exceeds maximum {_MAX_COLUMN_COUNT}")
417-
if len(column_names) != column_count:
418-
raise DecodeError(
419-
f"column_names length ({len(column_names)}) does not match "
420-
f"column_count ({column_count})"
421-
)
422-
offset = 0
423-
rows: list[list[Any]] = []
424-
all_row_types: list[list[ValueType]] = []
425-
column_types: list[ValueType] = []
426-
427-
if column_count == 0:
428-
from dqlitewire.constants import ROW_DONE_BYTE, ROW_PART_BYTE, WORD_SIZE
429-
430-
if offset + WORD_SIZE > len(data):
431-
raise DecodeError(
432-
"RowsResponse continuation exhausted without end marker (zero-column result)"
433-
)
434-
marker_byte = data[offset]
435-
if marker_byte == ROW_DONE_BYTE:
436-
has_more = False
437-
elif marker_byte == ROW_PART_BYTE:
438-
has_more = True
439-
else:
440-
raise DecodeError(
441-
f"Expected DONE or PART marker for zero-column result, got 0x{marker_byte:02x}"
442-
)
443-
return cls(
444-
column_names=column_names,
445-
column_types=[],
446-
row_types=[],
447-
rows=[],
448-
has_more=has_more,
449-
)
450-
451-
while offset < len(data):
452-
prev_offset = offset
453-
454-
result, consumed = decode_row_header(data[offset:], column_count)
455-
offset += consumed
456-
457-
if result is RowMarker.DONE:
458-
return cls(
459-
column_names,
460-
column_types=column_types,
461-
row_types=all_row_types,
462-
rows=rows,
463-
has_more=False,
464-
)
465-
if result is RowMarker.PART:
466-
return cls(
467-
column_names,
468-
column_types=column_types,
469-
row_types=all_row_types,
470-
rows=rows,
471-
has_more=True,
472-
)
473-
474-
types = result
475-
if not isinstance(types, list):
476-
raise DecodeError(f"Expected column types list, got {type(types).__name__}")
477-
all_row_types.append(types)
478-
if not column_types:
479-
column_types = types
480-
481-
values, consumed = decode_row_values(data[offset:], types)
482-
rows.append(values)
483-
offset += consumed
484-
485-
if len(rows) >= max_rows:
486-
raise DecodeError(f"Row count {len(rows)} exceeds maximum {max_rows}")
487-
488-
if offset == prev_offset:
489-
raise DecodeError(
490-
"No progress in row decoding (possible zero-column result with malformed data)"
491-
)
492-
493-
raise DecodeError(
494-
f"RowsResponse continuation exhausted without end marker "
495-
f"(decoded {len(rows)} rows, consumed {offset} of {len(data)} bytes)"
496-
)
497-
498392

499393
@dataclass
500394
class EmptyResponse(Message):

tests/test_codec_signal_safety.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
``MessageDecoder.decode()`` and ``decode_continuation()`` both consume
44
bytes from the buffer via ``read_message()`` and then parse them via
5-
``decode_bytes`` / ``RowsResponse.decode_rows_continuation``. The
5+
``decode_bytes`` / ``RowsResponse.decode_body``. The
66
parse step is wrapped in ``try/except Exception`` so that a
77
``DecodeError``/``ValueError``/``struct.error`` from the parser
88
poisons the buffer before propagating.

tests/test_messages_responses.py

Lines changed: 6 additions & 147 deletions
Original file line numberDiff line numberDiff line change
@@ -154,19 +154,12 @@ class TestRowsResponseAliasing:
154154
"""Regression tests for issue 042.
155155
156156
``RowsResponse`` used to store caller-supplied ``column_names`` and
157-
``column_types`` lists by reference, creating two kinds of silent
158-
aliasing:
159-
160-
1. ``column_types`` was stored as ``all_row_types[0]`` inside
161-
``decode_body`` / ``decode_rows_continuation`` via
162-
``if not column_types: column_types = types``. The returned
163-
object then had ``r.column_types is r.row_types[0]``, so
164-
mutating one list silently rewrote the other.
165-
2. ``column_names`` was passed through ``decode_rows_continuation``
166-
unchanged and attached directly to every continuation
167-
``RowsResponse``. Callers who mutated ``msg.column_names`` on
168-
one continuation silently mutated every sibling continuation
169-
AND the original response.
157+
``column_types`` lists by reference, creating silent aliasing:
158+
159+
``column_types`` was stored as ``all_row_types[0]`` inside
160+
``decode_body`` via ``if not column_types: column_types = types``.
161+
The returned object then had ``r.column_types is r.row_types[0]``,
162+
so mutating one list silently rewrote the other.
170163
171164
The fix copies both lists on construction via ``__post_init__``.
172165
"""
@@ -461,60 +454,6 @@ def test_zero_columns_missing_marker_raises(self) -> None:
461454
with pytest.raises(DecodeError, match="end marker"):
462455
RowsResponse.decode_body(data)
463456

464-
def test_zero_columns_continuation_missing_marker_raises(self) -> None:
465-
"""Zero-column continuation with no end marker should raise DecodeError."""
466-
import pytest
467-
468-
from dqlitewire.exceptions import DecodeError
469-
470-
# Empty data for a zero-column continuation
471-
with pytest.raises(DecodeError, match="end marker"):
472-
RowsResponse.decode_rows_continuation(
473-
data=b"",
474-
column_names=[],
475-
column_count=0,
476-
)
477-
478-
def test_decode_rows_continuation(self) -> None:
479-
"""Continuation messages (after PART marker) have rows but no column header."""
480-
from dqlitewire.tuples import encode_row_header, encode_row_values
481-
from dqlitewire.types import encode_uint64
482-
483-
column_names = ["id", "name"]
484-
types = [ValueType.INTEGER, ValueType.TEXT]
485-
# Build a continuation body: rows + DONE marker (no column_count/names prefix)
486-
body = b""
487-
body += encode_row_header(types)
488-
body += encode_row_values([3, "Charlie"], types)
489-
body += encode_row_header(types)
490-
body += encode_row_values([4, "Diana"], types)
491-
body += encode_uint64(0xFFFFFFFFFFFFFFFF) # DONE marker
492-
493-
decoded = RowsResponse.decode_rows_continuation(body, column_names, len(column_names))
494-
assert decoded.column_names == ["id", "name"]
495-
assert len(decoded.rows) == 2
496-
assert decoded.rows[0] == [3, "Charlie"]
497-
assert decoded.rows[1] == [4, "Diana"]
498-
assert decoded.has_more is False
499-
500-
def test_decode_rows_continuation_truncated_raises(self) -> None:
501-
"""Truncated continuation (no marker) must raise DecodeError, not silently return."""
502-
import pytest
503-
504-
from dqlitewire.exceptions import DecodeError
505-
from dqlitewire.tuples import encode_row_header, encode_row_values
506-
507-
column_names = ["id", "name"]
508-
types = [ValueType.INTEGER, ValueType.TEXT]
509-
# Build continuation body with row data but NO end marker
510-
body = b""
511-
body += encode_row_header(types)
512-
body += encode_row_values([3, "Charlie"], types)
513-
# No DONE or PART marker at end!
514-
515-
with pytest.raises(DecodeError, match="end marker"):
516-
RowsResponse.decode_rows_continuation(body, column_names, len(column_names))
517-
518457
def test_decode_body_rejects_non_list_row_header(self) -> None:
519458
"""decode_body must raise DecodeError if decode_row_header returns unexpected type."""
520459
from unittest.mock import patch
@@ -539,27 +478,6 @@ def test_decode_body_rejects_non_list_row_header(self) -> None:
539478
):
540479
RowsResponse.decode_body(body)
541480

542-
def test_decode_rows_continuation_rejects_non_list_row_header(self) -> None:
543-
"""Same guard as decode_body but for the continuation path."""
544-
from unittest.mock import patch
545-
546-
import pytest
547-
548-
from dqlitewire.exceptions import DecodeError
549-
from dqlitewire.tuples import encode_row_header, encode_row_values
550-
from dqlitewire.types import encode_uint64
551-
552-
# Build valid continuation body (rows only, no column header)
553-
body = encode_row_header([ValueType.INTEGER])
554-
body += encode_row_values([42], [ValueType.INTEGER])
555-
body += encode_uint64(0xFFFFFFFFFFFFFFFF) # DONE
556-
557-
with (
558-
patch("dqlitewire.messages.responses.decode_row_header", return_value=("bad", 8)),
559-
pytest.raises(DecodeError, match="Expected column types list"),
560-
):
561-
RowsResponse.decode_rows_continuation(body, column_names=["id"], column_count=1)
562-
563481
def test_truncated_body_without_marker_raises(self) -> None:
564482
"""Body exhausted without DONE/PART marker must raise DecodeError."""
565483
import pytest
@@ -626,29 +544,6 @@ def test_max_rows_limit_decode_body(self) -> None:
626544
with pytest.raises(DecodeError, match="Row count.*exceeds maximum"):
627545
RowsResponse.decode_body(body, max_rows=3)
628546

629-
def test_max_rows_limit_decode_rows_continuation(self) -> None:
630-
"""decode_rows_continuation should reject messages exceeding the max_rows limit."""
631-
import pytest
632-
633-
from dqlitewire.exceptions import DecodeError
634-
from dqlitewire.tuples import encode_row_header, encode_row_values
635-
from dqlitewire.types import encode_uint64
636-
637-
types = [ValueType.INTEGER]
638-
body = b""
639-
for i in range(5):
640-
body += encode_row_header(types)
641-
body += encode_row_values([i], types)
642-
body += encode_uint64(0xFFFFFFFFFFFFFFFF) # DONE
643-
644-
# Should succeed with default limit
645-
decoded = RowsResponse.decode_rows_continuation(body, ["x"], 1)
646-
assert len(decoded.rows) == 5
647-
648-
# Should fail with max_rows=2
649-
with pytest.raises(DecodeError, match="Row count.*exceeds maximum"):
650-
RowsResponse.decode_rows_continuation(body, ["x"], 1, max_rows=2)
651-
652547
def test_max_rows_exact_boundary_rejects_at_limit(self) -> None:
653548
"""max_rows=3 with exactly 3 rows should raise DecodeError.
654549
@@ -680,42 +575,6 @@ def build_body(n_rows: int) -> bytes:
680575
decoded = RowsResponse.decode_body(build_body(2), max_rows=3)
681576
assert len(decoded.rows) == 2
682577

683-
def test_continuation_column_count_mismatch_raises(self) -> None:
684-
"""decode_rows_continuation should reject mismatched column_names/column_count."""
685-
import pytest
686-
687-
from dqlitewire.exceptions import DecodeError
688-
from dqlitewire.tuples import encode_row_header, encode_row_values
689-
from dqlitewire.types import encode_uint64
690-
691-
types = [ValueType.INTEGER, ValueType.TEXT]
692-
body = encode_row_header(types)
693-
body += encode_row_values([1, "hello"], types)
694-
body += encode_uint64(0xFFFFFFFFFFFFFFFF) # DONE
695-
696-
# column_names has 3 elements but column_count is 2 — mismatch
697-
with pytest.raises(DecodeError, match="column_names.*does not match.*column_count"):
698-
RowsResponse.decode_rows_continuation(
699-
body,
700-
column_names=["a", "b", "c"],
701-
column_count=2,
702-
)
703-
704-
def test_decode_rows_continuation_rejects_excessive_column_count(self) -> None:
705-
"""decode_rows_continuation should reject column_count exceeding _MAX_COLUMN_COUNT."""
706-
import pytest
707-
708-
from dqlitewire.exceptions import DecodeError
709-
710-
body = b"\xff" * 8 # DONE marker
711-
excessive = 20_000
712-
with pytest.raises(DecodeError, match="exceeds maximum"):
713-
RowsResponse.decode_rows_continuation(
714-
body,
715-
column_names=["c"] * excessive,
716-
column_count=excessive,
717-
)
718-
719578

720579
class TestRowsResponseValueTypes:
721580
"""Full RowsResponse round-trips with BOOLEAN, UNIXTIME, ISO8601, and BLOB."""

0 commit comments

Comments
 (0)