From 6172124b01dcd24febc28d3cbc312f5e1d19203d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Mon, 28 Oct 2024 09:59:17 +0100 Subject: [PATCH 1/2] response/result.rs: Replace generate_deser_type macro with a function This macro is unnecessary, function is enough here. For ease of future development, lets get rid of such macros. --- scylla-cql/src/frame/response/result.rs | 187 ++++++++++++------------ 1 file changed, 96 insertions(+), 91 deletions(-) diff --git a/scylla-cql/src/frame/response/result.rs b/scylla-cql/src/frame/response/result.rs index ab85cfa3c..a4c66bf23 100644 --- a/scylla-cql/src/frame/response/result.rs +++ b/scylla-cql/src/frame/response/result.rs @@ -1,8 +1,8 @@ use crate::cql_to_rust::{FromRow, FromRowError}; use crate::frame::frame_errors::{ ColumnSpecParseError, ColumnSpecParseErrorKind, CqlResultParseError, CqlTypeParseError, - PreparedParseError, ResultMetadataParseError, RowsParseError, SchemaChangeEventParseError, - SetKeyspaceParseError, TableSpecParseError, + LowLevelDeserializationError, PreparedParseError, ResultMetadataParseError, RowsParseError, + SchemaChangeEventParseError, SetKeyspaceParseError, TableSpecParseError, }; use crate::frame::request::query::PagingStateResponse; use crate::frame::response::event::SchemaChangeEvent; @@ -603,99 +603,104 @@ pub enum Result { SchemaChange(SchemaChange), } -macro_rules! generate_deser_type { - ($deser_type: ident, $l: lifetime, $read_string: expr) => { - fn $deser_type<'frame>( - buf: &mut &'frame [u8], - ) -> StdResult, CqlTypeParseError> { - use ColumnType::*; - let id = types::read_short(buf) - .map_err(|err| CqlTypeParseError::TypeIdParseError(err.into()))?; - Ok(match id { - 0x0000 => { - // We use types::read_string instead of $read_string here on purpose. - // Chances are the underlying string is `...DurationType`, in which case - // we don't need to allocate it at all. Only for Custom types - // (which we don't support anyway) do we need to allocate. - // OTOH, the macro argument function deserializes borrowed OR owned string; - // here we want to always deserialize borrowed string. - let type_str = types::read_string(buf) - .map_err(CqlTypeParseError::CustomTypeNameParseError)?; - match type_str { - "org.apache.cassandra.db.marshal.DurationType" => Duration, - _ => Custom(type_str.to_owned().into()), - } - } - 0x0001 => Ascii, - 0x0002 => BigInt, - 0x0003 => Blob, - 0x0004 => Boolean, - 0x0005 => Counter, - 0x0006 => Decimal, - 0x0007 => Double, - 0x0008 => Float, - 0x0009 => Int, - 0x000B => Timestamp, - 0x000C => Uuid, - 0x000D => Text, - 0x000E => Varint, - 0x000F => Timeuuid, - 0x0010 => Inet, - 0x0011 => Date, - 0x0012 => Time, - 0x0013 => SmallInt, - 0x0014 => TinyInt, - 0x0015 => Duration, - 0x0020 => List(Box::new($deser_type(buf)?)), - 0x0021 => Map(Box::new($deser_type(buf)?), Box::new($deser_type(buf)?)), - 0x0022 => Set(Box::new($deser_type(buf)?)), - 0x0030 => { - let keyspace_name = - $read_string(buf).map_err(CqlTypeParseError::UdtKeyspaceNameParseError)?; - let type_name = - $read_string(buf).map_err(CqlTypeParseError::UdtNameParseError)?; - let fields_size: usize = types::read_short(buf) - .map_err(|err| CqlTypeParseError::UdtFieldsCountParseError(err.into()))? - .into(); - - let mut field_types: Vec<(Cow<$l, str>, ColumnType)> = - Vec::with_capacity(fields_size); - - for _ in 0..fields_size { - let field_name = - $read_string(buf).map_err(CqlTypeParseError::UdtFieldNameParseError)?; - let field_type = $deser_type(buf)?; - - field_types.push((field_name.into(), field_type)); - } - - UserDefinedType { - type_name: type_name.into(), - keyspace: keyspace_name.into(), - field_types, - } - } - 0x0031 => { - let len: usize = types::read_short(buf) - .map_err(|err| CqlTypeParseError::TupleLengthParseError(err.into()))? - .into(); - let mut types = Vec::with_capacity(len); - for _ in 0..len { - types.push($deser_type(buf)?); - } - Tuple(types) - } - id => { - return Err(CqlTypeParseError::TypeNotImplemented(id)); - } - }) +fn deser_type_generic<'frame, 'result, StrT: Into>>( + buf: &mut &'frame [u8], + read_string: fn(&mut &'frame [u8]) -> StdResult, +) -> StdResult, CqlTypeParseError> { + use ColumnType::*; + let id = + types::read_short(buf).map_err(|err| CqlTypeParseError::TypeIdParseError(err.into()))?; + Ok(match id { + 0x0000 => { + // We use types::read_string instead of read_string argument here on purpose. + // Chances are the underlying string is `...DurationType`, in which case + // we don't need to allocate it at all. Only for Custom types + // (which we don't support anyway) do we need to allocate. + // OTOH, the macro argument function deserializes borrowed OR owned string; + // here we want to always deserialize borrowed string. + let type_str = + types::read_string(buf).map_err(CqlTypeParseError::CustomTypeNameParseError)?; + match type_str { + "org.apache.cassandra.db.marshal.DurationType" => Duration, + _ => Custom(type_str.to_owned().into()), + } } - }; + 0x0001 => Ascii, + 0x0002 => BigInt, + 0x0003 => Blob, + 0x0004 => Boolean, + 0x0005 => Counter, + 0x0006 => Decimal, + 0x0007 => Double, + 0x0008 => Float, + 0x0009 => Int, + 0x000B => Timestamp, + 0x000C => Uuid, + 0x000D => Text, + 0x000E => Varint, + 0x000F => Timeuuid, + 0x0010 => Inet, + 0x0011 => Date, + 0x0012 => Time, + 0x0013 => SmallInt, + 0x0014 => TinyInt, + 0x0015 => Duration, + 0x0020 => List(Box::new(deser_type_generic(buf, read_string)?)), + 0x0021 => Map( + Box::new(deser_type_generic(buf, read_string)?), + Box::new(deser_type_generic(buf, read_string)?), + ), + 0x0022 => Set(Box::new(deser_type_generic(buf, read_string)?)), + 0x0030 => { + let keyspace_name = + read_string(buf).map_err(CqlTypeParseError::UdtKeyspaceNameParseError)?; + let type_name = read_string(buf).map_err(CqlTypeParseError::UdtNameParseError)?; + let fields_size: usize = types::read_short(buf) + .map_err(|err| CqlTypeParseError::UdtFieldsCountParseError(err.into()))? + .into(); + + let mut field_types: Vec<(Cow<'result, str>, ColumnType)> = + Vec::with_capacity(fields_size); + + for _ in 0..fields_size { + let field_name = + read_string(buf).map_err(CqlTypeParseError::UdtFieldNameParseError)?; + let field_type = deser_type_generic(buf, read_string)?; + + field_types.push((field_name.into(), field_type)); + } + + UserDefinedType { + type_name: type_name.into(), + keyspace: keyspace_name.into(), + field_types, + } + } + 0x0031 => { + let len: usize = types::read_short(buf) + .map_err(|err| CqlTypeParseError::TupleLengthParseError(err.into()))? + .into(); + let mut types = Vec::with_capacity(len); + for _ in 0..len { + types.push(deser_type_generic(buf, read_string)?); + } + Tuple(types) + } + id => { + return Err(CqlTypeParseError::TypeNotImplemented(id)); + } + }) } -generate_deser_type!(_deser_type_borrowed, 'frame, types::read_string); +fn _deser_type_borrowed<'frame>( + buf: &mut &'frame [u8], +) -> StdResult, CqlTypeParseError> { + deser_type_generic(buf, |buf| types::read_string(buf)) +} -generate_deser_type!(deser_type_owned, 'static, |buf| types::read_string(buf).map(ToOwned::to_owned)); +fn deser_type_owned(buf: &mut &[u8]) -> StdResult, CqlTypeParseError> { + deser_type_generic(buf, |buf| types::read_string(buf).map(ToOwned::to_owned)) +} /// Deserializes a table spec, be it per-column one or a global one, /// in the borrowed form. From b1b3b81cf4de90529a65285af907d3d7e3151cb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20Bary=C5=82a?= Date: Fri, 25 Oct 2024 15:39:10 +0200 Subject: [PATCH 2/2] response/result.rs: Replace generate_deser_col_specs macro with a function This macro is unnecessary, function is enough here. For ease of future development, lets get rid of such macros. --- scylla-cql/src/frame/response/result.rs | 115 +++++++++++++----------- 1 file changed, 65 insertions(+), 50 deletions(-) diff --git a/scylla-cql/src/frame/response/result.rs b/scylla-cql/src/frame/response/result.rs index a4c66bf23..d5627a305 100644 --- a/scylla-cql/src/frame/response/result.rs +++ b/scylla-cql/src/frame/response/result.rs @@ -780,58 +780,73 @@ fn deser_table_spec_for_col_spec<'frame>( Ok(table_spec) } -macro_rules! generate_deser_col_specs { - ($deser_col_specs: ident, $l: lifetime, $deser_type: ident, $make_col_spec: expr $(,)?) => { - /// Deserializes col specs (part of ResultMetadata or PreparedMetadata) - /// in the form mentioned by its name. - /// - /// Checks for equality of table specs across columns, because the protocol - /// does not guarantee that and we want to be sure that the assumption - /// of them being all the same is correct. - /// - /// To avoid needless allocations, it is advised to pass `global_table_spec` - /// in the borrowed form, so that cloning it is cheap. - fn $deser_col_specs<'frame>( - buf: &mut &'frame [u8], - global_table_spec: Option>, - col_count: usize, - ) -> StdResult>, ColumnSpecParseError> { - let global_table_spec_provided = global_table_spec.is_some(); - let mut known_table_spec = global_table_spec; - - let mut col_specs = Vec::with_capacity(col_count); - for col_idx in 0..col_count { - let table_spec = deser_table_spec_for_col_spec( - buf, - global_table_spec_provided, - &mut known_table_spec, - col_idx, - )?; - - let name = - types::read_string(buf).map_err(|err| mk_col_spec_parse_error(col_idx, err))?; - let typ = $deser_type(buf).map_err(|err| mk_col_spec_parse_error(col_idx, err))?; - let col_spec = $make_col_spec(name, typ, table_spec); - col_specs.push(col_spec); - } - Ok(col_specs) - } - }; +/// Deserializes col specs (part of ResultMetadata or PreparedMetadata) +/// in the form mentioned by its name. +/// +/// Checks for equality of table specs across columns, because the protocol +/// does not guarantee that and we want to be sure that the assumption +/// of them being all the same is correct. +/// +/// To avoid needless allocations, it is advised to pass `global_table_spec` +/// in the borrowed form, so that cloning it is cheap. +fn deser_col_specs_generic<'frame, 'result>( + buf: &mut &'frame [u8], + global_table_spec: Option>, + col_count: usize, + make_col_spec: fn(&'frame str, ColumnType<'result>, TableSpec<'frame>) -> ColumnSpec<'result>, + deser_type: fn(&mut &'frame [u8]) -> StdResult, CqlTypeParseError>, +) -> StdResult>, ColumnSpecParseError> { + let global_table_spec_provided = global_table_spec.is_some(); + let mut known_table_spec = global_table_spec; + + let mut col_specs = Vec::with_capacity(col_count); + for col_idx in 0..col_count { + let table_spec = deser_table_spec_for_col_spec( + buf, + global_table_spec_provided, + &mut known_table_spec, + col_idx, + )?; + + let name = types::read_string(buf).map_err(|err| mk_col_spec_parse_error(col_idx, err))?; + let typ = deser_type(buf).map_err(|err| mk_col_spec_parse_error(col_idx, err))?; + let col_spec = make_col_spec(name, typ, table_spec); + col_specs.push(col_spec); + } + Ok(col_specs) +} + +fn _deser_col_specs_borrowed<'frame>( + buf: &mut &'frame [u8], + global_table_spec: Option>, + col_count: usize, +) -> StdResult>, ColumnSpecParseError> { + deser_col_specs_generic( + buf, + global_table_spec, + col_count, + ColumnSpec::borrowed, + _deser_type_borrowed, + ) } -generate_deser_col_specs!( - _deser_col_specs_borrowed, - 'frame, - _deser_type_borrowed, - ColumnSpec::borrowed, -); - -generate_deser_col_specs!( - deser_col_specs_owned, - 'static, - deser_type_owned, - |name: &str, typ, table_spec: TableSpec| ColumnSpec::owned(name.to_owned(), typ, table_spec.into_owned()), -); +fn deser_col_specs_owned<'frame>( + buf: &mut &'frame [u8], + global_table_spec: Option>, + col_count: usize, +) -> StdResult>, ColumnSpecParseError> { + let result: StdResult>, ColumnSpecParseError> = deser_col_specs_generic( + buf, + global_table_spec, + col_count, + |name: &str, typ, table_spec: TableSpec| { + ColumnSpec::owned(name.to_owned(), typ, table_spec.into_owned()) + }, + deser_type_owned, + ); + + result +} fn deser_result_metadata( buf: &mut &[u8],