Merge pull request #205 from muzarski/cass-result-cleanup

muzarski · web-flow · commit 739855afc615 · 2024-11-21T15:47:59.000+01:00
result: small cleanups before bump to 0.15
diff --git a/Makefile b/Makefile
@@ -26,7 +26,7 @@ SCYLLA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\
 :HeartbeatTests.Integration_Cassandra_HeartbeatFailed\
 :ExecutionProfileTest.InvalidName\
 :*NoCompactEnabledConnection\
-:PreparedMetadataTests.Integration_Cassandra_AlterDoesntUpdateColumnCount\
+:PreparedMetadataTests.Integration_Cassandra_AlterProperlyUpdatesColumnCount\
 :UseKeyspaceCaseSensitiveTests.Integration_Cassandra_ConnectWithKeyspace)
 endif
 
@@ -56,7 +56,7 @@ CASSANDRA_TEST_FILTER := $(subst ${SPACE},${EMPTY},ClusterTests.*\
 :SslTests.Integration_Cassandra_ReconnectAfterClusterCrashAndRestart\
 :ExecutionProfileTest.InvalidName\
 :*NoCompactEnabledConnection\
-:PreparedMetadataTests.Integration_Cassandra_AlterDoesntUpdateColumnCount\
+:PreparedMetadataTests.Integration_Cassandra_AlterProperlyUpdatesColumnCount\
 :UseKeyspaceCaseSensitiveTests.Integration_Cassandra_ConnectWithKeyspace)
 endif
 
diff --git a/scylla-rust-wrapper/src/cass_types.rs b/scylla-rust-wrapper/src/cass_types.rs
@@ -138,6 +138,12 @@ pub enum MapDataType {
     KeyAndValue(Arc<CassDataType>, Arc<CassDataType>),
 }
 
+#[derive(Debug)]
+pub struct CassColumnSpec {
+    pub name: String,
+    pub data_type: Arc<CassDataType>,
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum CassDataType {
     Value(CassValueType),
diff --git a/scylla-rust-wrapper/src/future.rs b/scylla-rust-wrapper/src/future.rs
@@ -398,7 +398,7 @@ pub unsafe extern "C" fn cass_future_tracing_id(
     tracing_id: *mut CassUuid,
 ) -> CassError {
     ptr_to_ref(future).with_waited_result(|r: &mut CassFutureResult| match r {
-        Ok(CassResultValue::QueryResult(result)) => match result.metadata.tracing_id {
+        Ok(CassResultValue::QueryResult(result)) => match result.tracing_id {
             Some(id) => {
                 *tracing_id = CassUuid::from(id);
                 CassError::CASS_OK
diff --git a/scylla-rust-wrapper/src/prepared.rs b/scylla-rust-wrapper/src/prepared.rs
@@ -5,6 +5,7 @@ use crate::{
     argconv::*,
     cass_error::CassError,
     cass_types::{get_column_type, CassDataType},
+    query_result::CassResultMetadata,
     statement::{CassStatement, Statement},
     types::size_t,
 };
@@ -14,11 +15,10 @@ use scylla::prepared_statement::PreparedStatement;
 pub struct CassPrepared {
     // Data types of columns from PreparedMetadata.
     pub variable_col_data_types: Vec<Arc<CassDataType>>,
-    // Data types of columns from ResultMetadata.
-    //
-    // Arc<CassDataType> -> to share each data type with other structs such as `CassValue`
-    // Arc<Vec<...>> -> to share the whole vector with `CassResultData`.
-    pub result_col_data_types: Arc<Vec<Arc<CassDataType>>>,
+
+    // Cached result metadata. Arc'ed since we want to share it
+    // with result metadata after execution.
+    pub result_metadata: Arc<CassResultMetadata>,
     pub statement: PreparedStatement,
 }
 
@@ -30,17 +30,13 @@ impl CassPrepared {
             .map(|col_spec| Arc::new(get_column_type(col_spec.typ())))
             .collect();
 
-        let result_col_data_types: Arc<Vec<Arc<CassDataType>>> = Arc::new(
-            statement
-                .get_result_set_col_specs()
-                .iter()
-                .map(|col_spec| Arc::new(get_column_type(col_spec.typ())))
-                .collect(),
-        );
+        let result_metadata = Arc::new(CassResultMetadata::from_column_specs(
+            statement.get_result_set_col_specs(),
+        ));
 
         Self {
             variable_col_data_types,
-            result_col_data_types,
+            result_metadata,
             statement,
         }
     }
diff --git a/scylla-rust-wrapper/src/query_result.rs b/scylla-rust-wrapper/src/query_result.rs
@@ -1,7 +1,7 @@
 use crate::argconv::*;
 use crate::cass_error::CassError;
 use crate::cass_types::{
-    cass_data_type_type, get_column_type, CassDataType, CassValueType, MapDataType,
+    cass_data_type_type, get_column_type, CassColumnSpec, CassDataType, CassValueType, MapDataType,
 };
 use crate::inet::CassInet;
 use crate::metadata::{
@@ -18,50 +18,37 @@ use uuid::Uuid;
 
 pub struct CassResult {
     pub rows: Option<Vec<CassRow>>,
-    pub metadata: Arc<CassResultData>,
+    pub metadata: Arc<CassResultMetadata>,
+    pub tracing_id: Option<Uuid>,
+    pub paging_state_response: PagingStateResponse,
 }
 
-pub struct CassResultData {
-    pub paging_state_response: PagingStateResponse,
-    pub col_specs: Vec<ColumnSpec<'static>>,
-    pub col_data_types: Arc<Vec<Arc<CassDataType>>>,
-    pub tracing_id: Option<Uuid>,
+#[derive(Debug)]
+pub struct CassResultMetadata {
+    pub col_specs: Vec<CassColumnSpec>,
 }
 
-impl CassResultData {
-    pub fn from_result_payload(
-        paging_state_response: PagingStateResponse,
-        col_specs: Vec<ColumnSpec<'static>>,
-        maybe_col_data_types: Option<Arc<Vec<Arc<CassDataType>>>>,
-        tracing_id: Option<Uuid>,
-    ) -> CassResultData {
-        // `maybe_col_data_types` is:
-        // - Some(_) for prepared statements executions
-        // - None for unprepared (simple) queries executions
-        let col_data_types = maybe_col_data_types.unwrap_or_else(|| {
-            // This allocation is unfortunately necessary, because of the type of CassResultData::col_data_types.
-            Arc::new(
-                col_specs
-                    .iter()
-                    .map(|col_spec| Arc::new(get_column_type(col_spec.typ())))
-                    .collect(),
-            )
-        });
-
-        CassResultData {
-            paging_state_response,
-            col_specs,
-            col_data_types,
-            tracing_id,
-        }
+impl CassResultMetadata {
+    pub fn from_column_specs(col_specs: &[ColumnSpec<'_>]) -> CassResultMetadata {
+        let col_specs = col_specs
+            .iter()
+            .map(|col_spec| {
+                let name = col_spec.name().to_owned();
+                let data_type = Arc::new(get_column_type(col_spec.typ()));
+
+                CassColumnSpec { name, data_type }
+            })
+            .collect();
+
+        CassResultMetadata { col_specs }
     }
 }
 
 /// The lifetime of CassRow is bound to CassResult.
 /// It will be freed, when CassResult is freed.(see #[cass_result_free])
 pub struct CassRow {
     pub columns: Vec<CassValue>,
-    pub result_metadata: Arc<CassResultData>,
+    pub result_metadata: Arc<CassResultMetadata>,
 }
 
 pub enum Value {
@@ -851,7 +838,7 @@ pub unsafe extern "C" fn cass_result_free(result_raw: *const CassResult) {
 #[no_mangle]
 pub unsafe extern "C" fn cass_result_has_more_pages(result: *const CassResult) -> cass_bool_t {
     let result = ptr_to_ref(result);
-    (!result.metadata.paging_state_response.finished()) as cass_bool_t
+    (!result.paging_state_response.finished()) as cass_bool_t
 }
 
 #[no_mangle]
@@ -902,9 +889,9 @@ pub unsafe extern "C" fn cass_row_get_column_by_name_n(
         .col_specs
         .iter()
         .enumerate()
-        .find(|(_, spec)| {
-            is_case_sensitive && spec.name() == name_str
-                || !is_case_sensitive && spec.name().eq_ignore_ascii_case(name_str)
+        .find(|(_, col_spec)| {
+            is_case_sensitive && col_spec.name == name_str
+                || !is_case_sensitive && col_spec.name.eq_ignore_ascii_case(name_str)
         })
         .map(|(index, _)| {
             return match row_from_raw.columns.get(index) {
@@ -929,8 +916,12 @@ pub unsafe extern "C" fn cass_result_column_name(
         return CassError::CASS_ERROR_LIB_INDEX_OUT_OF_BOUNDS;
     }
 
-    let column_spec: &ColumnSpec = result_from_raw.metadata.col_specs.get(index_usize).unwrap();
-    let column_name = column_spec.name();
+    let column_name = &result_from_raw
+        .metadata
+        .col_specs
+        .get(index_usize)
+        .unwrap()
+        .name;
 
     write_str_to_c(column_name, name, name_length);
 
@@ -961,9 +952,9 @@ pub unsafe extern "C" fn cass_result_column_data_type(
 
     result_from_raw
         .metadata
-        .col_data_types
+        .col_specs
         .get(index_usize)
-        .map(Arc::as_ptr)
+        .map(|col_spec| Arc::as_ptr(&col_spec.data_type))
         .unwrap_or(std::ptr::null())
 }
 
@@ -1365,7 +1356,7 @@ pub unsafe extern "C" fn cass_result_paging_state_token(
 
     let result_from_raw = ptr_to_ref(result);
 
-    match &result_from_raw.metadata.paging_state_response {
+    match &result_from_raw.paging_state_response {
         PagingStateResponse::HasMorePages { state } => match state.as_bytes_slice() {
             Some(result_paging_state) => {
                 *paging_state_size = result_paging_state.len() as u64;
@@ -1404,7 +1395,9 @@ mod tests {
         session::create_cass_rows_from_rows,
     };
 
-    use super::{cass_result_column_count, cass_result_column_type, CassResult, CassResultData};
+    use super::{
+        cass_result_column_count, cass_result_column_type, CassResult, CassResultMetadata,
+    };
 
     fn col_spec(name: &'static str, typ: ColumnType<'static>) -> ColumnSpec<'static> {
         ColumnSpec::borrowed(name, typ, TableSpec::borrowed("ks", "tbl"))
@@ -1414,19 +1407,14 @@ mod tests {
     const SECOND_COLUMN_NAME: &str = "varint_col";
     const THIRD_COLUMN_NAME: &str = "list_double_col";
     fn create_cass_rows_result() -> CassResult {
-        let metadata = Arc::new(CassResultData::from_result_payload(
-            PagingStateResponse::NoMorePages,
-            vec![
-                col_spec(FIRST_COLUMN_NAME, ColumnType::BigInt),
-                col_spec(SECOND_COLUMN_NAME, ColumnType::Varint),
-                col_spec(
-                    THIRD_COLUMN_NAME,
-                    ColumnType::List(Box::new(ColumnType::Double)),
-                ),
-            ],
-            None,
-            None,
-        ));
+        let metadata = Arc::new(CassResultMetadata::from_column_specs(&[
+            col_spec(FIRST_COLUMN_NAME, ColumnType::BigInt),
+            col_spec(SECOND_COLUMN_NAME, ColumnType::Varint),
+            col_spec(
+                THIRD_COLUMN_NAME,
+                ColumnType::List(Box::new(ColumnType::Double)),
+            ),
+        ]));
 
         let rows = create_cass_rows_from_rows(
             vec![Row {
@@ -1446,6 +1434,8 @@ mod tests {
         CassResult {
             rows: Some(rows),
             metadata,
+            tracing_id: None,
+            paging_state_response: PagingStateResponse::NoMorePages,
         }
     }
 
@@ -1532,15 +1522,12 @@ mod tests {
     }
 
     fn create_non_rows_cass_result() -> CassResult {
-        let metadata = Arc::new(CassResultData::from_result_payload(
-            PagingStateResponse::NoMorePages,
-            vec![],
-            None,
-            None,
-        ));
+        let metadata = Arc::new(CassResultMetadata::from_column_specs(&[]));
         CassResult {
             rows: None,
             metadata,
+            tracing_id: None,
+            paging_state_response: PagingStateResponse::NoMorePages,
         }
     }
 
diff --git a/scylla-rust-wrapper/src/session.rs b/scylla-rust-wrapper/src/session.rs
diff --git a/scylla-rust-wrapper/src/statement.rs b/scylla-rust-wrapper/src/statement.rs
