Skip to content

Commit

Permalink
cqltypes: Serialize None values in collections as NULLs
Browse files Browse the repository at this point in the history
Fixes scylladb#201

When using parepared statements, None values in collections
were serialized as empty values (values with length == 0). This is unexpected and
inconsistent - None values are serialized as NULLs (vlaues with length == -1) in other cases:
 - Statement arguments, both for simple and prepared statements
 - Collection elements in simple statement

This commit fixes this weird behavior - now None values should be serialized as NULLs in all cases.
It also adds an integration test that checks new behavior.
  • Loading branch information
Lorak-mmk committed Jul 14, 2023
1 parent 18ea6d4 commit 324b586
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 9 deletions.
27 changes: 18 additions & 9 deletions cassandra/cqltypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -832,9 +832,12 @@ def serialize_safe(cls, items, protocol_version):
buf.write(pack(len(items)))
inner_proto = max(3, protocol_version)
for item in items:
itembytes = subtype.to_binary(item, inner_proto)
buf.write(pack(len(itembytes)))
buf.write(itembytes)
if item is None:
buf.write(int32_pack(-1))
else:
itembytes = subtype.to_binary(item, inner_proto)
buf.write(pack(len(itembytes)))
buf.write(itembytes)
return buf.getvalue()


Expand Down Expand Up @@ -902,12 +905,18 @@ def serialize_safe(cls, themap, protocol_version):
raise TypeError("Got a non-map object for a map value")
inner_proto = max(3, protocol_version)
for key, val in items:
keybytes = key_type.to_binary(key, inner_proto)
valbytes = value_type.to_binary(val, inner_proto)
buf.write(pack(len(keybytes)))
buf.write(keybytes)
buf.write(pack(len(valbytes)))
buf.write(valbytes)
if key is not None:
keybytes = key_type.to_binary(key, inner_proto)
buf.write(pack(len(keybytes)))
buf.write(keybytes)
else:
buf.write(int32_pack(-1))
if val is not None:
valbytes = value_type.to_binary(val, inner_proto)
buf.write(pack(len(valbytes)))
buf.write(valbytes)
else:
buf.write(int32_pack(-1))
return buf.getvalue()


Expand Down
49 changes: 49 additions & 0 deletions tests/integration/standard/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,55 @@ def test_can_insert_tuples_with_nulls(self):
self.assertEqual(('', None, None, b''), result[0].t)
self.assertEqual(('', None, None, b''), s.execute(read)[0].t)

def test_insert_collection_with_null_fails(self):
"""
NULLs in list / sets / maps are forbidden.
This is a regression test - there was a bug that serialized None values
in collections as empty values instead of nulls.
"""
s = self.session
columns = []
for collection_type in ['list', 'set']:
for simple_type in PRIMITIVE_DATATYPES_KEYS:
columns.append(f'{collection_type}_{simple_type} {collection_type}<{simple_type}>')
for simple_type in PRIMITIVE_DATATYPES_KEYS:
columns.append(f'map_k_{simple_type} map<{simple_type}, ascii>')
columns.append(f'map_v_{simple_type} map<ascii, {simple_type}>')
s.execute(f'CREATE TABLE collection_nulls (k int PRIMARY KEY, {", ".join(columns)})')

def raises_simple_and_prepared(exc_type, query_str, args):
self.assertRaises(exc_type, lambda: s.execute(query_str, args))
p = s.prepare(query_str.replace('%s', '?'))
self.assertRaises(exc_type, lambda: s.execute(p, args))

i = 0
for simple_type in PRIMITIVE_DATATYPES_KEYS:
if simple_type == 'blob':
continue # Unhashable type
query_str = f'INSERT INTO collection_nulls (k, set_{simple_type}) VALUES (%s, %s)'
args = [i, {None, get_sample(simple_type)}]
raises_simple_and_prepared(InvalidRequest, query_str, args)
i += 1
for simple_type in PRIMITIVE_DATATYPES_KEYS:
query_str = f'INSERT INTO collection_nulls (k, list_{simple_type}) VALUES (%s, %s)'
args = [i, [None, get_sample(simple_type)]]
raises_simple_and_prepared(InvalidRequest, query_str, args)
i += 1
for simple_type in PRIMITIVE_DATATYPES_KEYS:
if simple_type == 'blob':
continue # Unhashable type
query_str = f'INSERT INTO collection_nulls (k, map_k_{simple_type}) VALUES (%s, %s)'
args = [i, {get_sample(simple_type): 'abc', None: 'def'}]
raises_simple_and_prepared(InvalidRequest, query_str, args)
i += 1
for simple_type in PRIMITIVE_DATATYPES_KEYS:
query_str = f'INSERT INTO collection_nulls (k, map_v_{simple_type}) VALUES (%s, %s)'
args = [i, {'abc': None, 'def': get_sample(simple_type)}]
raises_simple_and_prepared(InvalidRequest, query_str, args)
i += 1



def test_can_insert_unicode_query_string(self):
"""
Test to ensure unicode strings can be used in a query
Expand Down

0 comments on commit 324b586

Please sign in to comment.