From 6b72adbb9923dfd6225b532cf8cfd810eb0e3e5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wojciech=20Przytu=C5=82a?= Date: Mon, 2 Dec 2024 14:14:37 +0100 Subject: [PATCH] frame/result: allow differing TableSpecs in PreparedMetadata 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. --- scylla-cql/src/frame/response/result.rs | 41 +++++++++++++++---------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/scylla-cql/src/frame/response/result.rs b/scylla-cql/src/frame/response/result.rs index 4858d0efc1..4120a9a6a8 100644 --- a/scylla-cql/src/frame/response/result.rs +++ b/scylla-cql/src/frame/response/result.rs @@ -992,6 +992,7 @@ fn deser_table_spec_for_col_spec<'frame>( global_table_spec_provided: bool, known_table_spec: &'_ mut Option>, col_idx: usize, + expect_all_table_specs_the_same: bool, ) -> StdResult, ColumnSpecParseError> { let table_spec = match known_table_spec { // If global table spec was provided, we simply clone it to each column spec. @@ -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 @@ -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, CqlTypeParseError>, + expect_all_table_specs_the_same: bool, ) -> StdResult>, ColumnSpecParseError> { let global_table_spec_provided = global_table_spec.is_some(); let mut known_table_spec = global_table_spec; @@ -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))?; @@ -1072,6 +1077,7 @@ fn deser_col_specs_borrowed<'frame>( buf: &mut &'frame [u8], global_table_spec: Option>, col_count: usize, + expect_all_table_specs_the_same: bool, ) -> StdResult>, ColumnSpecParseError> { deser_col_specs_generic( buf, @@ -1079,6 +1085,7 @@ fn deser_col_specs_borrowed<'frame>( col_count, ColumnSpec::borrowed, deser_type_borrowed, + expect_all_table_specs_the_same, ) } @@ -1095,6 +1102,7 @@ fn deser_col_specs_owned<'frame>( buf: &mut &'frame [u8], global_table_spec: Option>, col_count: usize, + expect_all_table_specs_the_same: bool, ) -> StdResult>, ColumnSpecParseError> { let result: StdResult>, ColumnSpecParseError> = deser_col_specs_generic( buf, @@ -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 @@ -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 { @@ -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, @@ -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,