From 48417d011315391efad17ba28c433443fa7db8ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Wed, 14 Aug 2024 11:38:04 +0200 Subject: [PATCH] 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 [#1001](https://github.com/scylladb/scylla-rust-driver/issues/1001), it's a nice QOL feature to be able to deserialize empty CQL collections to empty Rust collections instead of `None::`. Deserialization logic is modified to enable that. --- scylla-cql/src/types/deserialize/value.rs | 73 +++++++++++++++---- .../src/types/deserialize/value_tests.rs | 58 +++++++++------ 2 files changed, 93 insertions(+), 38 deletions(-) diff --git a/scylla-cql/src/types/deserialize/value.rs b/scylla-cql/src/types/deserialize/value.rs index 074a7c298a..ca6e22b651 100644 --- 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>, ) -> Result { - let mut v = ensure_not_null_frame_slice::(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::( 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>, ) -> Result { - let mut v = ensure_not_null_frame_slice::(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::( 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 index 9375ce47f6..f74eb17850 100644 --- 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) { + >::type_check(&typ).unwrap(); + >::deserialize(&typ, None).unwrap(); + } + + check::>(&list_typ); + check::>(&set_typ); + check::>(&set_typ); + check::>(&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) { + >::type_check(&typ).unwrap(); + >::deserialize(&typ, None).unwrap(); + } + + check::>(&map_typ); + check::>(&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; - 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::()); - 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; - 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::()); - assert_eq!(err.cql_type, ser_typ); - assert_matches!(err.kind, BuiltinDeserializationErrorKind::ExpectedNonNull); - } - // Key type mismatch { let err = deserialize::>(