Merge pull request #1004 from wprzytula/pre-deserialization-changes

Pre-deserialization-refactor preparatory changes

Author: wprzytula

Cargo.lock.msrv

@@ -28,7 +28,8 @@ serde = { version = "1.0", features = ["derive"], optional = true }
 time = { version = "0.3", optional = true }
 
 [dev-dependencies]
-criterion = "0.4" # Note: v0.5 needs at least rust 1.70.0
+assert_matches = "1.5.0"
+criterion = "0.4"        # Note: v0.5 needs at least rust 1.70.0
 # Use large-dates feature to test potential edge cases
 time = { version = "0.3.21", features = ["large-dates"] }
 
@@ -43,7 +44,14 @@ chrono = ["dep:chrono"]
 num-bigint-03 = ["dep:num-bigint-03"]
 num-bigint-04 = ["dep:num-bigint-04"]
 bigdecimal-04 = ["dep:bigdecimal-04"]
-full-serialization = ["chrono", "time", "secret", "num-bigint-03", "num-bigint-04", "bigdecimal-04"]
+full-serialization = [
+    "chrono",
+    "time",
+    "secret",
+    "num-bigint-03",
+    "num-bigint-04",
+    "bigdecimal-04",
+]
 
 [lints.rust]
 unreachable_pub = "warn"

scylla-cql/Cargo.toml

@@ -173,7 +173,7 @@ fn read_raw_bytes<'a>(count: usize, buf: &mut &'a [u8]) -> Result<&'a [u8], Pars
     Ok(ret)
 }
 
-pub fn read_int(buf: &mut &[u8]) -> Result<i32, ParseError> {
+pub fn read_int(buf: &mut &[u8]) -> Result<i32, std::io::Error> {
     let v = buf.read_i32::<BigEndian>()?;
     Ok(v)
 }
@@ -206,7 +206,7 @@ fn type_int() {
     }
 }
 
-pub fn read_long(buf: &mut &[u8]) -> Result<i64, ParseError> {
+pub fn read_long(buf: &mut &[u8]) -> Result<i64, std::io::Error> {
     let v = buf.read_i64::<BigEndian>()?;
     Ok(v)
 }
@@ -225,7 +225,7 @@ fn type_long() {
     }
 }
 
-pub fn read_short(buf: &mut &[u8]) -> Result<u16, ParseError> {
+pub fn read_short(buf: &mut &[u8]) -> Result<u16, std::io::Error> {
     let v = buf.read_u16::<BigEndian>()?;
     Ok(v)
 }
@@ -234,7 +234,7 @@ pub fn write_short(v: u16, buf: &mut impl BufMut) {
     buf.put_u16(v);
 }
 
-pub(crate) fn read_short_length(buf: &mut &[u8]) -> Result<usize, ParseError> {
+pub(crate) fn read_short_length(buf: &mut &[u8]) -> Result<usize, std::io::Error> {
     let v = read_short(buf)?;
     let v: usize = v.into();
     Ok(v)
@@ -302,11 +302,14 @@ pub fn write_bytes(v: &[u8], buf: &mut impl BufMut) -> Result<(), ParseError> {
     Ok(())
 }
 
-pub fn write_bytes_opt(v: Option<&Vec<u8>>, buf: &mut impl BufMut) -> Result<(), ParseError> {
+pub fn write_bytes_opt(
+    v: Option<impl AsRef<[u8]>>,
+    buf: &mut impl BufMut,
+) -> Result<(), ParseError> {
     match v {
         Some(bytes) => {
-            write_int_length(bytes.len(), buf)?;
-            buf.put_slice(bytes);
+            write_int_length(bytes.as_ref().len(), buf)?;
+            buf.put_slice(bytes.as_ref());
         }
         None => write_int(-1, buf),
     }
@@ -511,9 +514,12 @@ fn type_string_multimap() {
 pub fn read_uuid(buf: &mut &[u8]) -> Result<Uuid, ParseError> {
     let raw = read_raw_bytes(16, buf)?;
 
-    // It's safe to unwrap here because Uuid::from_slice only fails
-    // if the argument slice's length is not 16.
-    Ok(Uuid::from_slice(raw).unwrap())
+    // It's safe to unwrap here because the conversion only fails
+    // if the argument slice's length does not match, which
+    // `read_raw_bytes` prevents.
+    let raw_array: &[u8; 16] = raw.try_into().unwrap();
+
+    Ok(Uuid::from_bytes(*raw_array))
 }
 
 pub fn write_uuid(uuid: &Uuid, buf: &mut impl BufMut) {
@@ -646,7 +652,7 @@ fn unsigned_vint_encode(v: u64, buf: &mut Vec<u8>) {
     buf.put_uint(v, number_of_bytes as usize)
 }
 
-fn unsigned_vint_decode(buf: &mut &[u8]) -> Result<u64, ParseError> {
+fn unsigned_vint_decode(buf: &mut &[u8]) -> Result<u64, std::io::Error> {
     let first_byte = buf.read_u8()?;
     let extra_bytes = first_byte.leading_ones() as usize;
 
@@ -668,7 +674,7 @@ pub(crate) fn vint_encode(v: i64, buf: &mut Vec<u8>) {
     unsigned_vint_encode(zig_zag_encode(v), buf)
 }
 
-pub(crate) fn vint_decode(buf: &mut &[u8]) -> Result<i64, ParseError> {
+pub(crate) fn vint_decode(buf: &mut &[u8]) -> Result<i64, std::io::Error> {
     unsigned_vint_decode(buf).map(zig_zag_decode)
 }
 

scylla-cql/src/frame/types.rs

@@ -10,6 +10,8 @@ use super::value::{
     CqlDate, CqlDuration, CqlTime, CqlTimestamp, LegacyBatchValues, LegacySerializedValues,
     MaybeUnset, SerializeValuesError, Unset, Value, ValueList, ValueTooBig,
 };
+#[cfg(test)]
+use assert_matches::assert_matches;
 use bytes::BufMut;
 use std::collections::hash_map::DefaultHasher;
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
@@ -1262,7 +1264,7 @@ fn serialized_values_value_list() {
     ser_values.add_value(&"qwertyuiop").unwrap();
 
     let ser_ser_values: Cow<LegacySerializedValues> = ser_values.serialized().unwrap();
-    assert!(matches!(ser_ser_values, Cow::Borrowed(_)));
+    assert_matches!(ser_ser_values, Cow::Borrowed(_));
 
     assert_eq!(&ser_values, ser_ser_values.as_ref());
 }
@@ -1272,7 +1274,7 @@ fn cow_serialized_values_value_list() {
     let cow_ser_values: Cow<LegacySerializedValues> = Cow::Owned(LegacySerializedValues::new());
 
     let serialized: Cow<LegacySerializedValues> = cow_ser_values.serialized().unwrap();
-    assert!(matches!(serialized, Cow::Borrowed(_)));
+    assert_matches!(serialized, Cow::Borrowed(_));
 
     assert_eq!(cow_ser_values.as_ref(), serialized.as_ref());
 }

scylla-cql/src/frame/value_tests.rs

@@ -108,8 +108,8 @@ macro_rules! impl_serialize_row_for_unit {
             if !ctx.columns().is_empty() {
                 return Err(mk_typck_err::<Self>(
                     BuiltinTypeCheckErrorKind::WrongColumnCount {
-                        actual: 0,
-                        asked_for: ctx.columns().len(),
+                        rust_cols: 0,
+                        cql_cols: ctx.columns().len(),
                     },
                 ));
             }
@@ -142,8 +142,8 @@ macro_rules! impl_serialize_row_for_slice {
             if ctx.columns().len() != self.len() {
                 return Err(mk_typck_err::<Self>(
                     BuiltinTypeCheckErrorKind::WrongColumnCount {
-                        actual: self.len(),
-                        asked_for: ctx.columns().len(),
+                        rust_cols: self.len(),
+                        cql_cols: ctx.columns().len(),
                     },
                 ));
             }
@@ -289,8 +289,8 @@ macro_rules! impl_tuple {
                     [$($tidents),*] => ($($tidents,)*),
                     _ => return Err(mk_typck_err::<Self>(
                         BuiltinTypeCheckErrorKind::WrongColumnCount {
-                            actual: $length,
-                            asked_for: ctx.columns().len(),
+                            rust_cols: $length,
+                            cql_cols: ctx.columns().len(),
                         },
                     )),
                 };
@@ -582,13 +582,13 @@ fn mk_ser_err_named(
 #[derive(Debug, Clone)]
 #[non_exhaustive]
 pub enum BuiltinTypeCheckErrorKind {
-    /// The Rust type expects `actual` column, but the statement requires `asked_for`.
+    /// The Rust type provides `rust_cols` columns, but the statement operates on `cql_cols`.
     WrongColumnCount {
         /// The number of values that the Rust type provides.
-        actual: usize,
+        rust_cols: usize,
 
-        /// The number of columns that the statement requires.
-        asked_for: usize,
+        /// The number of columns that the statement operates on.
+        cql_cols: usize,
     },
 
     /// The Rust type provides a value for some column, but that column is not
@@ -618,8 +618,8 @@ pub enum BuiltinTypeCheckErrorKind {
 impl Display for BuiltinTypeCheckErrorKind {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            BuiltinTypeCheckErrorKind::WrongColumnCount { actual, asked_for } => {
-                write!(f, "wrong column count: the query requires {asked_for} columns, but {actual} were provided")
+            BuiltinTypeCheckErrorKind::WrongColumnCount { rust_cols, cql_cols } => {
+                write!(f, "wrong column count: the statement operates on {cql_cols} columns, but the given rust type provides {rust_cols}")
             }
             BuiltinTypeCheckErrorKind::NoColumnWithName { name } => {
                 write!(
@@ -865,6 +865,7 @@ mod tests {
     };
 
     use super::SerializedValues;
+    use assert_matches::assert_matches;
     use scylla_macros::SerializeRow;
 
     fn col_spec(name: &str, typ: ColumnType) -> ColumnSpec {
@@ -1044,13 +1045,13 @@ mod tests {
         let err = do_serialize_err(v, &spec);
         let err = get_typeck_err(&err);
         assert_eq!(err.rust_name, std::any::type_name::<()>());
-        assert!(matches!(
+        assert_matches!(
             err.kind,
             BuiltinTypeCheckErrorKind::WrongColumnCount {
-                actual: 0,
-                asked_for: 1,
+                rust_cols: 0,
+                cql_cols: 1,
             }
-        ));
+        );
 
         // Non-unit tuple
         // Count mismatch
@@ -1059,13 +1060,13 @@ mod tests {
         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!(
+        assert_matches!(
             err.kind,
             BuiltinTypeCheckErrorKind::WrongColumnCount {
-                actual: 1,
-                asked_for: 2,
+                rust_cols: 1,
+                cql_cols: 2,
             }
-        ));
+        );
 
         // Serialization of one of the element fails
         let v = ("Ala ma kota", 123_i32);
@@ -1086,13 +1087,13 @@ mod tests {
         let err = do_serialize_err(v, &spec);
         let err = get_typeck_err(&err);
         assert_eq!(err.rust_name, std::any::type_name::<Vec<&str>>());
-        assert!(matches!(
+        assert_matches!(
             err.kind,
             BuiltinTypeCheckErrorKind::WrongColumnCount {
-                actual: 1,
-                asked_for: 2,
+                rust_cols: 1,
+                cql_cols: 2,
             }
-        ));
+        );
 
         // Serialization of one of the element fails
         let v = vec!["Ala ma kota", "Kot ma pchły"];
@@ -1214,10 +1215,10 @@ mod tests {
         };
         let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut row_writer).unwrap_err();
         let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
-        assert!(matches!(
+        assert_matches!(
             err.kind,
             BuiltinTypeCheckErrorKind::ValueMissingForColumn { .. }
-        ));
+        );
 
         let spec_duplicate_column = [
             col("a", ColumnType::Text),
@@ -1232,10 +1233,7 @@ mod tests {
         };
         let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut row_writer).unwrap_err();
         let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
-        assert!(matches!(
-            err.kind,
-            BuiltinTypeCheckErrorKind::NoColumnWithName { .. }
-        ));
+        assert_matches!(err.kind, BuiltinTypeCheckErrorKind::NoColumnWithName { .. });
 
         let spec_wrong_type = [
             col("a", ColumnType::Text),
@@ -1248,10 +1246,10 @@ mod tests {
         };
         let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut row_writer).unwrap_err();
         let err = err.0.downcast_ref::<BuiltinSerializationError>().unwrap();
-        assert!(matches!(
+        assert_matches!(
             err.kind,
             BuiltinSerializationErrorKind::ColumnSerializationFailed { .. }
-        ));
+        );
     }
 
     #[derive(SerializeRow)]
@@ -1325,10 +1323,10 @@ mod tests {
         let ctx = RowSerializationContext { columns: &spec };
         let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err();
         let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
-        assert!(matches!(
+        assert_matches!(
             err.kind,
             BuiltinTypeCheckErrorKind::ColumnNameMismatch { .. }
-        ));
+        );
 
         let spec_without_c = [
             col("a", ColumnType::Text),
@@ -1341,10 +1339,10 @@ mod tests {
         };
         let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err();
         let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
-        assert!(matches!(
+        assert_matches!(
             err.kind,
             BuiltinTypeCheckErrorKind::ValueMissingForColumn { .. }
-        ));
+        );
 
         let spec_duplicate_column = [
             col("a", ColumnType::Text),
@@ -1359,10 +1357,7 @@ mod tests {
         };
         let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err();
         let err = err.0.downcast_ref::<BuiltinTypeCheckError>().unwrap();
-        assert!(matches!(
-            err.kind,
-            BuiltinTypeCheckErrorKind::NoColumnWithName { .. }
-        ));
+        assert_matches!(err.kind, BuiltinTypeCheckErrorKind::NoColumnWithName { .. });
 
         let spec_wrong_type = [
             col("a", ColumnType::Text),
@@ -1375,10 +1370,10 @@ mod tests {
         };
         let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err();
         let err = err.0.downcast_ref::<BuiltinSerializationError>().unwrap();
-        assert!(matches!(
+        assert_matches!(
             err.kind,
             BuiltinSerializationErrorKind::ColumnSerializationFailed { .. }
-        ));
+        );
     }
 
     #[test]