From eade6b66c675bcffc7d5e7f35f097c1c17adee02 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 2c5a76e29..76210697a 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,