Skip to content

Commit

Permalink
Merge pull request scylladb#1004 from wprzytula/pre-deserialization-c…
Browse files Browse the repository at this point in the history 
…hanges

Pre-deserialization-refactor preparatory changes
  • Loading branch information
@wprzytula
wprzytula authored May 21, 2024
2 parents e6d8625 + c00ae72 commit 5dbb10b
Show file tree
Hide file tree
Showing 11 changed files with 221 additions and 200 deletions.
1 change: 1 addition & 0 deletions Cargo.lock.msrv
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 10 additions & 2 deletions scylla-cql/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }

Expand All @@ -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"
30 changes: 18 additions & 12 deletions scylla-cql/src/frame/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
Expand Down Expand Up @@ -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),
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand All @@ -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)
}

Expand Down
6 changes: 4 additions & 2 deletions scylla-cql/src/frame/value_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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());
}
Expand All @@ -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());
}
Expand Down
79 changes: 37 additions & 42 deletions scylla-cql/src/types/serialize/row.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
},
));
}
Expand Down Expand Up @@ -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(),
},
));
}
Expand Down Expand Up @@ -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(),
},
)),
};
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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"];
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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)]
Expand Down Expand Up @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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]
Expand Down
Loading

0 comments on commit 5dbb10b

Please sign in to comment.