Skip to content

Commit ace6cf0

Browse files
wprzytulawprzytula
committed
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](#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.
1 parent e65f760 commit ace6cf0

File tree

2 files changed

+93
-38
lines changed

2 files changed

+93
-38
lines changed

‎scylla-cql/src/types/deserialize/value.rs‎

Lines changed: 59 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -670,6 +670,15 @@ impl<'frame, T> ListlikeIterator<'frame, T> {
670670
phantom_data: std::marker::PhantomData,
671671
}
672672
}
673+
674+
fn empty(coll_typ: &'frame ColumnType, elem_typ: &'frame ColumnType) -> Self {
675+
Self {
676+
coll_typ,
677+
elem_typ,
678+
raw_iter: FixedLengthBytesSequenceIterator::empty(),
679+
phantom_data: std::marker::PhantomData,
680+
}
681+
}
673682
}
674683

675684
impl<'frame, T> DeserializeValue<'frame> for ListlikeIterator<'frame, T>
@@ -699,7 +708,19 @@ where
699708
typ: &'frame ColumnType,
700709
v: Option<FrameSlice<'frame>>,
701710
) -> Result<Self, DeserializationError> {
702-
let mut v = ensure_not_null_frame_slice::<Self>(typ, v)?;
711+
let elem_typ = match typ {
712+
ColumnType::List(elem_typ) | ColumnType::Set(elem_typ) => elem_typ,
713+
_ => {
714+
unreachable!("Typecheck should have prevented this scenario!")
715+
}
716+
};
717+
718+
let mut v = if let Some(v) = v {
719+
v
720+
} else {
721+
return Ok(Self::empty(typ, elem_typ));
722+
};
723+
703724
let count = types::read_int_length(v.as_slice_mut()).map_err(|err| {
704725
mk_deser_err::<Self>(
705726
typ,
@@ -708,12 +729,7 @@ where
708729
),
709730
)
710731
})?;
711-
let elem_typ = match typ {
712-
ColumnType::List(elem_typ) | ColumnType::Set(elem_typ) => elem_typ,
713-
_ => {
714-
unreachable!("Typecheck should have prevented this scenario!")
715-
}
716-
};
732+
717733
Ok(Self::new(typ, elem_typ, count, v))
718734
}
719735
}
@@ -849,6 +865,21 @@ impl<'frame, K, V> MapIterator<'frame, K, V> {
849865
phantom_data_v: std::marker::PhantomData,
850866
}
851867
}
868+
869+
fn empty(
870+
coll_typ: &'frame ColumnType,
871+
k_typ: &'frame ColumnType,
872+
v_typ: &'frame ColumnType,
873+
) -> Self {
874+
Self {
875+
coll_typ,
876+
k_typ,
877+
v_typ,
878+
raw_iter: FixedLengthBytesSequenceIterator::empty(),
879+
phantom_data_k: std::marker::PhantomData,
880+
phantom_data_v: std::marker::PhantomData,
881+
}
882+
}
852883
}
853884

854885
impl<'frame, K, V> DeserializeValue<'frame> for MapIterator<'frame, K, V>
@@ -875,7 +906,19 @@ where
875906
typ: &'frame ColumnType,
876907
v: Option<FrameSlice<'frame>>,
877908
) -> Result<Self, DeserializationError> {
878-
let mut v = ensure_not_null_frame_slice::<Self>(typ, v)?;
909+
let (k_typ, v_typ) = match typ {
910+
ColumnType::Map(k_t, v_t) => (k_t, v_t),
911+
_ => {
912+
unreachable!("Typecheck should have prevented this scenario!")
913+
}
914+
};
915+
916+
let mut v = if let Some(v) = v {
917+
v
918+
} else {
919+
return Ok(Self::empty(typ, k_typ, v_typ));
920+
};
921+
879922
let count = types::read_int_length(v.as_slice_mut()).map_err(|err| {
880923
mk_deser_err::<Self>(
881924
typ,
@@ -884,12 +927,7 @@ where
884927
),
885928
)
886929
})?;
887-
let (k_typ, v_typ) = match typ {
888-
ColumnType::Map(k_t, v_t) => (k_t, v_t),
889-
_ => {
890-
unreachable!("Typecheck should have prevented this scenario!")
891-
}
892-
};
930+
893931
Ok(Self::new(typ, k_typ, v_typ, 2 * count, v))
894932
}
895933
}
@@ -1275,6 +1313,13 @@ impl<'frame> FixedLengthBytesSequenceIterator<'frame> {
12751313
remaining: count,
12761314
}
12771315
}
1316+
1317+
fn empty() -> Self {
1318+
Self {
1319+
slice: FrameSlice::new_empty(),
1320+
remaining: 0,
1321+
}
1322+
}
12781323
}
12791324

12801325
impl<'frame> Iterator for FixedLengthBytesSequenceIterator<'frame> {

‎scylla-cql/src/types/deserialize/value_tests.rs‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,24 @@ fn test_list_and_set() {
424424
expected_vec_string.into_iter().collect(),
425425
);
426426

427+
// Null collections are interpreted as empty collections, to retain convenience:
428+
// when an empty collection is sent to the DB, the DB nullifies the column instead.
429+
{
430+
let list_typ = ColumnType::List(Box::new(ColumnType::BigInt));
431+
let set_typ = ColumnType::Set(Box::new(ColumnType::BigInt));
432+
type CollTyp = i64;
433+
434+
fn check<'frame, Collection: DeserializeValue<'frame>>(typ: &'frame ColumnType) {
435+
<Collection as DeserializeValue<'_>>::type_check(typ).unwrap();
436+
<Collection as DeserializeValue<'_>>::deserialize(typ, None).unwrap();
437+
}
438+
439+
check::<Vec<CollTyp>>(&list_typ);
440+
check::<Vec<CollTyp>>(&set_typ);
441+
check::<HashSet<CollTyp>>(&set_typ);
442+
check::<BTreeSet<CollTyp>>(&set_typ);
443+
}
444+
427445
// ser/de identity
428446
assert_ser_de_identity(&list_typ, &vec!["qwik"], &mut Bytes::new());
429447
assert_ser_de_identity(&set_typ, &vec!["qwik"], &mut Bytes::new());
@@ -486,6 +504,22 @@ fn test_map() {
486504
);
487505
assert_eq!(decoded_btree_string, expected_string.into_iter().collect());
488506

507+
// Null collections are interpreted as empty collections, to retain convenience:
508+
// when an empty collection is sent to the DB, the DB nullifies the column instead.
509+
{
510+
let map_typ = ColumnType::Map(Box::new(ColumnType::BigInt), Box::new(ColumnType::Ascii));
511+
type KeyTyp = i64;
512+
type ValueTyp<'s> = &'s str;
513+
514+
fn check<'frame, Collection: DeserializeValue<'frame>>(typ: &'frame ColumnType) {
515+
<Collection as DeserializeValue<'_>>::type_check(typ).unwrap();
516+
<Collection as DeserializeValue<'_>>::deserialize(typ, None).unwrap();
517+
}
518+
519+
check::<HashMap<KeyTyp, ValueTyp>>(&map_typ);
520+
check::<BTreeMap<KeyTyp, ValueTyp>>(&map_typ);
521+
}
522+
489523
// ser/de identity
490524
assert_ser_de_identity(
491525
&typ,
@@ -1218,18 +1252,6 @@ fn test_set_or_list_errors() {
12181252
);
12191253
}
12201254

1221-
// Got null
1222-
{
1223-
type RustTyp = Vec<i32>;
1224-
let ser_typ = ColumnType::List(Box::new(ColumnType::Int));
1225-
1226-
let err = RustTyp::deserialize(&ser_typ, None).unwrap_err();
1227-
let err = get_deser_err(&err);
1228-
assert_eq!(err.rust_name, std::any::type_name::<RustTyp>());
1229-
assert_eq!(err.cql_type, ser_typ);
1230-
assert_matches!(err.kind, BuiltinDeserializationErrorKind::ExpectedNonNull);
1231-
}
1232-
12331255
// Bad element type
12341256
{
12351257
assert_type_check_error!(
@@ -1316,18 +1338,6 @@ fn test_map_errors() {
13161338
);
13171339
}
13181340

1319-
// Got null
1320-
{
1321-
type RustTyp = HashMap<i32, bool>;
1322-
let ser_typ = ColumnType::Map(Box::new(ColumnType::Int), Box::new(ColumnType::Boolean));
1323-
1324-
let err = RustTyp::deserialize(&ser_typ, None).unwrap_err();
1325-
let err = get_deser_err(&err);
1326-
assert_eq!(err.rust_name, std::any::type_name::<RustTyp>());
1327-
assert_eq!(err.cql_type, ser_typ);
1328-
assert_matches!(err.kind, BuiltinDeserializationErrorKind::ExpectedNonNull);
1329-
}
1330-
13311341
// Key type mismatch
13321342
{
13331343
let err = deserialize::<HashMap<i64, bool>>(

0 commit comments

Comments
 (0)