From e1b50227f857b643c69bc841e41ca08bfa5fc6d7 Mon Sep 17 00:00:00 2001
From: Marco Neumann <marco@crepererum.net>
Date: Wed, 19 Oct 2022 06:28:15 +0000
Subject: [PATCH] refactor: avoid some clones while caching ns schema (#5896)

Found while reviewing the code.

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
---
 querier/src/cache/namespace.rs     | 21 ++++++++-------------
 querier/src/chunk/mod.rs           |  2 +-
 querier/src/namespace/test_util.rs |  2 +-
 query_tests/src/scenarios/util.rs  |  2 +-
 4 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/querier/src/cache/namespace.rs b/querier/src/cache/namespace.rs
index 7f7d55414d..d0f59b8916 100644
--- a/querier/src/cache/namespace.rs
+++ b/querier/src/cache/namespace.rs
@@ -94,7 +94,7 @@ impl NamespaceCache {
                     .await
                     .expect("retry forever")?;
 
-                Some(Arc::new((&schema).into()))
+                Some(Arc::new(schema.into()))
             }
         });
         let loader = Arc::new(MetricsLoader::new(
@@ -214,8 +214,8 @@ impl CachedTable {
     }
 }
 
-impl From<&TableSchema> for CachedTable {
-    fn from(table: &TableSchema) -> Self {
+impl From<TableSchema> for CachedTable {
+    fn from(table: TableSchema) -> Self {
         let mut column_id_map: HashMap<ColumnId, Arc<str>> = table
             .columns
             .iter()
@@ -225,12 +225,7 @@ impl From<&TableSchema> for CachedTable {
 
         Self {
             id: table.id,
-            schema: Arc::new(
-                table
-                    .clone()
-                    .try_into()
-                    .expect("Catalog table schema broken"),
-            ),
+            schema: Arc::new(table.try_into().expect("Catalog table schema broken")),
             column_id_map,
         }
     }
@@ -254,14 +249,14 @@ impl CachedNamespace {
     }
 }
 
-impl From<&NamespaceSchema> for CachedNamespace {
-    fn from(ns: &NamespaceSchema) -> Self {
+impl From<NamespaceSchema> for CachedNamespace {
+    fn from(ns: NamespaceSchema) -> Self {
         let mut tables: HashMap<Arc<str>, Arc<CachedTable>> = ns
             .tables
-            .iter()
+            .into_iter()
             .map(|(name, table)| {
                 let table: CachedTable = table.into();
-                (Arc::from(name.clone()), Arc::new(table))
+                (Arc::from(name), Arc::new(table))
             })
             .collect();
         tables.shrink_to_fit();
diff --git a/querier/src/chunk/mod.rs b/querier/src/chunk/mod.rs
index 8156d7cdb2..1793f90786 100644
--- a/querier/src/chunk/mod.rs
+++ b/querier/src/chunk/mod.rs
@@ -474,7 +474,7 @@ pub mod tests {
         }
 
         async fn chunk(&self, namespace_schema: Arc<NamespaceSchema>) -> QuerierChunk {
-            let cached_namespace: CachedNamespace = namespace_schema.as_ref().into();
+            let cached_namespace: CachedNamespace = namespace_schema.as_ref().clone().into();
             let cached_table = cached_namespace.tables.get("table").expect("table exists");
             self.adapter
                 .new_chunk(
diff --git a/querier/src/namespace/test_util.rs b/querier/src/namespace/test_util.rs
index 666a73a617..0cf5671ec5 100644
--- a/querier/src/namespace/test_util.rs
+++ b/querier/src/namespace/test_util.rs
@@ -23,7 +23,7 @@ pub async fn querier_namespace_with_limit(
     let schema = get_schema_by_name(&ns.namespace.name, repos.as_mut())
         .await
         .unwrap();
-    let cached_ns = Arc::new(CachedNamespace::from(&schema));
+    let cached_ns = Arc::new(CachedNamespace::from(schema));
 
     let catalog_cache = Arc::new(QuerierCatalogCache::new_testing(
         ns.catalog.catalog(),
diff --git a/query_tests/src/scenarios/util.rs b/query_tests/src/scenarios/util.rs
index 5d813486d1..c23bd3f2fd 100644
--- a/query_tests/src/scenarios/util.rs
+++ b/query_tests/src/scenarios/util.rs
@@ -930,7 +930,7 @@ impl MockIngester {
             catalog_cache,
             catalog.metric_registry(),
             ns.namespace.name.clone().into(),
-            Arc::new(schema.as_ref().into()),
+            Arc::new(schema.as_ref().clone().into()),
             catalog.exec(),
             Some(ingester_connection),
             sharder,