Merge pull request #1453 from wprzytula/accept-shorter-encoded-tuples

deserialize/value: Accept shorter encoded tuples

Author: wprzytula

scylla-cql/src/deserialize/value.rs

@@ -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)

scylla/tests/integration/types/cql_collections.rs

@@ -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;
@@ -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]

scylla/tests/integration/utils.rs

@@ -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::{
@@ -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;
@@ -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 }
+    }
+}