Merge pull request #858 from Lorak-mmk/switch-serialization-traits

Switch Session to new serialization traits

docs/source/data-types/udt.md

@@ -37,10 +37,10 @@ Now it can be sent and received just like any other CQL value:
 # use std::error::Error;
 # async fn check_only_compiles(session: &Session) -> Result<(), Box<dyn Error>> {
 use scylla::IntoTypedRows;
-use scylla::macros::{FromUserType, IntoUserType};
+use scylla::macros::{FromUserType, IntoUserType, SerializeCql};
 use scylla::cql_to_rust::FromCqlVal;
 
-#[derive(Debug, IntoUserType, FromUserType)]
+#[derive(Debug, IntoUserType, FromUserType, SerializeCql)]
 struct MyType {
     int_val: i32,
     text_val: Option<String>,

docs/source/queries/values.md

@@ -12,7 +12,7 @@ or a custom struct which derives from `ValueList`.
 A few examples:
 ```rust
 # extern crate scylla;
-# use scylla::{Session, ValueList, frame::response::result::CqlValue};
+# use scylla::{Session, ValueList, SerializeRow, frame::response::result::CqlValue};
 # use std::error::Error;
 # use std::collections::HashMap;
 # async fn check_only_compiles(session: &Session) -> Result<(), Box<dyn Error>> {
@@ -34,7 +34,7 @@ session
 
 // Sending an integer and a string using a named struct.
 // The values will be passed in the order from the struct definition
-#[derive(ValueList)]
+#[derive(ValueList, SerializeRow)]
 struct IntString {
     first_col: i32,
     second_col: String,

examples/compare-tokens.rs

@@ -1,5 +1,4 @@
 use anyhow::Result;
-use scylla::frame::value::ValueList;
 use scylla::routing::Token;
 use scylla::transport::NodeAddr;
 use scylla::{Session, SessionBuilder};
@@ -29,8 +28,7 @@ async fn main() -> Result<()> {
             .query("INSERT INTO ks.t (pk) VALUES (?)", (pk,))
             .await?;
 
-        let serialized_pk = (pk,).serialized()?.into_owned();
-        let t = prepared.calculate_token(&serialized_pk)?.unwrap().value;
+        let t = prepared.calculate_token(&(pk,))?.unwrap().value;
 
         println!(
             "Token endpoints for query: {:?}",

examples/user-defined-type.rs

@@ -1,6 +1,6 @@
 use anyhow::Result;
-use scylla::macros::{FromUserType, IntoUserType};
-use scylla::{IntoTypedRows, Session, SessionBuilder};
+use scylla::macros::FromUserType;
+use scylla::{IntoTypedRows, SerializeCql, Session, SessionBuilder};
 use std::env;
 
 #[tokio::main]
@@ -29,7 +29,7 @@ async fn main() -> Result<()> {
 
     // Define custom struct that matches User Defined Type created earlier
     // wrapping field in Option will gracefully handle null field values
-    #[derive(Debug, IntoUserType, FromUserType)]
+    #[derive(Debug, FromUserType, SerializeCql)]
     struct MyType {
         int_val: i32,
         text_val: Option<String>,

examples/value_list.rs

@@ -19,7 +19,7 @@ async fn main() {
         .await
         .unwrap();
 
-    #[derive(scylla::ValueList)]
+    #[derive(scylla::SerializeRow)]
     struct MyType<'a> {
         k: i32,
         my: Option<&'a str>,
@@ -36,8 +36,8 @@ async fn main() {
         .unwrap();
 
     // You can also use type generics:
-    #[derive(scylla::ValueList)]
-    struct MyTypeWithGenerics<S: scylla::frame::value::Value> {
+    #[derive(scylla::SerializeRow)]
+    struct MyTypeWithGenerics<S: scylla::serialize::value::SerializeCql> {
         k: i32,
         my: Option<S>,
     }

scylla-cql/benches/benchmark.rs

@@ -3,17 +3,17 @@ use std::borrow::Cow;
 use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion};
 
 use scylla_cql::frame::request::SerializableRequest;
-use scylla_cql::frame::value::SerializedValues;
-use scylla_cql::frame::value::ValueList;
+use scylla_cql::frame::response::result::ColumnType;
 use scylla_cql::frame::{request::query, Compression, SerializedRequest};
+use scylla_cql::types::serialize::row::SerializedValues;
 
-fn make_query<'a>(contents: &'a str, values: &'a SerializedValues) -> query::Query<'a> {
+fn make_query(contents: &str, values: SerializedValues) -> query::Query<'_> {
     query::Query {
         contents: Cow::Borrowed(contents),
         parameters: query::QueryParameters {
             consistency: scylla_cql::Consistency::LocalQuorum,
             serial_consistency: None,
-            values: Cow::Borrowed(values),
+            values: Cow::Owned(values),
             page_size: None,
             paging_state: None,
             timestamp: None,
@@ -22,13 +22,24 @@ fn make_query<'a>(contents: &'a str, values: &'a SerializedValues) -> query::Que
 }
 
 fn serialized_request_make_bench(c: &mut Criterion) {
+    let mut values = SerializedValues::new();
     let mut group = c.benchmark_group("LZ4Compression.SerializedRequest");
     let query_args = [
-        ("INSERT foo INTO ks.table_name (?)", &(1234,).serialized().unwrap()),
-        ("INSERT foo, bar, baz INTO ks.table_name (?, ?, ?)", &(1234, "a value", "i am storing a string").serialized().unwrap()),
+        ("INSERT foo INTO ks.table_name (?)", {
+            values.add_value(&1234, &ColumnType::Int).unwrap();
+            values.clone()
+        }),
+        ("INSERT foo, bar, baz INTO ks.table_name (?, ?, ?)", {
+            values.add_value(&"a value", &ColumnType::Text).unwrap();
+            values.add_value(&"i am storing a string", &ColumnType::Text).unwrap();
+            values.clone()
+        }),
         (
             "INSERT foo, bar, baz, boop, blah INTO longer_keyspace.a_big_table_name (?, ?, ?, ?, 1000)",
-            &(1234, "a value", "i am storing a string", "dc0c8cd7-d954-47c1-8722-a857941c43fb").serialized().unwrap()
+            {
+                values.add_value(&"dc0c8cd7-d954-47c1-8722-a857941c43fb", &ColumnType::Text).unwrap();
+                values.clone()
+            }
         ),
     ];
     let queries = query_args.map(|(q, v)| make_query(q, v));

scylla-cql/src/errors.rs

@@ -3,6 +3,7 @@
 use crate::frame::frame_errors::{FrameError, ParseError};
 use crate::frame::protocol_features::ProtocolFeatures;
 use crate::frame::value::SerializeValuesError;
+use crate::types::serialize::SerializationError;
 use crate::Consistency;
 use bytes::Bytes;
 use std::io::ErrorKind;
@@ -340,6 +341,9 @@ pub enum BadQuery {
     #[error("Serializing values failed: {0} ")]
     SerializeValuesError(#[from] SerializeValuesError),
 
+    #[error("Serializing values failed: {0} ")]
+    SerializationError(#[from] SerializationError),
+
     /// Serialized values are too long to compute partition key
     #[error("Serialized values are too long to compute partition key! Length: {0}, Max allowed length: {1}")]
     ValuesTooLongForKey(usize, usize),
@@ -443,6 +447,12 @@ impl From<SerializeValuesError> for QueryError {
     }
 }
 
+impl From<SerializationError> for QueryError {
+    fn from(serialized_err: SerializationError) -> QueryError {
+        QueryError::BadQuery(BadQuery::SerializationError(serialized_err))
+    }
+}
+
 impl From<ParseError> for QueryError {
     fn from(parse_error: ParseError) -> QueryError {
         QueryError::InvalidMessage(format!("Error parsing message: {}", parse_error))

scylla-cql/src/frame/frame_errors.rs

@@ -1,6 +1,7 @@
 use super::response;
 use crate::cql_to_rust::CqlTypeError;
 use crate::frame::value::SerializeValuesError;
+use crate::types::serialize::SerializationError;
 use thiserror::Error;
 
 #[derive(Error, Debug)]
@@ -44,5 +45,7 @@ pub enum ParseError {
     #[error(transparent)]
     SerializeValuesError(#[from] SerializeValuesError),
     #[error(transparent)]
+    SerializationError(#[from] SerializationError),
+    #[error(transparent)]
     CqlTypeError(#[from] CqlTypeError),
 }

scylla-cql/src/frame/request/batch.rs

@@ -5,7 +5,7 @@ use crate::frame::{
     frame_errors::ParseError,
     request::{RequestOpcode, SerializableRequest},
     types::{self, SerialConsistency},
-    value::{BatchValues, BatchValuesIterator, SerializedValues},
+    value::{BatchValues, BatchValuesIterator, LegacySerializedValues},
 };
 
 use super::DeserializableRequest;
@@ -186,7 +186,7 @@ impl<'s, 'b> From<&'s BatchStatement<'b>> for BatchStatement<'s> {
     }
 }
 
-impl<'b> DeserializableRequest for Batch<'b, BatchStatement<'b>, Vec<SerializedValues>> {
+impl<'b> DeserializableRequest for Batch<'b, BatchStatement<'b>, Vec<LegacySerializedValues>> {
     fn deserialize(buf: &mut &[u8]) -> Result<Self, ParseError> {
         let batch_type = buf.get_u8().try_into()?;
 
@@ -196,7 +196,7 @@ impl<'b> DeserializableRequest for Batch<'b, BatchStatement<'b>, Vec<SerializedV
                 let batch_statement = BatchStatement::deserialize(buf)?;
 
                 // As stated in CQL protocol v4 specification, values names in Batch are broken and should be never used.
-                let values = SerializedValues::new_from_frame(buf, false)?;
+                let values = LegacySerializedValues::new_from_frame(buf, false)?;
 
                 Ok((batch_statement, values))
             })
@@ -233,7 +233,7 @@ impl<'b> DeserializableRequest for Batch<'b, BatchStatement<'b>, Vec<SerializedV
             .then(|| types::read_long(buf))
             .transpose()?;
 
-        let (statements, values): (Vec<BatchStatement>, Vec<SerializedValues>) =
+        let (statements, values): (Vec<BatchStatement>, Vec<LegacySerializedValues>) =
             statements_with_values.into_iter().unzip();
 
         Ok(Self {

scylla-cql/src/frame/request/mod.rs

@@ -22,7 +22,7 @@ pub use startup::Startup;
 use self::batch::BatchStatement;
 
 use super::types::SerialConsistency;
-use super::value::SerializedValues;
+use super::value::LegacySerializedValues;
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, TryFromPrimitive)]
 #[repr(u8)]
@@ -59,7 +59,7 @@ pub trait DeserializableRequest: SerializableRequest + Sized {
 pub enum Request<'r> {
     Query(Query<'r>),
     Execute(Execute<'r>),
-    Batch(Batch<'r, BatchStatement<'r>, Vec<SerializedValues>>),
+    Batch(Batch<'r, BatchStatement<'r>, Vec<LegacySerializedValues>>),
 }
 
 impl<'r> Request<'r> {
@@ -112,9 +112,10 @@ mod tests {
                 query::{Query, QueryParameters},
                 DeserializableRequest, SerializableRequest,
             },
+            response::result::ColumnType,
             types::{self, SerialConsistency},
-            value::SerializedValues,
         },
+        types::serialize::row::SerializedValues,
         Consistency,
     };
 
@@ -130,7 +131,7 @@ mod tests {
             paging_state: Some(vec![2, 1, 3, 7].into()),
             values: {
                 let mut vals = SerializedValues::new();
-                vals.add_value(&2137).unwrap();
+                vals.add_value(&2137, &ColumnType::Int).unwrap();
                 Cow::Owned(vals)
             },
         };
@@ -157,8 +158,8 @@ mod tests {
             paging_state: None,
             values: {
                 let mut vals = SerializedValues::new();
-                vals.add_named_value("the_answer", &42).unwrap();
-                vals.add_named_value("really?", &2137).unwrap();
+                vals.add_value(&42, &ColumnType::Int).unwrap();
+                vals.add_value(&2137, &ColumnType::Int).unwrap();
                 Cow::Owned(vals)
             },
         };
@@ -189,8 +190,8 @@ mod tests {
 
             // Not execute's values, because named values are not supported in batches.
             values: vec![
-                query.parameters.values.deref().clone(),
-                query.parameters.values.deref().clone(),
+                query.parameters.values.deref().to_old_serialized_values(),
+                query.parameters.values.deref().to_old_serialized_values(),
             ],
         };
         {
@@ -212,7 +213,7 @@ mod tests {
             timestamp: None,
             page_size: None,
             paging_state: None,
-            values: Cow::Owned(SerializedValues::new()),
+            values: Cow::Borrowed(SerializedValues::EMPTY),
         };
         let query = Query {
             contents: contents.clone(),
@@ -261,7 +262,7 @@ mod tests {
             serial_consistency: None,
             timestamp: None,
 
-            values: vec![query.parameters.values.deref().clone()],
+            values: vec![query.parameters.values.deref().to_old_serialized_values()],
         };
         {
             let mut buf = Vec::new();

scylla-cql/src/frame/request/query.rs

@@ -1,12 +1,14 @@
 use std::borrow::Cow;
 
-use crate::frame::{frame_errors::ParseError, types::SerialConsistency};
+use crate::{
+    frame::{frame_errors::ParseError, types::SerialConsistency},
+    types::serialize::row::SerializedValues,
+};
 use bytes::{Buf, BufMut, Bytes};
 
 use crate::{
     frame::request::{RequestOpcode, SerializableRequest},
     frame::types,
-    frame::value::SerializedValues,
 };
 
 use super::DeserializableRequest;
@@ -102,10 +104,6 @@ impl QueryParameters<'_> {
             flags |= FLAG_WITH_DEFAULT_TIMESTAMP;
         }
 
-        if self.values.has_names() {
-            flags |= FLAG_WITH_NAMES_FOR_VALUES;
-        }
-
         buf.put_u8(flags);
 
         if !self.values.is_empty() {
@@ -151,8 +149,14 @@ impl<'q> QueryParameters<'q> {
         let default_timestamp_flag = (flags & FLAG_WITH_DEFAULT_TIMESTAMP) != 0;
         let values_have_names_flag = (flags & FLAG_WITH_NAMES_FOR_VALUES) != 0;
 
+        if values_have_names_flag {
+            return Err(ParseError::BadIncomingData(
+                "Named values in frame are currently unsupported".to_string(),
+            ));
+        }
+
         let values = Cow::Owned(if values_flag {
-            SerializedValues::new_from_frame(buf, values_have_names_flag)?
+            SerializedValues::new_from_frame(buf)?
         } else {
             SerializedValues::new()
         });