Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions scylla-cql/src/deserialize/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1340,17 +1340,26 @@ macro_rules! impl_tuple {
let mut v = ensure_not_null_frame_slice::<Self>(typ, v)?;
let ret = (
$(
v.read_cql_bytes()
.map_err(|err| DeserializationError::new(err))
.and_then(|cql_bytes| <$Ti>::deserialize($idf, cql_bytes))
.map_err(|err| mk_deser_err::<Self>(
typ,
TupleDeserializationErrorKind::FieldDeserializationFailed {
position: $idx,
err,
}
)
)?,
{
let cql_bytes = if v.is_empty() {
// Special case: if there are no bytes left to read, consider the tuple element as null.
// This is needed to handle tuples with less elements than expected
// (see https://github.com/scylladb/scylla-rust-driver/issues/1452 for context).
None
} else {
v.read_cql_bytes().map_err(|err| DeserializationError::new(err))?
};

<$Ti>::deserialize($idf, cql_bytes)
.map_err(|err| mk_deser_err::<Self>(
typ,
TupleDeserializationErrorKind::FieldDeserializationFailed {
position: $idx,
err,
}
)
)?
},
)*
);
Ok(ret)
Expand Down
93 changes: 91 additions & 2 deletions scylla/tests/integration/types/cql_collections.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::utils::{
DeserializeOwnedValue, PerformDDL, create_new_session_builder, setup_tracing,
unique_keyspace_name,
DeserializeOwnedValue, PerformDDL, SerializeValueWithFakeType, create_new_session_builder,
setup_tracing, unique_keyspace_name,
};
use scylla::cluster::metadata::NativeType;
use scylla::deserialize::value::DeserializeValue;
Expand Down Expand Up @@ -254,6 +254,95 @@ async fn test_cql_tuple() {
.unwrap();
}

// Cassandra does not support altering column types starting with version 3.0.11 and 3.10.
// See https://stackoverflow.com/a/76926622 for explanation.
#[cfg_attr(cassandra_tests, ignore)]
#[tokio::test]
async fn test_alter_column_add_field_to_tuple() {
setup_tracing();
let session: Session = connect().await;

let table_name: &str = "test_cql_tuple_alter_tab";
create_table(&session, table_name, "tuple<int, int>").await;

let tuple1: (i32, i32) = (1, 2);
session
.query_unpaged(
format!("INSERT INTO {table_name} (p, val) VALUES (0, ?)"),
&(tuple1,),
)
.await
.unwrap();

// Add a field to the tuple. Existing rows will still have 2 fields in the tuple.
session
.query_unpaged(
format!("ALTER TABLE {table_name} ALTER val TYPE tuple<int, int, text>"),
&(),
)
.await
.unwrap();

// Select a tuple - ScyllaDB will send 2-element tuple.
// Driver should return the third element as null.
let selected_value: (Option<i32>, Option<i32>, Option<String>) = session
.query_unpaged(format!("SELECT val FROM {table_name} WHERE p = 0"), ())
.await
.unwrap()
.into_rows_result()
.unwrap()
.single_row::<((Option<i32>, Option<i32>, Option<String>),)>()
.unwrap()
.0;

assert!(selected_value.2.is_none());

session
.ddl(format!("DROP KEYSPACE {}", session.get_keyspace().unwrap()))
.await
.unwrap();
}

#[tokio::test]
async fn test_cql_tuple_db_repr_shorter_than_metadata() {
use ColumnType::*;
use NativeType::*;

setup_tracing();
let session: Session = connect().await;

{
let table_name: &str = "test_cql_shorter_tuple_tab";
create_table(
&session,
table_name,
"tuple<tuple<int, text, int>, int, tuple<int>>",
)
.await;

// We craft a tuple that is shorter in DB representation than in metadata,
// and also contains another nested tuple with the same property.
// In order to do that, we use SerializeValueWithFakeType to lie about the type
// the value is being serialized to, so that tuple serialization logic does not complain.
let inner_tuple: (i32, String) = (1, "Ala".to_owned());
let inner_tuple_ser =
SerializeValueWithFakeType::new(inner_tuple, Tuple(vec![Native(Int), Native(Text)]));
let tuple: (SerializeValueWithFakeType<_>, i32) = (inner_tuple_ser, 2);
let tuple_ser = SerializeValueWithFakeType::new(
tuple,
Tuple(vec![Tuple(vec![Native(Int), Native(Text)]), Native(Int)]),
);
// The expected deserialized tuple has None for the missing elements.
let tuple_deser = ((1, "Ala".to_owned(), None::<i32>), 2, None::<(i32,)>);
insert_and_select(&session, table_name, &tuple_ser, &tuple_deser).await;
}

session
.ddl(format!("DROP KEYSPACE {}", session.get_keyspace().unwrap()))
.await
.unwrap();
}

// TODO: Remove this ignore when vector type is supported in ScyllaDB
#[cfg_attr(not(cassandra_tests), ignore)]
#[tokio::test]
Expand Down
24 changes: 24 additions & 0 deletions scylla/tests/integration/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use scylla::client::session::Session;
use scylla::client::session_builder::{GenericSessionBuilder, SessionBuilderKind};
use scylla::cluster::ClusterState;
use scylla::cluster::NodeRef;
use scylla::cluster::metadata::ColumnType;
use scylla::deserialize::value::DeserializeValue;
use scylla::errors::{DbError, ExecutionError, RequestAttemptError};
use scylla::policies::load_balancing::{
Expand All @@ -16,6 +17,7 @@ use scylla::policies::retry::{RequestInfo, RetryDecision, RetryPolicy, RetrySess
use scylla::response::query_result::QueryResult;
use scylla::routing::Shard;
use scylla::serialize::row::SerializeRow;
use scylla::serialize::value::SerializeValue;
use scylla::statement::prepared::PreparedStatement;
use scylla::statement::unprepared::Statement;
use std::collections::HashMap;
Expand Down Expand Up @@ -455,3 +457,25 @@ pub(crate) async fn execute_unprepared_statement_everywhere(
})
.await
}

pub(crate) struct SerializeValueWithFakeType<'typ, T> {
fake_type: ColumnType<'typ>,
value: T,
}

impl<T: SerializeValue> SerializeValue for SerializeValueWithFakeType<'_, T> {
fn serialize<'b>(
&self,
_typ: &ColumnType,
writer: scylla_cql::serialize::CellWriter<'b>,
) -> Result<scylla::serialize::writers::WrittenCellProof<'b>, scylla::errors::SerializationError>
{
<T as SerializeValue>::serialize(&self.value, &self.fake_type, writer)
}
}

impl<'typ, T> SerializeValueWithFakeType<'typ, T> {
pub(crate) fn new(value: T, fake_type: ColumnType<'typ>) -> Self {
Self { fake_type, value }
}
}
Comment on lines +460 to +481
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I posted this util before I knew about the serialization mechanism that you found (driver allowing tuples shorter than in db).
Do we need this util if the driver allows shorter serialization itself?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The util is useful in wider set of cases than just shorter tuples, so I'd keep it anyway.
  2. I don't want to depend on the lenient behaviour of CqlValue::Tuple, as I don't know if this behaviour is desirable or not. If not, we'll eventually alter it, and the tests would then break.

Loading