Skip to content

Commit

Permalink
frame/result: allow differing TableSpecs in PreparedMetadata
Browse files Browse the repository at this point in the history
As noted in #1134, when preparing batches containing requests to
multiple to different tables, the PreparedMetadata received upon
preparation contains differing TableSpecs (i.e., TableSpecs with more
than table mentioned).
This fails the check that we introduced with ResultMetadata in mind:
we've been assuming that all TableSpecs are the same, because ScyllaDB/
Cassandra has no support for JOINs.
Not to fail preparation of cross-table batches, the check is now only
turned on for ResultMetadata and turned off for PreparedMetadata.
  • Loading branch information
wprzytula committed Dec 2, 2024
1 parent 49c342e commit eade6b6
Showing 1 changed file with 25 additions and 16 deletions.
41 changes: 25 additions & 16 deletions scylla-cql/src/frame/response/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,7 @@ fn deser_table_spec_for_col_spec<'frame>(
global_table_spec_provided: bool,
known_table_spec: &'_ mut Option<TableSpec<'frame>>,
col_idx: usize,
expect_all_table_specs_the_same: bool,
) -> StdResult<TableSpec<'frame>, ColumnSpecParseError> {
let table_spec = match known_table_spec {
// If global table spec was provided, we simply clone it to each column spec.
Expand All @@ -1005,19 +1006,21 @@ fn deser_table_spec_for_col_spec<'frame>(
deser_table_spec(buf).map_err(|err| mk_col_spec_parse_error(col_idx, err))?;

if let Some(ref known_spec) = known_table_spec {
// We assume that for each column, table spec is the same.
// As this is not guaranteed by the CQL protocol specification but only by how
// Cassandra and ScyllaDB work (no support for joins), we perform a sanity check here.
if known_spec.table_name != table_spec.table_name
|| known_spec.ks_name != table_spec.ks_name
{
return Err(mk_col_spec_parse_error(
col_idx,
ColumnSpecParseErrorKind::TableSpecDiffersAcrossColumns(
known_spec.clone().into_owned(),
table_spec.into_owned(),
),
));
if expect_all_table_specs_the_same {
// In case of ResultMetadata, we assume that for each column, table spec is the same.
// As this is not guaranteed by the CQL protocol specification but only by how
// Cassandra and ScyllaDB work (no support for joins), we perform a sanity check here.
if known_spec.table_name != table_spec.table_name
|| known_spec.ks_name != table_spec.ks_name
{
return Err(mk_col_spec_parse_error(
col_idx,
ColumnSpecParseErrorKind::TableSpecDiffersAcrossColumns(
known_spec.clone().into_owned(),
table_spec.into_owned(),
),
));
}
}
} else {
// Once we have read the first column spec, we save its table spec
Expand All @@ -1038,6 +1041,7 @@ fn deser_col_specs_generic<'frame, 'result>(
col_count: usize,
make_col_spec: fn(&'frame str, ColumnType<'result>, TableSpec<'frame>) -> ColumnSpec<'result>,
deser_type: fn(&mut &'frame [u8]) -> StdResult<ColumnType<'result>, CqlTypeParseError>,
expect_all_table_specs_the_same: bool,
) -> StdResult<Vec<ColumnSpec<'result>>, ColumnSpecParseError> {
let global_table_spec_provided = global_table_spec.is_some();
let mut known_table_spec = global_table_spec;
Expand All @@ -1049,6 +1053,7 @@ fn deser_col_specs_generic<'frame, 'result>(
global_table_spec_provided,
&mut known_table_spec,
col_idx,
expect_all_table_specs_the_same,
)?;

let name = types::read_string(buf).map_err(|err| mk_col_spec_parse_error(col_idx, err))?;
Expand All @@ -1072,13 +1077,15 @@ fn deser_col_specs_borrowed<'frame>(
buf: &mut &'frame [u8],
global_table_spec: Option<TableSpec<'frame>>,
col_count: usize,
expect_all_table_specs_the_same: bool,
) -> StdResult<Vec<ColumnSpec<'frame>>, ColumnSpecParseError> {
deser_col_specs_generic(
buf,
global_table_spec,
col_count,
ColumnSpec::borrowed,
deser_type_borrowed,
expect_all_table_specs_the_same,
)
}

Expand All @@ -1095,6 +1102,7 @@ fn deser_col_specs_owned<'frame>(
buf: &mut &'frame [u8],
global_table_spec: Option<TableSpec<'frame>>,
col_count: usize,
expect_all_table_specs_the_same: bool,
) -> StdResult<Vec<ColumnSpec<'static>>, ColumnSpecParseError> {
let result: StdResult<Vec<ColumnSpec<'static>>, ColumnSpecParseError> = deser_col_specs_generic(
buf,
Expand All @@ -1104,6 +1112,7 @@ fn deser_col_specs_owned<'frame>(
ColumnSpec::owned(name.to_owned(), typ, table_spec.into_owned())
},
deser_type_owned,
expect_all_table_specs_the_same,
);

result
Expand Down Expand Up @@ -1134,7 +1143,7 @@ fn deser_result_metadata(
.then(|| deser_table_spec(buf))
.transpose()?;

deser_col_specs_owned(buf, global_table_spec, col_count)?
deser_col_specs_owned(buf, global_table_spec, col_count, true)?
};

let metadata = ResultMetadata {
Expand Down Expand Up @@ -1203,7 +1212,7 @@ impl RawMetadataAndRawRows {
.then(|| deser_table_spec(buf))
.transpose()?;

let col_specs = deser_col_specs_borrowed(buf, global_table_spec, col_count)?;
let col_specs = deser_col_specs_borrowed(buf, global_table_spec, col_count, true)?;

ResultMetadata {
col_count,
Expand Down Expand Up @@ -1303,7 +1312,7 @@ fn deser_prepared_metadata(
.then(|| deser_table_spec(buf))
.transpose()?;

let col_specs = deser_col_specs_owned(buf, global_table_spec, col_count)?;
let col_specs = deser_col_specs_owned(buf, global_table_spec, col_count, false)?;

Ok(PreparedMetadata {
flags,
Expand Down

0 comments on commit eade6b6

Please sign in to comment.