Skip to content

deserialize/value: Accept shorter encoded tuples #1453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wprzytula merged 4 commits into from
Oct 29, 2025
Merged

deserialize/value: Accept shorter encoded tuples #1453

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d216b0
deserialize/value: accept shorter encoded tuples
wprzytula Oct 23, 2025
b17ab6f
integration: add regression test for shorter tuples
wprzytula Oct 23, 2025
7f16f81
integration: introduce SerializeValueWithFakeType
wprzytula Oct 28, 2025
79ac7b1
integration: add test for nested shorter tuples
wprzytula Oct 28, 2025
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
24 changes: 24 additions & 0 deletions scylla/tests/integration/utils.rs
View file
Open in desktop
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

@Lorak-mmk Lorak-mmk Oct 29, 2025

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

@wprzytula wprzytula Oct 29, 2025

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.