deser/value: convenient deserialization of empty collections

ScyllaDB does not distinguish empty collections from nulls. That is, INSERTing an empty collection is equivalent to nullifying the corresponding column. As pointed out in [scylladb#1001](scylladb#1001), it's a nice QOL feature to be able to deserialize empty CQL collections to empty Rust collections instead of `None::<RustCollection>`. Deserialization logic is modified to enable that.
wprzytula · Aug 23, 2024 · 48417d0 · 48417d0
1 parent e65f760
commit 48417d0
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 38 deletions.
diff --git a/scylla-cql/src/types/deserialize/value.rs b/scylla-cql/src/types/deserialize/value.rs
@@ -670,6 +670,15 @@ impl<'frame, T> ListlikeIterator<'frame, T> {
             phantom_data: std::marker::PhantomData,
         }
     }
+
+    fn empty(coll_typ: &'frame ColumnType, elem_typ: &'frame ColumnType) -> Self {
+        Self {
+            coll_typ,
+            elem_typ,
+            raw_iter: FixedLengthBytesSequenceIterator::empty(),
+            phantom_data: std::marker::PhantomData,
+        }
+    }
 }
 
 impl<'frame, T> DeserializeValue<'frame> for ListlikeIterator<'frame, T>
@@ -699,7 +708,19 @@ where
         typ: &'frame ColumnType,
         v: Option<FrameSlice<'frame>>,
     ) -> Result<Self, DeserializationError> {
-        let mut v = ensure_not_null_frame_slice::<Self>(typ, v)?;
+        let elem_typ = match typ {
+            ColumnType::List(elem_typ) | ColumnType::Set(elem_typ) => elem_typ,
+            _ => {
+                unreachable!("Typecheck should have prevented this scenario!")
+            }
+        };
+
+        let mut v = if let Some(v) = v {
+            v
+        } else {
+            return Ok(Self::empty(typ, elem_typ));
+        };
+
         let count = types::read_int_length(v.as_slice_mut()).map_err(|err| {
             mk_deser_err::<Self>(
                 typ,
@@ -708,12 +729,7 @@ where
                 ),
             )
         })?;
-        let elem_typ = match typ {
-            ColumnType::List(elem_typ) | ColumnType::Set(elem_typ) => elem_typ,
-            _ => {
-                unreachable!("Typecheck should have prevented this scenario!")
-            }
-        };
+
         Ok(Self::new(typ, elem_typ, count, v))
     }
 }
@@ -849,6 +865,21 @@ impl<'frame, K, V> MapIterator<'frame, K, V> {
             phantom_data_v: std::marker::PhantomData,
         }
     }
+
+    fn empty(
+        coll_typ: &'frame ColumnType,
+        k_typ: &'frame ColumnType,
+        v_typ: &'frame ColumnType,
+    ) -> Self {
+        Self {
+            coll_typ,
+            k_typ,
+            v_typ,
+            raw_iter: FixedLengthBytesSequenceIterator::empty(),
+            phantom_data_k: std::marker::PhantomData,
+            phantom_data_v: std::marker::PhantomData,
+        }
+    }
 }
 
 impl<'frame, K, V> DeserializeValue<'frame> for MapIterator<'frame, K, V>
@@ -875,7 +906,19 @@ where
         typ: &'frame ColumnType,
         v: Option<FrameSlice<'frame>>,
     ) -> Result<Self, DeserializationError> {
-        let mut v = ensure_not_null_frame_slice::<Self>(typ, v)?;
+        let (k_typ, v_typ) = match typ {
+            ColumnType::Map(k_t, v_t) => (k_t, v_t),
+            _ => {
+                unreachable!("Typecheck should have prevented this scenario!")
+            }
+        };
+
+        let mut v = if let Some(v) = v {
+            v
+        } else {
+            return Ok(Self::empty(typ, k_typ, v_typ));
+        };
+
         let count = types::read_int_length(v.as_slice_mut()).map_err(|err| {
             mk_deser_err::<Self>(
                 typ,
@@ -884,12 +927,7 @@ where
                 ),
             )
         })?;
-        let (k_typ, v_typ) = match typ {
-            ColumnType::Map(k_t, v_t) => (k_t, v_t),
-            _ => {
-                unreachable!("Typecheck should have prevented this scenario!")
-            }
-        };
+
         Ok(Self::new(typ, k_typ, v_typ, 2 * count, v))
     }
 }
@@ -1275,6 +1313,13 @@ impl<'frame> FixedLengthBytesSequenceIterator<'frame> {
             remaining: count,
         }
     }
+
+    fn empty() -> Self {
+        Self {
+            slice: FrameSlice::new_empty(),
+            remaining: 0,
+        }
+    }
 }
 
 impl<'frame> Iterator for FixedLengthBytesSequenceIterator<'frame> {

diff --git a/scylla-cql/src/types/deserialize/value_tests.rs b/scylla-cql/src/types/deserialize/value_tests.rs
@@ -424,6 +424,24 @@ fn test_list_and_set() {
         expected_vec_string.into_iter().collect(),
     );
 
+    // Null collections are interpreted as empty collections, to retain convenience:
+    // when an empty collection is sent to the DB, the DB nullifies the column instead.
+    {
+        let list_typ = ColumnType::List(Box::new(ColumnType::BigInt));
+        let set_typ = ColumnType::Set(Box::new(ColumnType::BigInt));
+        type CollTyp = i64;
+
+        fn check<'frame, Collection: DeserializeValue<'frame>>(typ: &'frame ColumnType) {
+            <Collection as DeserializeValue<'_>>::type_check(&typ).unwrap();
+            <Collection as DeserializeValue<'_>>::deserialize(&typ, None).unwrap();
+        }
+
+        check::<Vec<CollTyp>>(&list_typ);
+        check::<Vec<CollTyp>>(&set_typ);
+        check::<HashSet<CollTyp>>(&set_typ);
+        check::<BTreeSet<CollTyp>>(&set_typ);
+    }
+
     // ser/de identity
     assert_ser_de_identity(&list_typ, &vec!["qwik"], &mut Bytes::new());
     assert_ser_de_identity(&set_typ, &vec!["qwik"], &mut Bytes::new());
@@ -486,6 +504,22 @@ fn test_map() {
     );
     assert_eq!(decoded_btree_string, expected_string.into_iter().collect());
 
+    // Null collections are interpreted as empty collections, to retain convenience:
+    // when an empty collection is sent to the DB, the DB nullifies the column instead.
+    {
+        let map_typ = ColumnType::Map(Box::new(ColumnType::BigInt), Box::new(ColumnType::Ascii));
+        type KeyTyp = i64;
+        type ValueTyp<'s> = &'s str;
+
+        fn check<'frame, Collection: DeserializeValue<'frame>>(typ: &'frame ColumnType) {
+            <Collection as DeserializeValue<'_>>::type_check(&typ).unwrap();
+            <Collection as DeserializeValue<'_>>::deserialize(&typ, None).unwrap();
+        }
+
+        check::<HashMap<KeyTyp, ValueTyp>>(&map_typ);
+        check::<BTreeMap<KeyTyp, ValueTyp>>(&map_typ);
+    }
+
     // ser/de identity
     assert_ser_de_identity(
         &typ,
@@ -1218,18 +1252,6 @@ fn test_set_or_list_errors() {
         );
     }
 
-    // Got null
-    {
-        type RustTyp = Vec<i32>;
-        let ser_typ = ColumnType::List(Box::new(ColumnType::Int));
-
-        let err = RustTyp::deserialize(&ser_typ, None).unwrap_err();
-        let err = get_deser_err(&err);
-        assert_eq!(err.rust_name, std::any::type_name::<RustTyp>());
-        assert_eq!(err.cql_type, ser_typ);
-        assert_matches!(err.kind, BuiltinDeserializationErrorKind::ExpectedNonNull);
-    }
-
     // Bad element type
     {
         assert_type_check_error!(
@@ -1316,18 +1338,6 @@ fn test_map_errors() {
         );
     }
 
-    // Got null
-    {
-        type RustTyp = HashMap<i32, bool>;
-        let ser_typ = ColumnType::Map(Box::new(ColumnType::Int), Box::new(ColumnType::Boolean));
-
-        let err = RustTyp::deserialize(&ser_typ, None).unwrap_err();
-        let err = get_deser_err(&err);
-        assert_eq!(err.rust_name, std::any::type_name::<RustTyp>());
-        assert_eq!(err.cql_type, ser_typ);
-        assert_matches!(err.kind, BuiltinDeserializationErrorKind::ExpectedNonNull);
-    }
-
     // Key type mismatch
     {
         let err = deserialize::<HashMap<i64, bool>>(