Skip to content

Commit 33924cc

Browse files
committed
parametrize DeserializeRow with two lifetimes
`DeserializeRow` requires two types of data in order to perform deserialization: 1) a reference to the CQL frame (a FrameSlice), 2) a slice of specifications of all columns in the row, being part of the ResultMetadata. It's important to understand what is a _deserialized row_. It's not just an implementor of DeserializeRow; there are some implementors of `DeserializeRow` who are not yet final rows, but **partially** deserialized types that support further deserialization - _row deserializers_, such as `ColumnIterator`. _Row deserializers_, as they still need to deserialize some row, are naturally bound by 'metadata lifetime. However, _rows_ are completely deserialized, so they should not be bound by 'metadata - only by 'frame. When deserializing owned rows, both the frame and the metadata can have any lifetime and it's not important. When deserializing borrowing rows, however, they borrow from the frame, so their lifetime must necessarily be bound by the lifetime of the frame. Metadata is only needed for the deserialization, so its lifetime does not abstractly bound the deserialized row. Up to this commit, DeserializeRow was only parametrized by one lifetime: 'frame, which bounded both the frame slice and the metadata. (why? the reason is the same as in DeserializeValue - see the previous commit) This was done that way due to an assumption that both the metadata and the frame (shared using Bytes) will be owned by the same entity. However, the new idea of deserializing result metadata to a borrowed form (to save allocations) makes result metadata's lifetime shorter than the frame's lifetime. Not to unnecessarily shorten the deserialized values' lifetime, a separate lifetime parameter is introduced for result metadata: 'metadata. The commit is large, but the changes are mostly mechanical, by adding the second lifetime parameter. RowIterator and TypedRowIterator are going to get the second lifetime parameter, too, but for now they pass 'frame as both lifetime parameters of DeserializeRow.
1 parent f6441d5 commit 33924cc

File tree

6 files changed

+51
-63
lines changed

6 files changed

+51
-63
lines changed

scylla-cql/src/types/deserialize/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
//!
55
//! - A type that implements `DeserializeValue<'frame, 'metadata>` can be deserialized
66
//! from a single _CQL value_ - i.e. an element of a row in the query result,
7-
//! - A type that implements `DeserializeRow<'frame>` can be deserialized
7+
//! - A type that implements `DeserializeRow<'frame, 'metadata>` can be deserialized
88
//! from a single _row_ of a query result.
99
//!
1010
//! Those traits are quite similar to each other, both in the idea behind them

scylla-cql/src/types/deserialize/result.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ impl<'frame> RowIterator<'frame> {
4242
}
4343

4444
impl<'frame> Iterator for RowIterator<'frame> {
45-
type Item = Result<ColumnIterator<'frame>, DeserializationError>;
45+
type Item = Result<ColumnIterator<'frame, 'frame>, DeserializationError>;
4646

4747
#[inline]
4848
fn next(&mut self) -> Option<Self::Item> {
@@ -85,7 +85,7 @@ pub struct TypedRowIterator<'frame, R> {
8585

8686
impl<'frame, R> TypedRowIterator<'frame, R>
8787
where
88-
R: DeserializeRow<'frame>,
88+
R: DeserializeRow<'frame, 'frame>,
8989
{
9090
/// Creates a new [TypedRowIterator] from given [RowIterator].
9191
///
@@ -115,7 +115,7 @@ where
115115

116116
impl<'frame, R> Iterator for TypedRowIterator<'frame, R>
117117
where
118-
R: DeserializeRow<'frame>,
118+
R: DeserializeRow<'frame, 'frame>,
119119
{
120120
type Item = Result<R, DeserializationError>;
121121

scylla-cql/src/types/deserialize/row.rs

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,26 +10,29 @@ use crate::frame::response::result::{ColumnSpec, ColumnType, CqlValue, Row};
1010

1111
/// Represents a raw, unparsed column value.
1212
#[non_exhaustive]
13-
pub struct RawColumn<'frame> {
13+
pub struct RawColumn<'frame, 'metadata> {
1414
pub index: usize,
15-
pub spec: &'frame ColumnSpec<'frame>,
15+
pub spec: &'metadata ColumnSpec<'metadata>,
1616
pub slice: Option<FrameSlice<'frame>>,
1717
}
1818

1919
/// Iterates over columns of a single row.
2020
#[derive(Clone, Debug)]
21-
pub struct ColumnIterator<'frame> {
22-
specs: std::iter::Enumerate<std::slice::Iter<'frame, ColumnSpec<'frame>>>,
21+
pub struct ColumnIterator<'frame, 'metadata> {
22+
specs: std::iter::Enumerate<std::slice::Iter<'metadata, ColumnSpec<'metadata>>>,
2323
slice: FrameSlice<'frame>,
2424
}
2525

26-
impl<'frame> ColumnIterator<'frame> {
26+
impl<'frame, 'metadata> ColumnIterator<'frame, 'metadata> {
2727
/// Creates a new iterator over a single row.
2828
///
2929
/// - `specs` - information about columns of the serialized response,
3030
/// - `slice` - a [FrameSlice] which points to the serialized row.
3131
#[inline]
32-
pub(crate) fn new(specs: &'frame [ColumnSpec], slice: FrameSlice<'frame>) -> Self {
32+
pub(crate) fn new(
33+
specs: &'metadata [ColumnSpec<'metadata>],
34+
slice: FrameSlice<'frame>,
35+
) -> Self {
3336
Self {
3437
specs: specs.iter().enumerate(),
3538
slice,
@@ -44,8 +47,8 @@ impl<'frame> ColumnIterator<'frame> {
4447
}
4548
}
4649

47-
impl<'frame> Iterator for ColumnIterator<'frame> {
48-
type Item = Result<RawColumn<'frame>, DeserializationError>;
50+
impl<'frame, 'metadata> Iterator for ColumnIterator<'frame, 'metadata> {
51+
type Item = Result<RawColumn<'frame, 'metadata>, DeserializationError>;
4952

5053
#[inline]
5154
fn next(&mut self) -> Option<Self::Item> {
@@ -84,7 +87,7 @@ impl<'frame> Iterator for ColumnIterator<'frame> {
8487
/// The crate also provides a derive macro which allows to automatically
8588
/// implement the trait for a custom type. For more details on what the macro
8689
/// is capable of, see its documentation.
87-
pub trait DeserializeRow<'frame>
90+
pub trait DeserializeRow<'frame, 'metadata>
8891
where
8992
Self: Sized,
9093
{
@@ -100,7 +103,7 @@ where
100103
/// the row's type. Note that `deserialize` is not an unsafe function,
101104
/// so it should not use the assumption about `type_check` being called
102105
/// as an excuse to run `unsafe` code.
103-
fn deserialize(row: ColumnIterator<'frame>) -> Result<Self, DeserializationError>;
106+
fn deserialize(row: ColumnIterator<'frame, 'metadata>) -> Result<Self, DeserializationError>;
104107
}
105108

106109
// raw deserialization as ColumnIterator
@@ -111,14 +114,14 @@ where
111114
// Implementing DeserializeRow for it allows us to simplify our interface. For example,
112115
// we have `QueryResult::rows<T: DeserializeRow>()` - you can put T = ColumnIterator
113116
// instead of having a separate rows_raw function or something like this.
114-
impl<'frame> DeserializeRow<'frame> for ColumnIterator<'frame> {
117+
impl<'frame, 'metadata> DeserializeRow<'frame, 'metadata> for ColumnIterator<'frame, 'metadata> {
115118
#[inline]
116119
fn type_check(_specs: &[ColumnSpec]) -> Result<(), TypeCheckError> {
117120
Ok(())
118121
}
119122

120123
#[inline]
121-
fn deserialize(row: ColumnIterator<'frame>) -> Result<Self, DeserializationError> {
124+
fn deserialize(row: ColumnIterator<'frame, 'metadata>) -> Result<Self, DeserializationError> {
122125
Ok(row)
123126
}
124127
}
@@ -140,15 +143,17 @@ make_error_replace_rust_name!(
140143
/// While no longer encouraged (because the new framework encourages deserializing
141144
/// directly into desired types, entirely bypassing [CqlValue]), this can be indispensable
142145
/// for some use cases, i.e. those involving dynamic parsing (ORMs?).
143-
impl<'frame> DeserializeRow<'frame> for Row {
146+
impl<'frame, 'metadata> DeserializeRow<'frame, 'metadata> for Row {
144147
#[inline]
145148
fn type_check(_specs: &[ColumnSpec]) -> Result<(), TypeCheckError> {
146149
// CqlValues accept all types, no type checking needed.
147150
Ok(())
148151
}
149152

150153
#[inline]
151-
fn deserialize(mut row: ColumnIterator<'frame>) -> Result<Self, DeserializationError> {
154+
fn deserialize(
155+
mut row: ColumnIterator<'frame, 'metadata>,
156+
) -> Result<Self, DeserializationError> {
152157
let mut columns = Vec::with_capacity(row.size_hint().0);
153158
while let Some(column) = row
154159
.next()
@@ -181,17 +186,17 @@ impl<'frame> DeserializeRow<'frame> for Row {
181186
/// and needed conversions, issuing meaningful errors in case something goes wrong.
182187
macro_rules! impl_tuple {
183188
($($Ti:ident),*; $($idx:literal),*; $($idf:ident),*) => {
184-
impl<'frame, $($Ti),*> DeserializeRow<'frame> for ($($Ti,)*)
189+
impl<'frame, 'metadata, $($Ti),*> DeserializeRow<'frame, 'metadata> for ($($Ti,)*)
185190
where
186-
$($Ti: DeserializeValue<'frame, 'frame>),*
191+
$($Ti: DeserializeValue<'frame, 'metadata>),*
187192
{
188193
fn type_check(specs: &[ColumnSpec]) -> Result<(), TypeCheckError> {
189194
const TUPLE_LEN: usize = (&[$($idx),*] as &[i32]).len();
190195

191196
let column_types_iter = || specs.iter().map(|spec| spec.typ().clone().into_owned());
192197
if let [$($idf),*] = &specs {
193198
$(
194-
<$Ti as DeserializeValue<'frame, 'frame>>::type_check($idf.typ())
199+
<$Ti as DeserializeValue<'frame, 'metadata>>::type_check($idf.typ())
195200
.map_err(|err| mk_typck_err::<Self>(column_types_iter(), BuiltinTypeCheckErrorKind::ColumnTypeCheckFailed {
196201
column_index: $idx,
197202
column_name: specs[$idx].name().to_owned(),
@@ -206,7 +211,7 @@ macro_rules! impl_tuple {
206211
}
207212
}
208213

209-
fn deserialize(mut row: ColumnIterator<'frame>) -> Result<Self, DeserializationError> {
214+
fn deserialize(mut row: ColumnIterator<'frame, 'metadata>) -> Result<Self, DeserializationError> {
210215
const TUPLE_LEN: usize = (&[$($idx),*] as &[i32]).len();
211216

212217
let ret = (
@@ -217,7 +222,7 @@ macro_rules! impl_tuple {
217222
$idx
218223
)).map_err(deser_error_replace_rust_name::<Self>)?;
219224

220-
<$Ti as DeserializeValue<'frame, 'frame>>::deserialize(column.spec.typ(), column.slice)
225+
<$Ti as DeserializeValue<'frame, 'metadata>>::deserialize(column.spec.typ(), column.slice)
221226
.map_err(|err| mk_deser_err::<Self>(BuiltinDeserializationErrorKind::ColumnDeserializationFailed {
222227
column_index: column.index,
223228
column_name: column.spec.name().to_owned(),

scylla-cql/src/types/deserialize/row_tests.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -251,18 +251,18 @@ fn val_str(s: &str) -> Option<Vec<u8>> {
251251
Some(s.as_bytes().to_vec())
252252
}
253253

254-
fn deserialize<'frame, R>(
255-
specs: &'frame [ColumnSpec],
254+
fn deserialize<'frame, 'metadata, R>(
255+
specs: &'metadata [ColumnSpec<'metadata>],
256256
byts: &'frame Bytes,
257257
) -> Result<R, DeserializationError>
258258
where
259-
R: DeserializeRow<'frame>,
259+
R: DeserializeRow<'frame, 'metadata>,
260260
{
261-
<R as DeserializeRow<'frame>>::type_check(specs)
261+
<R as DeserializeRow<'frame, 'metadata>>::type_check(specs)
262262
.map_err(|typecheck_err| DeserializationError(typecheck_err.0))?;
263263
let slice = FrameSlice::new(byts);
264264
let iter = ColumnIterator::new(specs, slice);
265-
<R as DeserializeRow<'frame>>::deserialize(iter)
265+
<R as DeserializeRow<'frame, 'metadata>>::deserialize(iter)
266266
}
267267

268268
#[track_caller]

scylla-macros/src/deserialize/mod.rs

Lines changed: 6 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -114,29 +114,12 @@ where
114114
let trait_: syn::Path = parse_quote!(#macro_internal::#trait_);
115115
let items = items.into_iter();
116116

117-
// This `if` is purely temporary. When in a next commit DeserializeRow receives a 'metadata lifetime,
118-
// the handling of both traits is unified again.
119-
if trait_
120-
.segments
121-
.last()
122-
.is_some_and(|name| name.ident == "DeserializeValue")
123-
{
124-
parse_quote! {
125-
impl<#frame_lifetime, #metadata_lifetime, #impl_generics>
126-
#trait_<#frame_lifetime, #metadata_lifetime> for #struct_name #ty_generics
127-
where #(#predicates),*
128-
{
129-
#(#items)*
130-
}
131-
}
132-
} else {
133-
parse_quote! {
134-
impl<#frame_lifetime, #impl_generics>
135-
#trait_<#frame_lifetime> for #struct_name #ty_generics
136-
where #(#predicates),*
137-
{
138-
#(#items)*
139-
}
117+
parse_quote! {
118+
impl<#frame_lifetime, #metadata_lifetime, #impl_generics>
119+
#trait_<#frame_lifetime, #metadata_lifetime> for #struct_name #ty_generics
120+
where #(#predicates),*
121+
{
122+
#(#items)*
140123
}
141124
}
142125
}

scylla-macros/src/deserialize/row.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ impl<'sd> TypeCheckAssumeOrderGenerator<'sd> {
206206
// of the columns correspond fields' names/types.
207207

208208
let macro_internal = self.0.struct_attrs().macro_internal_path();
209-
let (constraint_lifetime, _) = self.0.constraint_lifetimes();
209+
let (frame_lifetime, metadata_lifetime) = self.0.constraint_lifetimes();
210210

211211
let required_fields_iter = || {
212212
self.0
@@ -242,7 +242,7 @@ impl<'sd> TypeCheckAssumeOrderGenerator<'sd> {
242242
#name_verifications
243243

244244
// Verify the type
245-
<#required_fields_deserializers as #macro_internal::DeserializeValue<#constraint_lifetime, #constraint_lifetime>>::type_check(#required_fields_idents.typ())
245+
<#required_fields_deserializers as #macro_internal::DeserializeValue<#frame_lifetime, #metadata_lifetime>>::type_check(#required_fields_idents.typ())
246246
.map_err(|err| #macro_internal::mk_row_typck_err::<Self>(
247247
column_types_iter(),
248248
#macro_internal::DeserBuiltinRowTypeCheckErrorKind::ColumnTypeCheckFailed {
@@ -281,7 +281,7 @@ impl<'sd> DeserializeAssumeOrderGenerator<'sd> {
281281
let macro_internal = self.0.struct_attrs().macro_internal_path();
282282
let cql_name_literal = field.cql_name_literal();
283283
let deserializer = field.deserialize_target();
284-
let (constraint_lifetime, _) = self.0.constraint_lifetimes();
284+
let (frame_lifetime, metadata_lifetime) = self.0.constraint_lifetimes();
285285

286286
let name_check: Option<syn::Stmt> = (!self.0.struct_attrs().skip_name_checks).then(|| parse_quote! {
287287
if col.spec.name() != #cql_name_literal {
@@ -301,7 +301,7 @@ impl<'sd> DeserializeAssumeOrderGenerator<'sd> {
301301

302302
#name_check
303303

304-
<#deserializer as #macro_internal::DeserializeValue<#constraint_lifetime, #constraint_lifetime>>::deserialize(col.spec.typ(), col.slice)
304+
<#deserializer as #macro_internal::DeserializeValue<#frame_lifetime, #metadata_lifetime>>::deserialize(col.spec.typ(), col.slice)
305305
.map_err(|err| #macro_internal::mk_row_deser_err::<Self>(
306306
#macro_internal::BuiltinRowDeserializationErrorKind::ColumnDeserializationFailed {
307307
column_index: #field_index,
@@ -315,7 +315,7 @@ impl<'sd> DeserializeAssumeOrderGenerator<'sd> {
315315

316316
fn generate(&self) -> syn::ImplItemFn {
317317
let macro_internal = self.0.struct_attrs().macro_internal_path();
318-
let (constraint_lifetime, _) = self.0.constraint_lifetimes();
318+
let (frame_lifetime, metadata_lifetime) = self.0.constraint_lifetimes();
319319

320320
let fields = self.0.fields();
321321
let field_idents = fields.iter().map(|f| f.ident.as_ref().unwrap());
@@ -327,7 +327,7 @@ impl<'sd> DeserializeAssumeOrderGenerator<'sd> {
327327
parse_quote! {
328328
fn deserialize(
329329
#[allow(unused_mut)]
330-
mut row: #macro_internal::ColumnIterator<#constraint_lifetime>,
330+
mut row: #macro_internal::ColumnIterator<#frame_lifetime, #metadata_lifetime>,
331331
) -> ::std::result::Result<Self, #macro_internal::DeserializationError> {
332332
::std::result::Result::Ok(Self {
333333
#(#field_idents: #field_finalizers,)*
@@ -362,7 +362,7 @@ impl<'sd> TypeCheckUnorderedGenerator<'sd> {
362362
fn generate_type_check(&self, field: &Field) -> Option<syn::Block> {
363363
(!field.skip).then(|| {
364364
let macro_internal = self.0.struct_attrs().macro_internal_path();
365-
let (constraint_lifetime, _) = self.0.constraint_lifetimes();
365+
let (frame_lifetime, metadata_lifetime) = self.0.constraint_lifetimes();
366366
let visited_flag = Self::visited_flag_variable(field);
367367
let typ = field.deserialize_target();
368368
let cql_name_literal = field.cql_name_literal();
@@ -373,7 +373,7 @@ impl<'sd> TypeCheckUnorderedGenerator<'sd> {
373373
parse_quote! {
374374
{
375375
if !#visited_flag {
376-
<#typ as #macro_internal::DeserializeValue<#constraint_lifetime, #constraint_lifetime>>::type_check(spec.typ())
376+
<#typ as #macro_internal::DeserializeValue<#frame_lifetime, #metadata_lifetime>>::type_check(spec.typ())
377377
.map_err(|err| {
378378
#macro_internal::mk_row_typck_err::<Self>(
379379
column_types_iter(),
@@ -516,7 +516,7 @@ impl<'sd> DeserializeUnorderedGenerator<'sd> {
516516
fn generate_deserialization(&self, column_index: usize, field: &Field) -> syn::Expr {
517517
assert!(!field.skip);
518518
let macro_internal = self.0.struct_attrs().macro_internal_path();
519-
let (constraint_lifetime, _) = self.0.constraint_lifetimes();
519+
let (frame_lifetime, metadata_lifetime) = self.0.constraint_lifetimes();
520520
let deserialize_field = Self::deserialize_field_variable(field);
521521
let deserializer = field.deserialize_target();
522522

@@ -529,7 +529,7 @@ impl<'sd> DeserializeUnorderedGenerator<'sd> {
529529
);
530530

531531
#deserialize_field = ::std::option::Option::Some(
532-
<#deserializer as #macro_internal::DeserializeValue<#constraint_lifetime, #constraint_lifetime>>::deserialize(col.spec.typ(), col.slice)
532+
<#deserializer as #macro_internal::DeserializeValue<#frame_lifetime, #metadata_lifetime>>::deserialize(col.spec.typ(), col.slice)
533533
.map_err(|err| {
534534
#macro_internal::mk_row_deser_err::<Self>(
535535
#macro_internal::BuiltinRowDeserializationErrorKind::ColumnDeserializationFailed {
@@ -557,7 +557,7 @@ impl<'sd> DeserializeUnorderedGenerator<'sd> {
557557

558558
fn generate(&self) -> syn::ImplItemFn {
559559
let macro_internal = self.0.struct_attrs().macro_internal_path();
560-
let (constraint_lifetime, _) = self.0.constraint_lifetimes();
560+
let (frame_lifetime, metadata_lifetime) = self.0.constraint_lifetimes();
561561
let fields = self.0.fields();
562562

563563
let deserialize_field_decls = fields
@@ -581,7 +581,7 @@ impl<'sd> DeserializeUnorderedGenerator<'sd> {
581581
parse_quote! {
582582
fn deserialize(
583583
#[allow(unused_mut)]
584-
mut row: #macro_internal::ColumnIterator<#constraint_lifetime>,
584+
mut row: #macro_internal::ColumnIterator<#frame_lifetime, #metadata_lifetime>,
585585
) -> ::std::result::Result<Self, #macro_internal::DeserializationError> {
586586

587587
// Generate fields that will serve as temporary storage

0 commit comments

Comments
 (0)