From a255d81aa3e0c5e2661755153022c535ed0d15ea Mon Sep 17 00:00:00 2001 From: Piotr Dulikowski Date: Fri, 27 Oct 2023 09:16:38 +0200 Subject: [PATCH] scylla-macros: implement enforce_order flavor of SerializeRow Like in the case of `SerializeRow`, some people might be used to working with the old `ValueList` and already order their Rust struct fields with accordance to the queries they are used with and don't need the overhead associated with looking up columns by name. The `enforce_order` mode is added to `SerializeRow` which works analogously as in `SerializeCql` - expects the columns to be in the correct order and verifies that this is the case when serializing, but just fails instead of reordering if that expectation is broken. --- scylla-cql/src/macros.rs | 19 ++++- scylla-cql/src/types/serialize/row.rs | 110 +++++++++++++++++++++++++ scylla-macros/src/serialize/row.rs | 113 +++++++++++++++++++++++++- 3 files changed, 237 insertions(+), 5 deletions(-) diff --git a/scylla-cql/src/macros.rs b/scylla-cql/src/macros.rs index 2b7b0b4ae7..51cc79ce24 100644 --- a/scylla-cql/src/macros.rs +++ b/scylla-cql/src/macros.rs @@ -91,9 +91,7 @@ pub use scylla_macros::SerializeCql; /// Derive macro for the [`SerializeRow`](crate::types::serialize::row::SerializeRow) trait /// which serializes given Rust structure into bind markers for a CQL statement. /// -/// At the moment, only structs with named fields are supported. The generated -/// implementation of the trait will match the struct fields to bind markers/columns -/// by name automatically. +/// At the moment, only structs with named fields are supported. /// /// Serialization will fail if there are some bind markers/columns in the statement /// that don't match to any of the Rust struct fields, _or vice versa_. @@ -127,6 +125,21 @@ pub use scylla_macros::SerializeCql; /// /// # Attributes /// +/// `#[scylla(flavor = "flavor_name")]` +/// +/// Allows to choose one of the possible "flavors", i.e. the way how the +/// generated code will approach serialization. Possible flavors are: +/// +/// - `"match_by_name"` (default) - the generated implementation _does not +/// require_ the fields in the Rust struct to be in the same order as the +/// columns/bind markers. During serialization, the implementation will take +/// care to serialize the fields in the order which the database expects. +/// - `"enforce_order"` - the generated implementation _requires_ the fields +/// in the Rust struct to be in the same order as the columns/bind markers. +/// If the order is incorrect, type checking/serialization will fail. +/// This is a less robust flavor than `"match_by_name"`, but should be +/// slightly more performant as it doesn't need to perform lookups by name. +/// /// `#[scylla(crate = crate_name)]` /// /// By default, the code generated by the derive macro will refer to the items diff --git a/scylla-cql/src/types/serialize/row.rs b/scylla-cql/src/types/serialize/row.rs index d398a42281..213af49c0f 100644 --- a/scylla-cql/src/types/serialize/row.rs +++ b/scylla-cql/src/types/serialize/row.rs @@ -504,6 +504,12 @@ pub enum BuiltinTypeCheckErrorKind { /// A value required by the statement is not provided by the Rust type. ColumnMissingForValue { name: String }, + + /// A different column name was expected at given position. + ColumnNameMismatch { + rust_column_name: String, + db_column_name: String, + }, } impl Display for BuiltinTypeCheckErrorKind { @@ -524,6 +530,10 @@ impl Display for BuiltinTypeCheckErrorKind { "value for column {name} was provided, but there is no bind marker for this column in the query" ) } + BuiltinTypeCheckErrorKind::ColumnNameMismatch { rust_column_name, db_column_name } => write!( + f, + "expected column with name {db_column_name} at given position, but the Rust field name is {rust_column_name}" + ), } } } @@ -838,4 +848,104 @@ mod tests { check_with_type(ColumnType::Int, 123_i32); check_with_type(ColumnType::Double, 123_f64); } + + #[derive(SerializeRow, Debug, PartialEq, Eq, Default)] + #[scylla(crate = crate, flavor = "enforce_order")] + struct TestRowWithEnforcedOrder { + a: String, + b: i32, + c: Vec, + } + + #[test] + fn test_row_serialization_with_enforced_order_correct_order() { + let spec = [ + col("a", ColumnType::Text), + col("b", ColumnType::Int), + col("c", ColumnType::List(Box::new(ColumnType::BigInt))), + ]; + + let reference = do_serialize(("Ala ma kota", 42i32, vec![1i64, 2i64, 3i64]), &spec); + let row = do_serialize( + TestRowWithEnforcedOrder { + a: "Ala ma kota".to_owned(), + b: 42, + c: vec![1, 2, 3], + }, + &spec, + ); + + assert_eq!(reference, row); + } + + #[test] + fn test_row_serialization_with_enforced_order_failing_type_check() { + let row = TestRowWithEnforcedOrder::default(); + let mut data = Vec::new(); + let mut writer = RowWriter::new(&mut data); + + // The order of two last columns is swapped + let spec = [ + col("a", ColumnType::Text), + col("c", ColumnType::List(Box::new(ColumnType::BigInt))), + col("b", ColumnType::Int), + ]; + let ctx = RowSerializationContext { columns: &spec }; + let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err(); + let err = err.0.downcast_ref::().unwrap(); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::ColumnNameMismatch { .. } + )); + + let spec_without_c = [ + col("a", ColumnType::Text), + col("b", ColumnType::Int), + // Missing column c + ]; + + let ctx = RowSerializationContext { + columns: &spec_without_c, + }; + let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err(); + let err = err.0.downcast_ref::().unwrap(); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::ColumnMissingForValue { .. } + )); + + let spec_duplicate_column = [ + col("a", ColumnType::Text), + col("b", ColumnType::Int), + col("c", ColumnType::List(Box::new(ColumnType::BigInt))), + // Unexpected last column + col("d", ColumnType::Counter), + ]; + + let ctx = RowSerializationContext { + columns: &spec_duplicate_column, + }; + let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err(); + let err = err.0.downcast_ref::().unwrap(); + assert!(matches!( + err.kind, + BuiltinTypeCheckErrorKind::MissingValueForColumn { .. } + )); + + let spec_wrong_type = [ + col("a", ColumnType::Text), + col("b", ColumnType::Int), + col("c", ColumnType::TinyInt), // Wrong type + ]; + + let ctx = RowSerializationContext { + columns: &spec_wrong_type, + }; + let err = <_ as SerializeRow>::serialize(&row, &ctx, &mut writer).unwrap_err(); + let err = err.0.downcast_ref::().unwrap(); + assert!(matches!( + err.kind, + BuiltinSerializationErrorKind::ColumnSerializationFailed { .. } + )); + } } diff --git a/scylla-macros/src/serialize/row.rs b/scylla-macros/src/serialize/row.rs index 0dd2356041..ee0f702d27 100644 --- a/scylla-macros/src/serialize/row.rs +++ b/scylla-macros/src/serialize/row.rs @@ -3,11 +3,15 @@ use proc_macro::TokenStream; use proc_macro2::Span; use syn::parse_quote; +use super::Flavor; + #[derive(FromAttributes)] #[darling(attributes(scylla))] struct Attributes { #[darling(rename = "crate")] crate_path: Option, + + flavor: Option, } impl Attributes { @@ -36,7 +40,11 @@ pub fn derive_serialize_row(tokens_input: TokenStream) -> Result = match ctx.attributes.flavor { + Some(Flavor::MatchByName) | None => Box::new(ColumnSortingGenerator { ctx: &ctx }), + Some(Flavor::EnforceOrder) => Box::new(ColumnOrderedGenerator { ctx: &ctx }), + }; let serialize_item = gen.generate_serialize(); let is_empty_item = gen.generate_is_empty(); @@ -80,13 +88,18 @@ impl Context { } } +trait Generator { + fn generate_serialize(&self) -> syn::TraitItemFn; + fn generate_is_empty(&self) -> syn::TraitItemFn; +} + // Generates an implementation of the trait which sorts the columns according // to how they are defined in prepared statement metadata. struct ColumnSortingGenerator<'a> { ctx: &'a Context, } -impl<'a> ColumnSortingGenerator<'a> { +impl<'a> Generator for ColumnSortingGenerator<'a> { fn generate_serialize(&self) -> syn::TraitItemFn { // Need to: // - Check that all required columns are there and no more @@ -200,3 +213,99 @@ impl<'a> ColumnSortingGenerator<'a> { } } } + +// Generates an implementation of the trait which requires the columns +// to be placed in the same order as they are defined in the struct. +struct ColumnOrderedGenerator<'a> { + ctx: &'a Context, +} + +impl<'a> Generator for ColumnOrderedGenerator<'a> { + fn generate_serialize(&self) -> syn::TraitItemFn { + let mut statements: Vec = Vec::new(); + + let crate_path = self.ctx.attributes.crate_path(); + + // Declare a helper lambda for creating errors + statements.push(self.ctx.generate_mk_typck_err()); + statements.push(self.ctx.generate_mk_ser_err()); + + // Create an iterator over fields + statements.push(parse_quote! { + let mut column_iter = ctx.columns().iter(); + }); + + // Serialize each field + for field in self.ctx.fields.iter() { + let rust_field_ident = field.ident.as_ref().unwrap(); + let rust_field_name = rust_field_ident.to_string(); + let typ = &field.ty; + statements.push(parse_quote! { + match column_iter.next() { + Some(spec) => { + if spec.name == #rust_field_name { + let cell_writer = #crate_path::RowWriter::make_cell_writer(writer); + match <#typ as #crate_path::SerializeCql>::serialize(&self.#rust_field_ident, &spec.typ, cell_writer) { + Ok(_proof) => {}, + Err(err) => { + return ::std::result::Result::Err(mk_ser_err( + #crate_path::BuiltinRowSerializationErrorKind::ColumnSerializationFailed { + name: <_ as ::std::clone::Clone>::clone(&spec.name), + err, + } + )); + } + } + } else { + return ::std::result::Result::Err(mk_typck_err( + #crate_path::BuiltinRowTypeCheckErrorKind::ColumnNameMismatch { + rust_column_name: <_ as ::std::string::ToString>::to_string(#rust_field_name), + db_column_name: <_ as ::std::clone::Clone>::clone(&spec.name), + } + )); + } + } + None => { + return ::std::result::Result::Err(mk_typck_err( + #crate_path::BuiltinRowTypeCheckErrorKind::ColumnMissingForValue { + name: <_ as ::std::string::ToString>::to_string(#rust_field_name), + } + )); + } + } + }); + } + + // Check whether there are some columns remaining + statements.push(parse_quote! { + if let Some(spec) = column_iter.next() { + return ::std::result::Result::Err(mk_typck_err( + #crate_path::BuiltinRowTypeCheckErrorKind::MissingValueForColumn { + name: <_ as ::std::clone::Clone>::clone(&spec.name), + } + )); + } + }); + + parse_quote! { + fn serialize<'b>( + &self, + ctx: &#crate_path::RowSerializationContext, + writer: &mut #crate_path::RowWriter<'b>, + ) -> ::std::result::Result<(), #crate_path::SerializationError> { + #(#statements)* + ::std::result::Result::Ok(()) + } + } + } + + fn generate_is_empty(&self) -> syn::TraitItemFn { + let is_empty = self.ctx.fields.is_empty(); + parse_quote! { + #[inline] + fn is_empty(&self) -> bool { + #is_empty + } + } + } +}