From 7030873fd108b3223fb90a31b05e5b4c87aa7c35 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Wed, 13 Dec 2023 08:35:18 +0100 Subject: [PATCH 1/7] serialize/{row,value}: rename confusing "mismatch" variants When serializing a Rust struct / CqlValue::UserDefinedType to a CQL UDT, or a Rust struct / BTreeMap / HashMap as a list of named values, there can be a fields/columns mismatch between the Rust and CQL definition and a field/column can be present in one or missing in another. Current error kind variants have very confusing or plainly wrong names/descriptions/messages. Fix the situation in this commit by enforcing a consistent naming scheme: - If something from missing in Rust data but required by the CQL type, use `MissingFor`. - If something is present in Rust data but missing from the CQL type, use `NoSuch` or `NoWithName`. Apart from that, fix the docstrings and the error messages. --- scylla-cql/src/types/serialize/row.rs | 24 ++++++++--------- scylla-cql/src/types/serialize/value.rs | 34 ++++++++++++------------- scylla-macros/src/serialize/cql.rs | 8 +++--- scylla-macros/src/serialize/row.rs | 8 +++--- 4 files changed, 37 insertions(+), 37 deletions(-) diff --git a/scylla-cql/src/types/serialize/row.rs b/scylla-cql/src/types/serialize/row.rs index 9ad7d0e56c..e8b2eae861 100644 --- a/scylla-cql/src/types/serialize/row.rs +++ b/scylla-cql/src/types/serialize/row.rs @@ -167,7 +167,7 @@ macro_rules! impl_serialize_row_for_map { match self.get(col.name.as_str()) { None => { return Err(mk_typck_err::( - BuiltinTypeCheckErrorKind::MissingValueForColumn { + BuiltinTypeCheckErrorKind::ValueMissingForColumn { name: col.name.clone(), }, )) @@ -191,7 +191,7 @@ macro_rules! impl_serialize_row_for_map { // Report the lexicographically first value for deterministic error messages let name = unused_columns.iter().min().unwrap(); return Err(mk_typck_err::( - BuiltinTypeCheckErrorKind::ColumnMissingForValue { + BuiltinTypeCheckErrorKind::NoColumnWithName { name: name.to_string(), }, )); @@ -513,10 +513,10 @@ pub enum BuiltinTypeCheckErrorKind { /// The Rust type provides a value for some column, but that column is not /// present in the statement. - MissingValueForColumn { name: String }, + NoColumnWithName { name: String }, /// A value required by the statement is not provided by the Rust type. - ColumnMissingForValue { name: String }, + ValueMissingForColumn { name: String }, /// A different column name was expected at given position. ColumnNameMismatch { @@ -531,16 +531,16 @@ impl Display for BuiltinTypeCheckErrorKind { BuiltinTypeCheckErrorKind::WrongColumnCount { actual, asked_for } => { write!(f, "wrong column count: the query requires {asked_for} columns, but {actual} were provided") } - BuiltinTypeCheckErrorKind::MissingValueForColumn { name } => { + BuiltinTypeCheckErrorKind::NoColumnWithName { name } => { write!( f, - "value for column {name} was not provided, but the query requires it" + "value for column {name} was provided, but there is no bind marker for this column in the query" ) } - BuiltinTypeCheckErrorKind::ColumnMissingForValue { name } => { + BuiltinTypeCheckErrorKind::ValueMissingForColumn { name } => { write!( f, - "value for column {name} was provided, but there is no bind marker for this column in the query" + "value for column {name} was not provided, but the query requires it" ) } BuiltinTypeCheckErrorKind::ColumnNameMismatch { rust_column_name, db_column_name } => write!( @@ -947,7 +947,7 @@ mod tests { let err = err.0.downcast_ref::().unwrap(); assert!(matches!( err.kind, - BuiltinTypeCheckErrorKind::ColumnMissingForValue { .. } + BuiltinTypeCheckErrorKind::ValueMissingForColumn { .. } )); let spec_duplicate_column = [ @@ -965,7 +965,7 @@ mod tests { let err = err.0.downcast_ref::().unwrap(); assert!(matches!( err.kind, - BuiltinTypeCheckErrorKind::MissingValueForColumn { .. } + BuiltinTypeCheckErrorKind::NoColumnWithName { .. } )); let spec_wrong_type = [ @@ -1074,7 +1074,7 @@ mod tests { let err = err.0.downcast_ref::().unwrap(); assert!(matches!( err.kind, - BuiltinTypeCheckErrorKind::ColumnMissingForValue { .. } + BuiltinTypeCheckErrorKind::ValueMissingForColumn { .. } )); let spec_duplicate_column = [ @@ -1092,7 +1092,7 @@ mod tests { let err = err.0.downcast_ref::().unwrap(); assert!(matches!( err.kind, - BuiltinTypeCheckErrorKind::MissingValueForColumn { .. } + BuiltinTypeCheckErrorKind::NoColumnWithName { .. } )); let spec_wrong_type = [ diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index 567b59cfab..a466ab136b 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -613,7 +613,7 @@ fn serialize_udt<'b>( let fname = indexed_fields.keys().min().unwrap(); return Err(mk_typck_err::( typ, - UdtTypeCheckErrorKind::UnexpectedFieldInDestination { + UdtTypeCheckErrorKind::NoSuchFieldInUdt { field_name: fname.to_string(), }, )); @@ -1309,11 +1309,11 @@ pub enum UdtTypeCheckErrorKind { /// The name of the UDT being serialized to does not match. NameMismatch { keyspace: String, type_name: String }, - /// One of the fields that is required to be present by the Rust struct was not present in the CQL UDT type. - MissingField { field_name: String }, + /// The Rust data does not have a field that is required in the CQL UDT type. + ValueMissingForUdtField { field_name: String }, - /// The Rust data contains a field that is not present in the UDT - UnexpectedFieldInDestination { field_name: String }, + /// The Rust data contains a field that is not present in the UDT. + NoSuchFieldInUdt { field_name: String }, /// A different field name was expected at given position. FieldNameMismatch { @@ -1336,12 +1336,12 @@ impl Display for UdtTypeCheckErrorKind { f, "the Rust UDT name does not match the actual CQL UDT name ({keyspace}.{type_name})" ), - UdtTypeCheckErrorKind::MissingField { field_name } => { - write!(f, "the field {field_name} is missing from the CQL UDT type") + UdtTypeCheckErrorKind::ValueMissingForUdtField { field_name } => { + write!(f, "the field {field_name} is missing in the Rust data but is required by the CQL UDT type") } - UdtTypeCheckErrorKind::UnexpectedFieldInDestination { field_name } => write!( + UdtTypeCheckErrorKind::NoSuchFieldInUdt { field_name } => write!( f, - "the field {field_name} present in the Rust data is not present in the CQL type" + "the field {field_name} that is present in the Rust data is not present in the CQL type" ), UdtTypeCheckErrorKind::FieldNameMismatch { rust_field_name, db_field_name } => write!( f, @@ -1584,7 +1584,9 @@ mod tests { let err = err.0.downcast_ref::().unwrap(); assert!(matches!( err.kind, - BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::MissingField { .. }) + BuiltinTypeCheckErrorKind::UdtError( + UdtTypeCheckErrorKind::ValueMissingForUdtField { .. } + ) )); let typ_unexpected_field = ColumnType::UserDefinedType { @@ -1608,9 +1610,7 @@ mod tests { let err = err.0.downcast_ref::().unwrap(); assert!(matches!( err.kind, - BuiltinTypeCheckErrorKind::UdtError( - UdtTypeCheckErrorKind::UnexpectedFieldInDestination { .. } - ) + BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. }) )); let typ_wrong_type = ColumnType::UserDefinedType { @@ -1788,7 +1788,9 @@ mod tests { let err = err.0.downcast_ref::().unwrap(); assert!(matches!( err.kind, - BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::MissingField { .. }) + BuiltinTypeCheckErrorKind::UdtError( + UdtTypeCheckErrorKind::ValueMissingForUdtField { .. } + ) )); let typ_unexpected_field = ColumnType::UserDefinedType { @@ -1812,9 +1814,7 @@ mod tests { let err = err.0.downcast_ref::().unwrap(); assert!(matches!( err.kind, - BuiltinTypeCheckErrorKind::UdtError( - UdtTypeCheckErrorKind::UnexpectedFieldInDestination { .. } - ) + BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { .. }) )); let typ_unexpected_field = ColumnType::UserDefinedType { diff --git a/scylla-macros/src/serialize/cql.rs b/scylla-macros/src/serialize/cql.rs index d3c5788401..4756901183 100644 --- a/scylla-macros/src/serialize/cql.rs +++ b/scylla-macros/src/serialize/cql.rs @@ -189,7 +189,7 @@ impl<'a> Generator for FieldSortingGenerator<'a> { } )* _ => return ::std::result::Result::Err(mk_typck_err( - #crate_path::UdtTypeCheckErrorKind::UnexpectedFieldInDestination { + #crate_path::UdtTypeCheckErrorKind::NoSuchFieldInUdt { field_name: <_ as ::std::clone::Clone>::clone(field_name), } )), @@ -204,7 +204,7 @@ impl<'a> Generator for FieldSortingGenerator<'a> { #( if !#visited_flag_names { return ::std::result::Result::Err(mk_typck_err( - #crate_path::UdtTypeCheckErrorKind::MissingField { + #crate_path::UdtTypeCheckErrorKind::ValueMissingForUdtField { field_name: <_ as ::std::string::ToString>::to_string(#rust_field_names), } )); @@ -299,7 +299,7 @@ impl<'a> Generator for FieldOrderedGenerator<'a> { } None => { return ::std::result::Result::Err(mk_typck_err( - #crate_path::UdtTypeCheckErrorKind::MissingField { + #crate_path::UdtTypeCheckErrorKind::ValueMissingForUdtField { field_name: <_ as ::std::string::ToString>::to_string(#rust_field_name), } )); @@ -312,7 +312,7 @@ impl<'a> Generator for FieldOrderedGenerator<'a> { statements.push(parse_quote! { if let Some((field_name, typ)) = field_iter.next() { return ::std::result::Result::Err(mk_typck_err( - #crate_path::UdtTypeCheckErrorKind::UnexpectedFieldInDestination { + #crate_path::UdtTypeCheckErrorKind::NoSuchFieldInUdt { field_name: <_ as ::std::clone::Clone>::clone(field_name), } )); diff --git a/scylla-macros/src/serialize/row.rs b/scylla-macros/src/serialize/row.rs index ee0f702d27..44b402d791 100644 --- a/scylla-macros/src/serialize/row.rs +++ b/scylla-macros/src/serialize/row.rs @@ -166,7 +166,7 @@ impl<'a> Generator for ColumnSortingGenerator<'a> { } )* _ => return ::std::result::Result::Err(mk_typck_err( - #crate_path::BuiltinRowTypeCheckErrorKind::MissingValueForColumn { + #crate_path::BuiltinRowTypeCheckErrorKind::NoColumnWithName { name: <_ as ::std::clone::Clone>::clone(&&spec.name), } )), @@ -181,7 +181,7 @@ impl<'a> Generator for ColumnSortingGenerator<'a> { #( if !#visited_flag_names { return ::std::result::Result::Err(mk_typck_err( - #crate_path::BuiltinRowTypeCheckErrorKind::ColumnMissingForValue { + #crate_path::BuiltinRowTypeCheckErrorKind::ValueMissingForColumn { name: <_ as ::std::string::ToString>::to_string(#rust_field_names), } )); @@ -267,7 +267,7 @@ impl<'a> Generator for ColumnOrderedGenerator<'a> { } None => { return ::std::result::Result::Err(mk_typck_err( - #crate_path::BuiltinRowTypeCheckErrorKind::ColumnMissingForValue { + #crate_path::BuiltinRowTypeCheckErrorKind::ValueMissingForColumn { name: <_ as ::std::string::ToString>::to_string(#rust_field_name), } )); @@ -280,7 +280,7 @@ impl<'a> Generator for ColumnOrderedGenerator<'a> { statements.push(parse_quote! { if let Some(spec) = column_iter.next() { return ::std::result::Result::Err(mk_typck_err( - #crate_path::BuiltinRowTypeCheckErrorKind::MissingValueForColumn { + #crate_path::BuiltinRowTypeCheckErrorKind::NoColumnWithName { name: <_ as ::std::clone::Clone>::clone(&spec.name), } )); From bdabeeceea76022ccc8d8a0e59b058e33efe473b Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Mon, 11 Dec 2023 19:28:04 +0100 Subject: [PATCH 2/7] serialize/row: check for unused named values in legacy fallback The code that adapts `ValueList` to `SerializeRow` and rewrites named values to regular values based on the context didn't use to check whether there are some superfluous values that do not match to any of the bind markers. Fix this. --- scylla-cql/src/types/serialize/row.rs | 30 +++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/scylla-cql/src/types/serialize/row.rs b/scylla-cql/src/types/serialize/row.rs index e8b2eae861..edada4f2d1 100644 --- a/scylla-cql/src/types/serialize/row.rs +++ b/scylla-cql/src/types/serialize/row.rs @@ -432,21 +432,40 @@ pub fn serialize_legacy_row( if !serialized.has_names() { serialized.iter().for_each(append_value); } else { - let values_by_name = serialized + let mut values_by_name = serialized .iter_name_value_pairs() - .map(|(k, v)| (k.unwrap(), v)) + .map(|(k, v)| (k.unwrap(), (v, false))) .collect::>(); + let mut unused_count = values_by_name.len(); for col in ctx.columns() { - let val = values_by_name.get(col.name.as_str()).ok_or_else(|| { + let (val, visited) = values_by_name.get_mut(col.name.as_str()).ok_or_else(|| { SerializationError(Arc::new( - ValueListToSerializeRowAdapterError::NoBindMarkerWithName { + ValueListToSerializeRowAdapterError::ValueMissingForBindMarker { name: col.name.clone(), }, )) })?; + if !*visited { + *visited = true; + unused_count -= 1; + } append_value(*val); } + + if unused_count != 0 { + // Choose the lexicographically earliest name for the sake + // of deterministic errors + let name = values_by_name + .iter() + .filter_map(|(k, (_, visited))| (!visited).then_some(k)) + .min() + .unwrap() + .to_string(); + return Err(SerializationError::new( + ValueListToSerializeRowAdapterError::NoBindMarkerWithName { name }, + )); + } } Ok(()) @@ -574,6 +593,9 @@ impl Display for BuiltinSerializationErrorKind { #[derive(Error, Debug)] pub enum ValueListToSerializeRowAdapterError { + #[error("Missing named value for column {name}")] + ValueMissingForBindMarker { name: String }, + #[error("There is no bind marker with name {name}, but a value for it was provided")] NoBindMarkerWithName { name: String }, } From 2810fafdc1bc1f8c9c811b56c7d90422db233b8e Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Mon, 11 Dec 2023 19:29:36 +0100 Subject: [PATCH 3/7] serialize/value: dedicated error variant for too big legacy values Add a dedicated error variant to ValueToSerializeCqlAdapterError, returned in case when the legacy implementation tried to serialize but overflowed the maximum allowed size by the protocol. Previously, the `ValueTooBig` error defined in frame::value would be returned, which was inconsistent with other error cases for the `Value` -> `SerializeCql` translation code. --- scylla-cql/src/types/serialize/value.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index a466ab136b..d91693e51e 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -909,7 +909,8 @@ pub fn serialize_legacy_value<'b, T: Value>( ) -> Result, SerializationError> { // It's an inefficient and slightly tricky but correct implementation. let mut buf = Vec::new(); - ::serialize(v, &mut buf).map_err(|err| SerializationError(Arc::new(err)))?; + ::serialize(v, &mut buf) + .map_err(|_| SerializationError::new(ValueToSerializeCqlAdapterError::TooBig))?; // Analyze the output. // All this dance shows how unsafe our previous interface was... @@ -1373,6 +1374,9 @@ impl Display for UdtSerializationErrorKind { #[derive(Error, Debug)] pub enum ValueToSerializeCqlAdapterError { + #[error("The value is too big to be serialized as it exceeds the maximum 2GB size limit")] + TooBig, + #[error("Output produced by the Value trait is too short to be considered a value: {size} < 4 minimum bytes")] TooShort { size: usize }, From e57a7f851de95a84ca66abb423b71be349f94e98 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Tue, 12 Dec 2023 03:14:19 +0100 Subject: [PATCH 4/7] serialize/value: fix error returned from dynamic tuples In case when serialization of one of the fields fails, the tuple should return an error indicating that serialization of the tuple failed, and nest the error returned by field serialization inside it. The error used to have the wrong ColumnType put into it - a loop variable containing the ColumnType of the field shadowed the function argument containing the ColumnType of the whole tuple. Fix the issue by renaming the loop variable and un-shadowing the function argument as a result. --- scylla-cql/src/types/serialize/value.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index d91693e51e..23203c8361 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -632,11 +632,11 @@ fn serialize_tuple_like<'t, 'b>( ) -> Result, SerializationError> { let mut builder = writer.into_value_builder(); - for (index, (el, typ)) in field_values.zip(field_types).enumerate() { + for (index, (el, el_typ)) in field_values.zip(field_types).enumerate() { let sub = builder.make_sub_writer(); match el { None => sub.set_null(), - Some(el) => serialize_cql_value(el, typ, sub).map_err(|err| { + Some(el) => serialize_cql_value(el, el_typ, sub).map_err(|err| { let err = fix_cql_value_name_in_err(err); mk_ser_err::( typ, From fa1cf21e0ecfe60c68a47eb0f2eea02e4924818b Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Tue, 12 Dec 2023 03:11:24 +0100 Subject: [PATCH 5/7] frame/value: implement some common traits for Unset It's a good practice to implement common traits for public types, and some of them will be used in the tests introduced in the next commits. --- scylla-cql/src/frame/value.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/scylla-cql/src/frame/value.rs b/scylla-cql/src/frame/value.rs index e4be751635..a6abccd89a 100644 --- a/scylla-cql/src/frame/value.rs +++ b/scylla-cql/src/frame/value.rs @@ -36,6 +36,7 @@ pub struct ValueTooBig; pub struct ValueOverflow; /// Represents an unset value +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub struct Unset; /// Represents an counter value From 53d14908516d7cb24e5efa9fbc91c5611671a1d7 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Tue, 12 Dec 2023 03:14:46 +0100 Subject: [PATCH 6/7] serialize/{row,value}: add tests for serialization errors The PR that introduced impls of the SerializeRow/SerializeCql for the types that used to implement the old traits relied on the tests in `value_tests.rs` - they were extended with more test cases and modified to run on both the old and the new traits. However, this was only done for the tests that serialize stuff without causing errors. The new impls return errors that are much more detailed than what the old ones used to return, and - as evidenced by the bugs fixed in previous commits - badly needed their own set of tests. Add such a set. All different kinds of built-in errors should be covered, with the exception of size overflow errors which would require allocating huge amounts of memory to trigger them. --- scylla-cql/src/types/serialize/row.rs | 128 ++++++- scylla-cql/src/types/serialize/value.rs | 440 +++++++++++++++++++++++- 2 files changed, 564 insertions(+), 4 deletions(-) diff --git a/scylla-cql/src/types/serialize/row.rs b/scylla-cql/src/types/serialize/row.rs index edada4f2d1..de9a45dc1d 100644 --- a/scylla-cql/src/types/serialize/row.rs +++ b/scylla-cql/src/types/serialize/row.rs @@ -750,10 +750,12 @@ impl<'a> Iterator for SerializedValuesIterator<'a> { #[cfg(test)] mod tests { + use std::collections::BTreeMap; + use crate::frame::response::result::{ColumnSpec, ColumnType, TableSpec}; use crate::frame::types::RawValue; use crate::frame::value::{LegacySerializedValues, MaybeUnset, ValueList}; - use crate::types::serialize::RowWriter; + use crate::types::serialize::{RowWriter, SerializationError}; use super::{ BuiltinSerializationError, BuiltinSerializationErrorKind, BuiltinTypeCheckError, @@ -881,6 +883,13 @@ mod tests { ret } + fn do_serialize_err(t: T, columns: &[ColumnSpec]) -> SerializationError { + let ctx = RowSerializationContext { columns }; + let mut ret = Vec::new(); + let mut builder = RowWriter::new(&mut ret); + t.serialize(&ctx, &mut builder).unwrap_err() + } + fn col(name: &str, typ: ColumnType) -> ColumnSpec { ColumnSpec { table_spec: TableSpec { @@ -892,6 +901,123 @@ mod tests { } } + fn get_typeck_err(err: &SerializationError) -> &BuiltinTypeCheckError { + match err.0.downcast_ref() { + Some(err) => err, + None => panic!("not a BuiltinTypeCheckError: {}", err), + } + } + + fn get_ser_err(err: &SerializationError) -> &BuiltinSerializationError { + match err.0.downcast_ref() { + Some(err) => err, + None => panic!("not a BuiltinSerializationError: {}", err), + } + } + + #[test] + fn test_tuple_errors() { + // Unit + #[allow(clippy::let_unit_value)] // The let binding below is intentional + let v = (); + let spec = [col("a", ColumnType::Text)]; + let err = do_serialize_err(v, &spec); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::<()>()); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::WrongColumnCount { + actual: 0, + asked_for: 1, + } + )); + + // Non-unit tuple + // Count mismatch + let v = ("Ala ma kota",); + let spec = [col("a", ColumnType::Text), col("b", ColumnType::Text)]; + let err = do_serialize_err(v, &spec); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::<(&str,)>()); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::WrongColumnCount { + actual: 1, + asked_for: 2, + } + )); + + // Serialization of one of the element fails + let v = ("Ala ma kota", 123_i32); + let spec = [col("a", ColumnType::Text), col("b", ColumnType::Text)]; + let err = do_serialize_err(v, &spec); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::<(&str, i32)>()); + let BuiltinSerializationErrorKind::ColumnSerializationFailed { name, err: _ } = &err.kind; + assert_eq!(name, "b"); + } + + #[test] + fn test_slice_errors() { + // Non-unit tuple + // Count mismatch + let v = vec!["Ala ma kota"]; + let spec = [col("a", ColumnType::Text), col("b", ColumnType::Text)]; + let err = do_serialize_err(v, &spec); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::WrongColumnCount { + actual: 1, + asked_for: 2, + } + )); + + // Serialization of one of the element fails + let v = vec!["Ala ma kota", "Kot ma pchły"]; + let spec = [col("a", ColumnType::Text), col("b", ColumnType::Int)]; + let err = do_serialize_err(v, &spec); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + let BuiltinSerializationErrorKind::ColumnSerializationFailed { name, err: _ } = &err.kind; + assert_eq!(name, "b"); + } + + #[test] + fn test_map_errors() { + // Missing value for a bind marker + let v: BTreeMap<_, _> = vec![("a", 123_i32)].into_iter().collect(); + let spec = [col("a", ColumnType::Int), col("b", ColumnType::Text)]; + let err = do_serialize_err(v, &spec); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + let BuiltinTypeCheckErrorKind::ValueMissingForColumn { name } = &err.kind else { + panic!("unexpected error kind: {}", err.kind) + }; + assert_eq!(name, "b"); + + // Additional value, not present in the query + let v: BTreeMap<_, _> = vec![("a", 123_i32), ("b", 456_i32)].into_iter().collect(); + let spec = [col("a", ColumnType::Int)]; + let err = do_serialize_err(v, &spec); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + let BuiltinTypeCheckErrorKind::NoColumnWithName { name } = &err.kind else { + panic!("unexpected error kind: {}", err.kind) + }; + assert_eq!(name, "b"); + + // Serialization of one of the element fails + let v: BTreeMap<_, _> = vec![("a", 123_i32), ("b", 456_i32)].into_iter().collect(); + let spec = [col("a", ColumnType::Int), col("b", ColumnType::Text)]; + let err = do_serialize_err(v, &spec); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + let BuiltinSerializationErrorKind::ColumnSerializationFailed { name, err: _ } = &err.kind; + assert_eq!(name, "b"); + } + // Do not remove. It's not used in tests but we keep it here to check that // we properly ignore warnings about unused variables, unnecessary `mut`s // etc. that usually pop up when generating code for empty structs. diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index 23203c8361..b12416ba97 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -1389,14 +1389,20 @@ pub enum ValueToSerializeCqlAdapterError { #[cfg(test)] mod tests { + use std::collections::BTreeMap; + use crate::frame::response::result::{ColumnType, CqlValue}; - use crate::frame::value::{MaybeUnset, Value}; + use crate::frame::value::{MaybeUnset, Unset, Value}; use crate::types::serialize::value::{ BuiltinSerializationError, BuiltinSerializationErrorKind, BuiltinTypeCheckError, - BuiltinTypeCheckErrorKind, + BuiltinTypeCheckErrorKind, MapSerializationErrorKind, MapTypeCheckErrorKind, + SetOrListSerializationErrorKind, SetOrListTypeCheckErrorKind, TupleSerializationErrorKind, + TupleTypeCheckErrorKind, }; - use crate::types::serialize::CellWriter; + use crate::types::serialize::{CellWriter, SerializationError}; + use bigdecimal::BigDecimal; + use num_bigint::BigInt; use scylla_macros::SerializeCql; use super::{SerializeCql, UdtSerializationErrorKind, UdtTypeCheckErrorKind}; @@ -1441,6 +1447,434 @@ mod tests { ret } + fn do_serialize_err(t: T, typ: &ColumnType) -> SerializationError { + let mut ret = Vec::new(); + let writer = CellWriter::new(&mut ret); + t.serialize(typ, writer).unwrap_err() + } + + fn get_typeck_err(err: &SerializationError) -> &BuiltinTypeCheckError { + match err.0.downcast_ref() { + Some(err) => err, + None => panic!("not a BuiltinTypeCheckError: {}", err), + } + } + + fn get_ser_err(err: &SerializationError) -> &BuiltinSerializationError { + match err.0.downcast_ref() { + Some(err) => err, + None => panic!("not a BuiltinSerializationError: {}", err), + } + } + + #[test] + fn test_native_errors() { + // Simple type mismatch + let v = 123_i32; + let err = do_serialize_err(v, &ColumnType::Double); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, ColumnType::Double); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MismatchedType { + expected: &[ColumnType::Int], + }, + )); + + // str (and also Uuid) are interesting because they accept two types, + // also check str here + let v = "Ala ma kota"; + let err = do_serialize_err(v, &ColumnType::Double); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::<&str>()); + assert_eq!(err.got, ColumnType::Double); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MismatchedType { + expected: &[ColumnType::Ascii, ColumnType::Text], + }, + )); + + // We'll skip testing for SizeOverflow as this would require producing + // a value which is at least 2GB in size. + + // Value overflow (type out of representable range) + let v = BigDecimal::new(BigInt::from(123), 1i64 << 40); + let err = do_serialize_err(v, &ColumnType::Decimal); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, ColumnType::Decimal); + assert!(matches!( + err.kind, + BuiltinSerializationErrorKind::ValueOverflow, + )); + } + + #[test] + fn test_set_or_list_errors() { + // Not a set or list + let v = vec![123_i32]; + let err = do_serialize_err(v, &ColumnType::Double); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + assert_eq!(err.got, ColumnType::Double); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::SetOrListError(SetOrListTypeCheckErrorKind::NotSetOrList), + )); + + // Trick: Unset is a ZST, so [Unset; 1 << 33] is a ZST, too. + // While it's probably incorrect to use Unset in a collection, this + // allows us to trigger the right error without going out of memory. + // Such an array is also created instantaneously. + let v = &[Unset; 1 << 33] as &[Unset]; + let typ = ColumnType::List(Box::new(ColumnType::Int)); + let err = do_serialize_err(v, &typ); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::<&[Unset]>()); + assert_eq!(err.got, typ); + assert!(matches!( + err.kind, + BuiltinSerializationErrorKind::SetOrListError( + SetOrListSerializationErrorKind::TooManyElements + ), + )); + + // Error during serialization of an element + let v = vec![123_i32]; + let typ = ColumnType::List(Box::new(ColumnType::Double)); + let err = do_serialize_err(v, &typ); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + assert_eq!(err.got, typ); + let BuiltinSerializationErrorKind::SetOrListError( + SetOrListSerializationErrorKind::ElementSerializationFailed(err), + ) = &err.kind + else { + panic!("unexpected error kind: {}", err.kind) + }; + let err = get_typeck_err(err); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MismatchedType { + expected: &[ColumnType::Int], + } + )); + } + + #[test] + fn test_map_errors() { + // Not a map + let v = BTreeMap::from([("foo", "bar")]); + let err = do_serialize_err(v, &ColumnType::Double); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + assert_eq!(err.got, ColumnType::Double); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MapError(MapTypeCheckErrorKind::NotMap), + )); + + // It's not practical to check the TooManyElements error as it would + // require allocating a huge amount of memory. + + // Error during serialization of a key + let v = BTreeMap::from([(123_i32, 456_i32)]); + let typ = ColumnType::Map(Box::new(ColumnType::Double), Box::new(ColumnType::Int)); + let err = do_serialize_err(v, &typ); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + assert_eq!(err.got, typ); + let BuiltinSerializationErrorKind::MapError( + MapSerializationErrorKind::KeySerializationFailed(err), + ) = &err.kind + else { + panic!("unexpected error kind: {}", err.kind) + }; + let err = get_typeck_err(err); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MismatchedType { + expected: &[ColumnType::Int], + } + )); + + // Error during serialization of a value + let v = BTreeMap::from([(123_i32, 456_i32)]); + let typ = ColumnType::Map(Box::new(ColumnType::Int), Box::new(ColumnType::Double)); + let err = do_serialize_err(v, &typ); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::>()); + assert_eq!(err.got, typ); + let BuiltinSerializationErrorKind::MapError( + MapSerializationErrorKind::ValueSerializationFailed(err), + ) = &err.kind + else { + panic!("unexpected error kind: {}", err.kind) + }; + let err = get_typeck_err(err); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MismatchedType { + expected: &[ColumnType::Int], + } + )); + } + + #[test] + fn test_tuple_errors() { + // Not a tuple + let v = (123_i32, 456_i32, 789_i32); + let err = do_serialize_err(v, &ColumnType::Double); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::<(i32, i32, i32)>()); + assert_eq!(err.got, ColumnType::Double); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::NotTuple), + )); + + // The Rust tuple has more elements than the CQL type + let v = (123_i32, 456_i32, 789_i32); + let typ = ColumnType::Tuple(vec![ColumnType::Int; 2]); + let err = do_serialize_err(v, &typ); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::<(i32, i32, i32)>()); + assert_eq!(err.got, typ); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::WrongElementCount { + actual: 3, + asked_for: 2, + }), + )); + + // Error during serialization of one of the elements + let v = (123_i32, "Ala ma kota", 789.0_f64); + let typ = ColumnType::Tuple(vec![ColumnType::Int, ColumnType::Text, ColumnType::Uuid]); + let err = do_serialize_err(v, &typ); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::<(i32, &str, f64)>()); + assert_eq!(err.got, typ); + let BuiltinSerializationErrorKind::TupleError( + TupleSerializationErrorKind::ElementSerializationFailed { index: 2, err }, + ) = &err.kind + else { + panic!("unexpected error kind: {}", err.kind) + }; + let err = get_typeck_err(err); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MismatchedType { + expected: &[ColumnType::Double], + } + )); + } + + #[test] + fn test_cql_value_errors() { + // Tried to encode Empty value into a non-emptyable type + let v = CqlValue::Empty; + let err = do_serialize_err(v, &ColumnType::Counter); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, ColumnType::Counter); + assert!(matches!(err.kind, BuiltinTypeCheckErrorKind::NotEmptyable)); + + // Handle tuples and UDTs in separate tests, as they have some + // custom logic + } + + #[test] + fn test_cql_value_tuple_errors() { + // Not a tuple + let v = CqlValue::Tuple(vec![ + Some(CqlValue::Int(123_i32)), + Some(CqlValue::Int(456_i32)), + Some(CqlValue::Int(789_i32)), + ]); + let err = do_serialize_err(v, &ColumnType::Double); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, ColumnType::Double); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::NotTuple), + )); + + // The Rust tuple has more elements than the CQL type + let v = CqlValue::Tuple(vec![ + Some(CqlValue::Int(123_i32)), + Some(CqlValue::Int(456_i32)), + Some(CqlValue::Int(789_i32)), + ]); + let typ = ColumnType::Tuple(vec![ColumnType::Int; 2]); + let err = do_serialize_err(v, &typ); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, typ); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::TupleError(TupleTypeCheckErrorKind::WrongElementCount { + actual: 3, + asked_for: 2, + }), + )); + + // Error during serialization of one of the elements + let v = CqlValue::Tuple(vec![ + Some(CqlValue::Int(123_i32)), + Some(CqlValue::Text("Ala ma kota".to_string())), + Some(CqlValue::Double(789_f64)), + ]); + let typ = ColumnType::Tuple(vec![ColumnType::Int, ColumnType::Text, ColumnType::Uuid]); + let err = do_serialize_err(v, &typ); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, typ); + let BuiltinSerializationErrorKind::TupleError( + TupleSerializationErrorKind::ElementSerializationFailed { index: 2, err }, + ) = &err.kind + else { + panic!("unexpected error kind: {}", err.kind) + }; + let err = get_typeck_err(err); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MismatchedType { + expected: &[ColumnType::Double], + } + )); + } + + #[test] + fn test_cql_value_udt_errors() { + // Not a UDT + let v = CqlValue::UserDefinedType { + keyspace: "ks".to_string(), + type_name: "udt".to_string(), + fields: vec![ + ("a".to_string(), Some(CqlValue::Int(123_i32))), + ("b".to_string(), Some(CqlValue::Int(456_i32))), + ("c".to_string(), Some(CqlValue::Int(789_i32))), + ], + }; + let err = do_serialize_err(v, &ColumnType::Double); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, ColumnType::Double); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NotUdt), + )); + + // Wrong type name + let v = CqlValue::UserDefinedType { + keyspace: "ks".to_string(), + type_name: "udt".to_string(), + fields: vec![ + ("a".to_string(), Some(CqlValue::Int(123_i32))), + ("b".to_string(), Some(CqlValue::Int(456_i32))), + ("c".to_string(), Some(CqlValue::Int(789_i32))), + ], + }; + let typ = ColumnType::UserDefinedType { + type_name: "udt2".to_string(), + keyspace: "ks".to_string(), + field_types: vec![ + ("a".to_string(), ColumnType::Int), + ("b".to_string(), ColumnType::Int), + ("c".to_string(), ColumnType::Int), + ], + }; + let err = do_serialize_err(v, &typ); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, typ); + let BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NameMismatch { + keyspace, + type_name, + }) = &err.kind + else { + panic!("unexpected error kind: {}", err.kind) + }; + assert_eq!(keyspace, "ks"); + assert_eq!(type_name, "udt2"); + + // Some fields are missing from the CQL type + let v = CqlValue::UserDefinedType { + keyspace: "ks".to_string(), + type_name: "udt".to_string(), + fields: vec![ + ("a".to_string(), Some(CqlValue::Int(123_i32))), + ("b".to_string(), Some(CqlValue::Int(456_i32))), + ("c".to_string(), Some(CqlValue::Int(789_i32))), + ], + }; + let typ = ColumnType::UserDefinedType { + type_name: "udt".to_string(), + keyspace: "ks".to_string(), + field_types: vec![ + ("a".to_string(), ColumnType::Int), + ("b".to_string(), ColumnType::Int), + // c is missing + ], + }; + let err = do_serialize_err(v, &typ); + let err = get_typeck_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, typ); + let BuiltinTypeCheckErrorKind::UdtError(UdtTypeCheckErrorKind::NoSuchFieldInUdt { + field_name, + }) = &err.kind + else { + panic!("unexpected error kind: {}", err.kind) + }; + assert_eq!(field_name, "c"); + + // It is allowed for a Rust UDT to have less fields than the CQL UDT, + // so skip UnexpectedFieldInDestination. + + // Error during serialization of one of the fields + let v = CqlValue::UserDefinedType { + keyspace: "ks".to_string(), + type_name: "udt".to_string(), + fields: vec![ + ("a".to_string(), Some(CqlValue::Int(123_i32))), + ("b".to_string(), Some(CqlValue::Int(456_i32))), + ("c".to_string(), Some(CqlValue::Int(789_i32))), + ], + }; + let typ = ColumnType::UserDefinedType { + type_name: "udt".to_string(), + keyspace: "ks".to_string(), + field_types: vec![ + ("a".to_string(), ColumnType::Int), + ("b".to_string(), ColumnType::Int), + ("c".to_string(), ColumnType::Double), + ], + }; + let err = do_serialize_err(v, &typ); + let err = get_ser_err(&err); + assert_eq!(err.rust_name, std::any::type_name::()); + assert_eq!(err.got, typ); + let BuiltinSerializationErrorKind::UdtError( + UdtSerializationErrorKind::FieldSerializationFailed { field_name, err }, + ) = &err.kind + else { + panic!("unexpected error kind: {}", err.kind) + }; + assert_eq!(field_name, "c"); + let err = get_typeck_err(err); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MismatchedType { + expected: &[ColumnType::Int], + } + )); + } + // Do not remove. It's not used in tests but we keep it here to check that // we properly ignore warnings about unused variables, unnecessary `mut`s // etc. that usually pop up when generating code for empty structs. From 7cf355b3e645faa1c8a970f4173358b894fb21f0 Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Mon, 11 Dec 2023 19:28:12 +0100 Subject: [PATCH 7/7] serialize: document all public items Put the `#![warn(missing_docs)]` attribute at the top of the `types::serialize` module to trigger warnings for undocumented public items, and then add docstrings to the items reported by the warnings. --- scylla-cql/src/types/serialize/mod.rs | 23 ++++++ scylla-cql/src/types/serialize/row.rs | 79 ++++++++++++++++++--- scylla-cql/src/types/serialize/value.rs | 94 ++++++++++++++++++++++--- 3 files changed, 178 insertions(+), 18 deletions(-) diff --git a/scylla-cql/src/types/serialize/mod.rs b/scylla-cql/src/types/serialize/mod.rs index 230462759d..b61541debf 100644 --- a/scylla-cql/src/types/serialize/mod.rs +++ b/scylla-cql/src/types/serialize/mod.rs @@ -1,3 +1,7 @@ +#![warn(missing_docs)] + +//! Types and traits related to serialization of values to the CQL format. + use std::{error::Error, fmt::Display, sync::Arc}; use thiserror::Error; @@ -7,6 +11,25 @@ pub mod value; pub mod writers; pub use writers::{CellValueBuilder, CellWriter, RowWriter}; + +/// An error indicating that a failure happened during serialization. +/// +/// The error is type-erased so that the crate users can define their own +/// serialization impls and their errors. As for the impls defined or generated +/// by the driver itself, the following errors can be returned: +/// +/// - [`row::BuiltinSerializationError`] is returned when serialization of +/// one of types with an impl built into the driver fails. It is also returned +/// from impls generated by the `SerializeRow` macro. +/// - [`value::BuiltinSerializationError`] is analogous to the above but is +/// returned from [`SerializeCql::serialize`](value::SerializeCql::serialize) +/// instead both in the case of builtin impls and impls generated by the +/// `SerializeCql` macro. It won't be returned by the `Session` directly, +/// but it might be nested in the [`row::BuiltinSerializationError`]. +/// - [`row::ValueListToSerializeRowAdapterError`] is returned in case when +/// a list of named values encoded with the legacy `ValueList` trait is passed +/// as an argument to the statement, and rewriting it using the new +/// `SerializeRow` interface fails. #[derive(Debug, Clone, Error)] pub struct SerializationError(Arc); diff --git a/scylla-cql/src/types/serialize/row.rs b/scylla-cql/src/types/serialize/row.rs index de9a45dc1d..edd0293cac 100644 --- a/scylla-cql/src/types/serialize/row.rs +++ b/scylla-cql/src/types/serialize/row.rs @@ -1,3 +1,5 @@ +//! Contains the [`SerializeRow`] trait and its implementations. + use std::borrow::Cow; use std::collections::{BTreeMap, HashSet}; use std::fmt::Display; @@ -24,6 +26,7 @@ pub struct RowSerializationContext<'a> { } impl<'a> RowSerializationContext<'a> { + /// Creates the serialization context from prepared statement metadata. #[inline] pub fn from_prepared(prepared: &'a PreparedMetadata) -> Self { Self { @@ -45,14 +48,30 @@ impl<'a> RowSerializationContext<'a> { } } +/// Represents a set of values that can be sent along a CQL statement. +/// +/// This is a low-level trait that is exposed to the specifics to the CQL +/// protocol and usually does not have to be implemented directly. See the +/// chapter on "Query Values" in the driver docs for information about how +/// this trait is supposed to be used. pub trait SerializeRow { /// Serializes the row according to the information in the given context. + /// + /// It's the trait's responsibility to produce values in the order as + /// specified in given serialization context. fn serialize( &self, ctx: &RowSerializationContext<'_>, writer: &mut RowWriter, ) -> Result<(), SerializationError>; + /// Returns whether this row contains any values or not. + /// + /// This method is used before executing a simple statement in order to check + /// whether there are any values provided to it. If there are some, then + /// the query will be prepared first in order to obtain information about + /// the bind marker types and names so that the values can be properly + /// type checked and serialized. fn is_empty(&self) -> bool; } @@ -424,7 +443,8 @@ pub fn serialize_legacy_row( RawValue::Null => cell_writer.set_null(), RawValue::Unset => cell_writer.set_unset(), // The unwrap below will succeed because the value was successfully - // deserialized from the CQL format, so it must have + // deserialized from the CQL format, so it must have had correct + // size. RawValue::Value(v) => cell_writer.set_value(v).unwrap(), }; }; @@ -527,19 +547,35 @@ fn mk_ser_err_named( #[derive(Debug, Clone)] #[non_exhaustive] pub enum BuiltinTypeCheckErrorKind { - /// The Rust type expects `asked_for` column, but the query requires `actual`. - WrongColumnCount { actual: usize, asked_for: usize }, + /// The Rust type expects `actual` column, but the statement requires `asked_for`. + WrongColumnCount { + /// The number of values that the Rust type provides. + actual: usize, + + /// The number of columns that the statement requires. + asked_for: usize, + }, /// The Rust type provides a value for some column, but that column is not /// present in the statement. - NoColumnWithName { name: String }, + NoColumnWithName { + /// Name of the column that is missing in the statement. + name: String, + }, /// A value required by the statement is not provided by the Rust type. - ValueMissingForColumn { name: String }, + ValueMissingForColumn { + /// Name of the column for which the Rust type doesn't + /// provide a value. + name: String, + }, /// A different column name was expected at given position. ColumnNameMismatch { + /// Name of the column, as expected by the Rust type. rust_column_name: String, + + /// Name of the column for which the DB requested a value. db_column_name: String, }, } @@ -576,7 +612,10 @@ impl Display for BuiltinTypeCheckErrorKind { pub enum BuiltinSerializationErrorKind { /// One of the columns failed to serialize. ColumnSerializationFailed { + /// Name of the column that failed to serialize. name: String, + + /// The error that caused the column serialization to fail. err: SerializationError, }, } @@ -591,13 +630,27 @@ impl Display for BuiltinSerializationErrorKind { } } +/// Describes a failure to translate the output of the [`ValueList`] legacy trait +/// into an output of the [`SerializeRow`] trait. #[derive(Error, Debug)] pub enum ValueListToSerializeRowAdapterError { + /// The values generated by the [`ValueList`] trait were provided in + /// name-value pairs, and there is a column in the statement for which + /// there is no corresponding named value. #[error("Missing named value for column {name}")] - ValueMissingForBindMarker { name: String }, + ValueMissingForBindMarker { + /// Name of the bind marker for which there is no value. + name: String, + }, + /// The values generated by the [`ValueList`] trait were provided in + /// name-value pairs, and there is a named value which does not match + /// to any of the columns. #[error("There is no bind marker with name {name}, but a value for it was provided")] - NoBindMarkerWithName { name: String }, + NoBindMarkerWithName { + /// Name of the value that does not match to any of the bind markers. + name: String, + }, } /// A buffer containing already serialized values. @@ -614,6 +667,7 @@ pub struct SerializedValues { } impl SerializedValues { + /// Constructs a new, empty `SerializedValues`. pub const fn new() -> Self { SerializedValues { serialized_values: Vec::new(), @@ -624,6 +678,7 @@ impl SerializedValues { /// A const empty instance, useful for taking references pub const EMPTY: &'static SerializedValues = &SerializedValues::new(); + /// Constructs `SerializedValues` from given [`SerializeRow`] object. pub fn from_serializable( ctx: &RowSerializationContext, row: &T, @@ -648,11 +703,13 @@ impl SerializedValues { }) } + /// Returns `true` if the row contains no elements. #[inline] pub fn is_empty(&self) -> bool { self.element_count() == 0 } + /// Returns an iterator over the values serialized into the object so far. #[inline] pub fn iter(&self) -> impl Iterator { SerializedValuesIterator { @@ -660,13 +717,13 @@ impl SerializedValues { } } + /// Returns the number of values written so far. #[inline] pub fn element_count(&self) -> u16 { - // We initialize first two bytes in new() and BufBackedRowWriter does too, - // so this unwrap is safe self.element_count } + /// Returns the total serialized size of the values written so far. #[inline] pub fn buffer_size(&self) -> usize { self.serialized_values.len() @@ -717,7 +774,8 @@ impl SerializedValues { }) } - // Temporary function, to be removed when we implement new batching API (right now it is needed in frame::request::mod.rs tests) + /// Temporary function, to be removed when we implement new batching API (right now it is needed in frame::request::mod.rs tests) + // TODO: Remove pub fn to_old_serialized_values(&self) -> LegacySerializedValues { let mut frame = Vec::new(); self.write_to_request(&mut frame); @@ -731,6 +789,7 @@ impl Default for SerializedValues { } } +/// An iterator over raw values in some [`SerializedValues`]. #[derive(Clone, Copy)] pub struct SerializedValuesIterator<'a> { serialized_values: &'a [u8], diff --git a/scylla-cql/src/types/serialize/value.rs b/scylla-cql/src/types/serialize/value.rs index b12416ba97..53061caa68 100644 --- a/scylla-cql/src/types/serialize/value.rs +++ b/scylla-cql/src/types/serialize/value.rs @@ -1,3 +1,5 @@ +//! Contains the [`SerializeCql`] trait and its implementations. + use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fmt::Display; use std::hash::BuildHasher; @@ -27,8 +29,28 @@ use crate::frame::value::ValueOverflow; use super::writers::WrittenCellProof; use super::{CellWriter, SerializationError}; +/// A type that can be serialized and sent along with a CQL statement. +/// +/// This is a low-level trait that is exposed to the specifics to the CQL +/// protocol and usually does not have to be implemented directly. See the +/// chapter on "Query Values" in the driver docs for information about how +/// this trait is supposed to be used. pub trait SerializeCql { /// Serializes the value to given CQL type. + /// + /// The value should produce a `[value]`, according to the [CQL protocol + /// specification](https://github.com/apache/cassandra/blob/trunk/doc/native_protocol_v4.spec), + /// containing the serialized value. See section 6 of the document on how + /// the contents of the `[value]` should look like. + /// + /// The value produced should match the type provided by `typ`. If the + /// value cannot be serialized to that type, an error should be returned. + /// + /// The [`CellWriter`] provided to the method ensures that the value produced + /// will be properly framed (i.e. incorrectly written value should not + /// cause the rest of the request to be misinterpreted), but otherwise + /// the implementor of the trait is responsible for producing the a value + /// in a correct format. fn serialize<'b>( &self, typ: &ColumnType, @@ -1014,7 +1036,10 @@ fn mk_ser_err_named( #[non_exhaustive] pub enum BuiltinTypeCheckErrorKind { /// Expected one from a list of particular types. - MismatchedType { expected: &'static [ColumnType] }, + MismatchedType { + /// The list of types that the Rust type can serialize as. + expected: &'static [ColumnType], + }, /// Expected a type that can be empty. NotEmptyable, @@ -1154,6 +1179,7 @@ impl Display for BuiltinSerializationErrorKind { } } +/// Describes why type checking of a map type failed. #[derive(Debug, Clone)] #[non_exhaustive] pub enum MapTypeCheckErrorKind { @@ -1174,6 +1200,7 @@ impl Display for MapTypeCheckErrorKind { } } +/// Describes why serialization of a map type failed. #[derive(Debug, Clone)] #[non_exhaustive] pub enum MapSerializationErrorKind { @@ -1206,6 +1233,7 @@ impl Display for MapSerializationErrorKind { } } +/// Describes why type checking of a set or list type failed. #[derive(Debug, Clone)] #[non_exhaustive] pub enum SetOrListTypeCheckErrorKind { @@ -1226,6 +1254,7 @@ impl Display for SetOrListTypeCheckErrorKind { } } +/// Describes why serialization of a set or list type failed. #[derive(Debug, Clone)] #[non_exhaustive] pub enum SetOrListSerializationErrorKind { @@ -1252,6 +1281,7 @@ impl Display for SetOrListSerializationErrorKind { } } +/// Describes why type checking of a tuple failed. #[derive(Debug, Clone)] #[non_exhaustive] pub enum TupleTypeCheckErrorKind { @@ -1263,7 +1293,13 @@ pub enum TupleTypeCheckErrorKind { /// Note that it is allowed to write a Rust tuple with less elements /// than the corresponding CQL type, but not more. The additional, unknown /// elements will be set to null. - WrongElementCount { actual: usize, asked_for: usize }, + WrongElementCount { + /// The number of elements that the Rust tuple has. + actual: usize, + + /// The number of elements that the CQL tuple type has. + asked_for: usize, + }, } impl Display for TupleTypeCheckErrorKind { @@ -1281,12 +1317,16 @@ impl Display for TupleTypeCheckErrorKind { } } +/// Describes why serialize of a tuple failed. #[derive(Debug, Clone)] #[non_exhaustive] pub enum TupleSerializationErrorKind { /// One of the tuple elements failed to serialize. ElementSerializationFailed { + /// Index of the tuple element that failed to serialize. index: usize, + + /// The error that caused the tuple field serialization to fail. err: SerializationError, }, } @@ -1301,6 +1341,7 @@ impl Display for TupleSerializationErrorKind { } } +/// Describes why type checking of a user defined type failed. #[derive(Debug, Clone)] #[non_exhaustive] pub enum UdtTypeCheckErrorKind { @@ -1308,17 +1349,32 @@ pub enum UdtTypeCheckErrorKind { NotUdt, /// The name of the UDT being serialized to does not match. - NameMismatch { keyspace: String, type_name: String }, + NameMismatch { + /// Keyspace in which the UDT was defined. + keyspace: String, + + /// Name of the UDT. + type_name: String, + }, /// The Rust data does not have a field that is required in the CQL UDT type. - ValueMissingForUdtField { field_name: String }, + ValueMissingForUdtField { + /// Name of field that the CQL UDT requires but is missing in the Rust struct. + field_name: String, + }, /// The Rust data contains a field that is not present in the UDT. - NoSuchFieldInUdt { field_name: String }, + NoSuchFieldInUdt { + /// Name of the Rust struct field that is missing in the UDT. + field_name: String, + }, /// A different field name was expected at given position. FieldNameMismatch { + /// The name of the Rust field. rust_field_name: String, + + /// The name of the CQL UDT field. db_field_name: String, }, } @@ -1352,12 +1408,16 @@ impl Display for UdtTypeCheckErrorKind { } } +/// Describes why serialization of a user defined type failed. #[derive(Debug, Clone)] #[non_exhaustive] pub enum UdtSerializationErrorKind { /// One of the fields failed to serialize. FieldSerializationFailed { + /// Name of the field which failed to serialize. field_name: String, + + /// The error that caused the UDT field serialization to fail. err: SerializationError, }, } @@ -1372,19 +1432,37 @@ impl Display for UdtSerializationErrorKind { } } +/// Describes a failure to translate the output of the [`Value`] legacy trait +/// into an output of the [`SerializeCql`] trait. #[derive(Error, Debug)] pub enum ValueToSerializeCqlAdapterError { + /// The value is too bit to be serialized as it exceeds the maximum 2GB size limit. #[error("The value is too big to be serialized as it exceeds the maximum 2GB size limit")] TooBig, + /// Output produced by the Value trait is less than 4 bytes in size and cannot be considered to be a proper CQL-encoded value. #[error("Output produced by the Value trait is too short to be considered a value: {size} < 4 minimum bytes")] - TooShort { size: usize }, + TooShort { + /// Size of the produced data. + size: usize, + }, + /// Mismatch between the value size written at the beginning and the actual size of the data appended to the Vec. #[error("Mismatch between the declared value size vs. actual size: {declared} != {actual}")] - DeclaredVsActualSizeMismatch { declared: usize, actual: usize }, + DeclaredVsActualSizeMismatch { + /// The declared size of the output. + declared: usize, + /// The actual size of the output. + actual: usize, + }, + + /// The value size written at the beginning is invalid (it is negative and less than -2). #[error("Invalid declared value size: {size}")] - InvalidDeclaredSize { size: i32 }, + InvalidDeclaredSize { + /// Declared size of the output. + size: i32, + }, } #[cfg(test)]