Skip to content

Commit 4292f0d

Browse files
committed
DRIVER-153: tests for SCYLLA_USE_METADATA_ID extension
Unit tests for the extension across its layers: test_protocol_features.py: SCYLLA_USE_METADATA_ID parsed from SUPPORTED and echoed in STARTUP options; absent by default. test_protocol.py (wire format): - metadata-id field written on v4 iff the connection negotiated the extension, with the exact bytes asserted; empty sentinel (b'') when the statement has no id, on both the extension path (v4) and the v5 native path (previously a TypeError); - _SKIP_METADATA_FLAG written when skip_meta is requested and the SCYLLA_USE_METADATA_ID extension is negotiated (v4 or v5), and NOT set on a native v5 connection without the extension (the id field is still written there, but the driver does not request skip); also suppressed - together with the id field - on a v4 connection without the extension, even when the statement carries an id; - PREPARED response decoding reads result_metadata_id iff the extension was negotiated (or v5); METADATA_CHANGED/NO_METADATA flag handling. test_query.py: PreparedStatement stores the (result_metadata, result_metadata_id) pair atomically - constructor, update_result_metadata, and the backwards-compatible single-attribute setters all replace the pair as one unit, and previously-taken snapshots stay internally consistent. test_response_future.py: - _create_response_future builds ExecuteMessage from a single pair snapshot: skip_meta only with both an id and usable cached metadata; disabled for id-less statements, NO_METADATA/LWT statements (result_metadata None) and zero-column statements (result_metadata []), while the id still rides on the message; - _query sends the message exactly as constructed (no per-connection mutation - regression test for the speculative-execution race) and decodes a skip_meta response against the metadata snapshotted when the message was built, not a later read of the statement cache (regression for a concurrent METADATA_CHANGED racing the send); - _set_result METADATA_CHANGED path replaces the cached pair atomically; a response with a new id but no column metadata (empty or absent) is ignored with a warning, leaving the cached pair unchanged - adopting the id alone would poison the cache with a stale-metadata/current-id pair the server would never refresh; - _execute_after_prepare refreshes the pair from exactly what the reprepare response carries, including the id, and no longer keeps the previous id when the response has none (@dkropachev: doing so risked pairing a stale id with metadata from a different schema version - test_execute_after_prepare_no_metadata_id_in_response_clears_id); - a statement with valid cached metadata+id must still get skip_meta=False when continuous_paging_options is set (@dkropachev: Connection.process_msg hardcodes result_metadata=None for paging-session pages after the first, so a skip_meta response would crash decoding them - test_create_execute_message_continuous_paging_disables_skip_meta). tests/integration/standard/test_scylla_metadata_id.py: live-server coverage against a real Scylla node via CCM, closing the one gap unit tests can't - whether Scylla actually treats the empty result_metadata_id sentinel as a mismatch rather than a protocol error. Confirms extension negotiation, the normal METADATA_CHANGED-after-ALTER-TABLE path, and the sentinel round trip: a statement forced back to result_metadata_id=None (simulating one prepared before the extension was known, e.g. mid rolling-upgrade) executes without error and comes back with a fresh id. Mirrors the equivalent live test already merged in the Java driver (scylladb/java-driver#758, should_handle_empty_metadata_id_when_executing_statement_when_supported). Run locally against Scylla 2026.1.9 via CCM; see PR description for setup and log excerpt.
1 parent 5713da7 commit 4292f0d

6 files changed

Lines changed: 837 additions & 8 deletions

File tree

cassandra/cluster.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4737,7 +4737,7 @@ class ResponseFuture(object):
47374737
_host = None
47384738
_control_connection_query_attempted = False
47394739
_TABLET_ROUTING_CTYPE = None
4740-
_bound_result_metadata = []
4740+
_bound_result_metadata = None
47414741

47424742
_warned_timeout = False
47434743

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import pytest
2+
3+
from tests.integration import use_singledc, SCYLLA_VERSION, BasicSharedKeyspaceUnitTestCase
4+
5+
pytestmark = pytest.mark.skipif(SCYLLA_VERSION is None, reason="SCYLLA_USE_METADATA_ID is a Scylla-only protocol extension")
6+
7+
8+
def setup_module():
9+
use_singledc()
10+
11+
12+
class ScyllaMetadataIdTests(BasicSharedKeyspaceUnitTestCase):
13+
"""
14+
Live-server coverage for the SCYLLA_USE_METADATA_ID protocol extension (DRIVER-153).
15+
"""
16+
17+
@classmethod
18+
def setUpClass(cls):
19+
cls.common_setup(1)
20+
21+
def setUp(self):
22+
self.table_name = "{}.{}".format(self.keyspace_name, self.function_table_name)
23+
self.session.execute("CREATE TABLE {} (a int PRIMARY KEY, b int, c int)".format(self.table_name))
24+
self.session.execute("INSERT INTO {} (a, b, c) VALUES (1, 1, 1)".format(self.table_name))
25+
26+
def tearDown(self):
27+
self.session.execute("DROP TABLE {}".format(self.table_name))
28+
29+
def test_extension_is_negotiated(self):
30+
"""
31+
Sanity check that SCYLLA_USE_METADATA_ID was actually negotiated on this
32+
connection. Without this, the tests below could pass vacuously if
33+
negotiation silently failed.
34+
"""
35+
pool = next(iter(self.session.get_pools()))
36+
connection, _ = pool.borrow_connection(timeout=2)
37+
try:
38+
assert connection.protocol_version == 4
39+
assert connection.features.use_metadata_id is True
40+
finally:
41+
pool.return_connection(connection)
42+
43+
def test_metadata_changed_recovers_after_schema_change(self):
44+
"""
45+
Normal METADATA_CHANGED path: after ALTER TABLE, the next EXECUTE must
46+
come back with a fresh result_metadata_id and updated column metadata,
47+
picked up automatically without re-preparing.
48+
"""
49+
prepared = self.session.prepare("SELECT * FROM {} WHERE a = ?".format(self.table_name))
50+
id_before = prepared.result_metadata_id
51+
assert id_before is not None
52+
assert len(prepared.result_metadata) == 3
53+
54+
self.session.execute(prepared.bind((1,)))
55+
56+
self.session.execute("ALTER TABLE {} ADD d int".format(self.table_name))
57+
self.session.execute(prepared.bind((1,)))
58+
59+
assert prepared.result_metadata_id is not None
60+
assert prepared.result_metadata_id != id_before
61+
assert len(prepared.result_metadata) == 4
62+
63+
def test_empty_sentinel_id_triggers_metadata_changed(self):
64+
"""
65+
Statements prepared before the extension was negotiated (e.g. mid rolling
66+
upgrade) start with result_metadata_id=None and must send the empty b''
67+
sentinel on their first EXECUTE. This must not be treated as a protocol
68+
error by the server: it must be treated as a mismatch, causing Scylla to
69+
respond with METADATA_CHANGED (fresh id + full metadata), which the
70+
driver then caches.
71+
"""
72+
prepared = self.session.prepare("SELECT * FROM {} WHERE a = ?".format(self.table_name))
73+
assert prepared.result_metadata_id is not None
74+
75+
# Simulate "prepared before the extension was known" by dropping the
76+
# cached id while keeping the cached metadata (mirrors the java-driver's
77+
# should_handle_empty_metadata_id_when_executing_statement_when_supported).
78+
prepared.update_result_metadata(prepared.result_metadata, None)
79+
assert prepared.result_metadata_id is None
80+
81+
result = self.session.execute(prepared.bind((1,)))
82+
83+
assert list(result) == [(1, 1, 1)]
84+
assert prepared.result_metadata_id is not None

tests/unit/test_protocol.py

Lines changed: 254 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,19 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
import io
16+
import struct
1517
import unittest
1618

19+
from typing import ClassVar
1720
from unittest.mock import Mock
1821

1922
from cassandra import ConsistencyLevel, ProtocolVersion, UnsupportedOperation
2023
from cassandra.protocol import (
21-
PrepareMessage, QueryMessage, ExecuteMessage, UnsupportedOperation,
24+
PrepareMessage, QueryMessage, ExecuteMessage,
2225
BatchMessage, StartupMessage, OptionsMessage, RegisterMessage,
23-
AuthResponseMessage, ProtocolHandler, _MessageType
26+
AuthResponseMessage, ProtocolHandler, _MessageType,
27+
ResultMessage, RESULT_KIND_ROWS
2428
)
2529
from cassandra.protocol_features import ProtocolFeatures
2630
from cassandra.query import BatchType
@@ -66,6 +70,253 @@ def test_execute_message(self):
6670
(b'\x00\x04',),
6771
(b'\x00\x00\x00\x01',), (b'\x00\x00',)])
6872

73+
def test_execute_message_skip_meta_flag_with_extension(self):
74+
"""
75+
skip_meta=True must set _SKIP_METADATA_FLAG (0x02) in the flags byte when
76+
the connection negotiated SCYLLA_USE_METADATA_ID, and the metadata id
77+
field must be written on the wire.
78+
"""
79+
message = ExecuteMessage('1', [], 4, skip_meta=True, result_metadata_id=b'foo')
80+
mock_io = Mock()
81+
82+
message.send_body(mock_io, 4, ProtocolFeatures(use_metadata_id=True))
83+
# flags byte should be VALUES_FLAG | SKIP_METADATA_FLAG = 0x01 | 0x02 = 0x03
84+
self._check_calls(mock_io, [(b'\x00\x01',), (b'1',),
85+
(b'\x00\x03',), (b'foo',),
86+
(b'\x00\x04',), (b'\x03',), (b'\x00\x00',)])
87+
88+
def test_execute_message_skip_meta_suppressed_without_extension(self):
89+
"""
90+
skip_meta=True must NOT reach the wire on a pre-v5 connection that did not
91+
negotiate SCYLLA_USE_METADATA_ID: without the metadata-id mechanism, a
92+
schema change after PREPARE would leave the driver decoding rows with
93+
stale cached metadata. The metadata id field must not be written either.
94+
"""
95+
message = ExecuteMessage('1', [], 4, skip_meta=True, result_metadata_id=b'foo')
96+
mock_io = Mock()
97+
98+
message.send_body(mock_io, 4)
99+
# flags byte contains only VALUES_FLAG; no metadata id field
100+
self._check_calls(mock_io, [(b'\x00\x01',), (b'1',), (b'\x00\x04',), (b'\x01',), (b'\x00\x00',)])
101+
102+
def test_execute_message_v5_native_skip_meta_not_set(self):
103+
"""
104+
On a native protocol v5 connection (no Scylla extension), skip_meta=True must
105+
NOT set _SKIP_METADATA_FLAG. Upstream never emitted the flag on any version, and
106+
this PR keeps native v5 byte-identical to upstream — enabling skip on native v5 is
107+
a separate, out-of-scope behavior change. The metadata id field is still written
108+
(it is part of the v5 EXECUTE frame layout), so only VALUES_FLAG is set.
109+
"""
110+
message = ExecuteMessage('1', [], 4, skip_meta=True)
111+
mock_io = Mock()
112+
113+
message.send_body(mock_io, 5)
114+
# v5 wire layout:
115+
# query_id: short(1) + b'1'
116+
# result_metadata_id: short(0) + b'' (sentinel — None on init)
117+
# consistency: short(4) = ONE
118+
# flags (4-byte int): VALUES_FLAG(0x01) only — skip is NOT set on native v5
119+
# param count: short(0)
120+
self._check_calls(mock_io, [
121+
(b'\x00\x01',), (b'1',),
122+
(b'\x00\x00',), (b'',),
123+
(b'\x00\x04',),
124+
(b'\x00\x00\x00\x01',), (b'\x00\x00',),
125+
])
126+
127+
def test_execute_message_v5_with_extension_sets_skip_flag(self):
128+
"""
129+
skip is extension-driven, not version-driven: on a v5 connection that ALSO
130+
negotiated SCYLLA_USE_METADATA_ID, skip_meta=True does set _SKIP_METADATA_FLAG.
131+
This also confirms _SKIP_METADATA_FLAG actually reaches the wire (it was dead code
132+
upstream) whenever the extension gates it on.
133+
"""
134+
message = ExecuteMessage('1', [], 4, skip_meta=True)
135+
mock_io = Mock()
136+
137+
message.send_body(mock_io, 5, ProtocolFeatures(use_metadata_id=True))
138+
# flags (4-byte int): VALUES_FLAG(0x01) | SKIP_METADATA_FLAG(0x02) = 0x03
139+
self._check_calls(mock_io, [
140+
(b'\x00\x01',), (b'1',),
141+
(b'\x00\x00',), (b'',),
142+
(b'\x00\x04',),
143+
(b'\x00\x00\x00\x03',), (b'\x00\x00',),
144+
])
145+
146+
def test_execute_message_scylla_metadata_id_v4(self):
147+
"""result_metadata_id should be written on protocol v4 when the connection negotiated the Scylla extension."""
148+
message = ExecuteMessage('1', [], 4, result_metadata_id=b'foo')
149+
mock_io = Mock()
150+
151+
message.send_body(mock_io, 4, ProtocolFeatures(use_metadata_id=True))
152+
# metadata_id written before query params (same position as v5)
153+
self._check_calls(mock_io, [(b'\x00\x01',), (b'1',),
154+
(b'\x00\x03',), (b'foo',),
155+
(b'\x00\x04',), (b'\x01',), (b'\x00\x00',)])
156+
157+
def test_execute_message_scylla_metadata_id_none_writes_sentinel(self):
158+
"""
159+
When the connection negotiated the extension but result_metadata_id is None
160+
(e.g. LWT statement or mixed cluster), send_body must still write the field
161+
as an empty string sentinel (\\x00\\x00) so the frame layout matches what
162+
the server expects.
163+
"""
164+
message = ExecuteMessage('1', [], 4)
165+
# result_metadata_id intentionally left as None
166+
mock_io = Mock()
167+
168+
message.send_body(mock_io, 4, ProtocolFeatures(use_metadata_id=True))
169+
# empty sentinel: \x00\x00 (zero-length short) + b'' (zero bytes), then normal query params
170+
self._check_calls(mock_io, [(b'\x00\x01',), (b'1',),
171+
(b'\x00\x00',), (b'',),
172+
(b'\x00\x04',), (b'\x01',), (b'\x00\x00',)])
173+
174+
def test_execute_message_v5_metadata_id_none_writes_sentinel(self):
175+
"""
176+
On protocol v5, result_metadata_id is always written (uses_prepared_metadata).
177+
When result_metadata_id is None (e.g. LWT statement or mixed cluster where the
178+
statement was prepared before the extension was active), send_body must write an
179+
empty sentinel instead of crashing with TypeError.
180+
"""
181+
message = ExecuteMessage('1', [], 4)
182+
# result_metadata_id intentionally left as None; use_metadata_id stays False (v5 native path)
183+
mock_io = Mock()
184+
185+
message.send_body(mock_io, 5)
186+
# v5 always writes metadata_id: None → empty sentinel \x00\x00 + b'', then query params
187+
# v5 uses 4-byte flags: VALUES_FLAG = \x00\x00\x00\x01
188+
self._check_calls(mock_io, [(b'\x00\x01',), (b'1',),
189+
(b'\x00\x00',), (b'',),
190+
(b'\x00\x04',),
191+
(b'\x00\x00\x00\x01',), (b'\x00\x00',)])
192+
193+
def test_recv_results_prepared_scylla_extension_reads_metadata_id(self):
194+
"""
195+
When use_metadata_id is True (Scylla extension), result_metadata_id must be
196+
read from the PREPARE response even for protocol v4.
197+
"""
198+
# Build a minimal valid PREPARE response binary (no bind/result columns):
199+
# query_id: short(2) + b'ab'
200+
# result_metadata_id: short(3) + b'xyz' <-- only present when extension active
201+
# prepared flags: int(1) = global_tables_spec
202+
# colcount: int(0)
203+
# num_pk_indexes: int(0)
204+
# ksname: short(2) + b'ks'
205+
# cfname: short(2) + b'tb'
206+
# result flags: int(4) = no_metadata
207+
# result colcount: int(0)
208+
buf = io.BytesIO(
209+
struct.pack('>H', 2) + b'ab' # query_id
210+
+ struct.pack('>H', 3) + b'xyz' # result_metadata_id
211+
+ struct.pack('>i', 1) # prepared flags: global_tables_spec
212+
+ struct.pack('>i', 0) # colcount = 0
213+
+ struct.pack('>i', 0) # num_pk_indexes = 0
214+
+ struct.pack('>H', 2) + b'ks' # ksname
215+
+ struct.pack('>H', 2) + b'tb' # cfname
216+
+ struct.pack('>i', 4) # result flags: no_metadata
217+
+ struct.pack('>i', 0) # result colcount = 0
218+
)
219+
220+
features_with_extension = ProtocolFeatures(use_metadata_id=True)
221+
msg = ResultMessage(kind=4) # RESULT_KIND_PREPARED = 4
222+
msg.recv_results_prepared(buf, protocol_version=4,
223+
protocol_features=features_with_extension,
224+
user_type_map={})
225+
assert msg.query_id == b'ab'
226+
assert msg.result_metadata_id == b'xyz'
227+
228+
def test_recv_results_prepared_no_extension_skips_metadata_id(self):
229+
"""
230+
Without use_metadata_id, result_metadata_id must NOT be read on protocol v4.
231+
The buffer must NOT contain a metadata_id field.
232+
"""
233+
buf = io.BytesIO(
234+
struct.pack('>H', 2) + b'ab' # query_id
235+
# no result_metadata_id
236+
+ struct.pack('>i', 1) # prepared flags: global_tables_spec
237+
+ struct.pack('>i', 0) # colcount = 0
238+
+ struct.pack('>i', 0) # num_pk_indexes = 0
239+
+ struct.pack('>H', 2) + b'ks' # ksname
240+
+ struct.pack('>H', 2) + b'tb' # cfname
241+
+ struct.pack('>i', 4) # result flags: no_metadata
242+
+ struct.pack('>i', 0) # result colcount = 0
243+
)
244+
245+
features_without_extension = ProtocolFeatures(use_metadata_id=False)
246+
msg = ResultMessage(kind=4)
247+
msg.recv_results_prepared(buf, protocol_version=4,
248+
protocol_features=features_without_extension,
249+
user_type_map={})
250+
assert msg.query_id == b'ab'
251+
assert msg.result_metadata_id is None
252+
253+
def test_recv_results_prepared_v5_reads_metadata_id(self):
254+
"""
255+
On protocol v5, ProtocolVersion.uses_prepared_metadata() is True, so
256+
result_metadata_id must be read from the PREPARE response even when
257+
use_metadata_id is False (native v5 path, not the Scylla extension).
258+
"""
259+
buf = io.BytesIO(
260+
struct.pack('>H', 2) + b'ab' # query_id
261+
+ struct.pack('>H', 3) + b'xyz' # result_metadata_id (always present on v5)
262+
+ struct.pack('>i', 1) # prepared flags: global_tables_spec
263+
+ struct.pack('>i', 0) # colcount = 0
264+
+ struct.pack('>i', 0) # num_pk_indexes = 0
265+
+ struct.pack('>H', 2) + b'ks' # ksname
266+
+ struct.pack('>H', 2) + b'tb' # cfname
267+
+ struct.pack('>i', 4) # result flags: no_metadata
268+
+ struct.pack('>i', 0) # result colcount = 0
269+
)
270+
271+
features_no_extension = ProtocolFeatures(use_metadata_id=False)
272+
msg = ResultMessage(kind=4) # RESULT_KIND_PREPARED = 4
273+
msg.recv_results_prepared(buf, protocol_version=5,
274+
protocol_features=features_no_extension,
275+
user_type_map={})
276+
assert msg.query_id == b'ab'
277+
assert msg.result_metadata_id == b'xyz'
278+
279+
def test_recv_results_metadata_reads_metadata_id_on_change(self):
280+
"""
281+
When _METADATA_ID_FLAG (0x0008) is set in a ROWS result,
282+
recv_results_metadata must read and store the new result_metadata_id
283+
sent by the server (METADATA_CHANGED signal), and still populate
284+
column_metadata normally.
285+
"""
286+
# Wire layout for a ROWS result with METADATA_CHANGED:
287+
# flags: int(0x0008) = _METADATA_ID_FLAG
288+
# colcount: int(0)
289+
# result_metadata_id: short(4) + b'new1'
290+
# (no columns — colcount=0 — to keep the buffer minimal)
291+
buf = io.BytesIO(
292+
struct.pack('>i', 0x0008) # flags: METADATA_ID_FLAG
293+
+ struct.pack('>i', 0) # colcount = 0
294+
+ struct.pack('>H', 4) + b'new1' # result_metadata_id = b'new1'
295+
)
296+
msg = ResultMessage(kind=RESULT_KIND_ROWS)
297+
msg.recv_results_metadata(buf, user_type_map={})
298+
assert msg.result_metadata_id == b'new1'
299+
assert msg.column_metadata == []
300+
301+
def test_recv_results_metadata_no_metadata_flag_skips_metadata_id(self):
302+
"""
303+
When _NO_METADATA_FLAG (0x0004) is set, recv_results_metadata returns
304+
early and must NOT read or set result_metadata_id, even if the caller
305+
mistakenly sets _METADATA_ID_FLAG alongside it.
306+
"""
307+
# flags = _NO_METADATA_FLAG (0x0004), colcount = 0
308+
buf = io.BytesIO(
309+
struct.pack('>i', 0x0004) # flags: NO_METADATA
310+
+ struct.pack('>i', 0) # colcount = 0
311+
)
312+
msg = ResultMessage(kind=RESULT_KIND_ROWS)
313+
msg.recv_results_metadata(buf, user_type_map={})
314+
# recv_results_metadata returns early on NO_METADATA; result_metadata_id
315+
# must never be set as an instance attribute (it is not a class default).
316+
# column_metadata is a class attribute defaulting to None and must remain so.
317+
assert not hasattr(msg, 'result_metadata_id')
318+
assert msg.column_metadata is None
319+
69320
def test_query_message(self):
70321
"""
71322
Test to check the appropriate calls are made
@@ -237,7 +488,7 @@ class FrameByteIdentityTest(unittest.TestCase):
237488
The expected frames below were captured from the pre-change encoder.
238489
"""
239490

240-
EXPECTED_FRAMES = {
491+
EXPECTED_FRAMES: ClassVar[dict] = {
241492
'startup_v4': '0400000701000000160001000b43514c5f56455253494f4e0005332e342e35',
242493
'options_v4': '040000070500000000',
243494
'register_v4': '040000070b000000220002000f544f504f4c4f47595f4348414e4745000d5354415455535f4348414e4745',

0 commit comments

Comments
 (0)