Don't expose pub fields

Xuanwo · Xuanwo · commit 39ed93de96dd · 2022-09-19T20:22:49.000+08:00
Signed-off-by: Xuanwo &lt;github@xuanwo.io&gt;
diff --git a/src/query/planner/src/metadata.rs b/src/query/planner/src/metadata.rs
@@ -238,14 +238,14 @@ impl TableEntry {
 
 #[derive(Clone, Debug)]
 pub struct ColumnEntry {
-    pub column_index: IndexType,
-    pub name: String,
-    pub data_type: DataTypeImpl,
+    column_index: IndexType,
+    name: String,
+    data_type: DataTypeImpl,
 
     /// Table index of column entry. None if column is derived from a subquery.
-    pub table_index: Option<IndexType>,
+    table_index: Option<IndexType>,
     /// Path indices for inner column of struct data type.
-    pub path_indices: Option<Vec<IndexType>>,
+    path_indices: Option<Vec<IndexType>>,
 }
 
 impl ColumnEntry {
@@ -264,6 +264,36 @@ impl ColumnEntry {
             path_indices,
         }
     }
+
+    /// Get the name of this column entry.
+    pub fn name(&self) -> &str {
+        &self.name
+    }
+
+    /// Get the index of this column entry.
+    pub fn index(&self) -> IndexType {
+        self.column_index
+    }
+
+    /// Get the data type of this column entry.
+    pub fn data_type(&self) -> &DataTypeImpl {
+        &self.data_type
+    }
+
+    /// Get the table index of this column entry.
+    pub fn table_index(&self) -> Option<IndexType> {
+        self.table_index
+    }
+
+    /// Get the path indices of this column entry.
+    pub fn path_indices(&self) -> Option<&[IndexType]> {
+        self.path_indices.as_deref()
+    }
+
+    /// Check if this column entry contains path_indices
+    pub fn has_path_indices(&self) -> bool {
+        self.path_indices.is_some()
+    }
 }
 
 /// TODO(xuanwo): migrate this as a function of metadata.
diff --git a/src/query/service/src/sql/executor/expression_builder.rs b/src/query/service/src/sql/executor/expression_builder.rs
@@ -63,11 +63,9 @@ where ExpressionBuilder<T>: FiledNameFormat
 
     pub fn build_and_rename(&self, scalar: &Scalar, index: IndexType) -> Result<Expression> {
         let expr = self.build(scalar)?;
-        let name = self.metadata.read().column(index).name.clone();
-        Ok(Expression::Alias(
-            Self::format(name.as_str(), index),
-            Box::new(expr),
-        ))
+        let metadata = self.metadata.read();
+        let name = metadata.column(index).name();
+        Ok(Expression::Alias(Self::format(name, index), Box::new(expr)))
     }
 
     pub fn build(&self, scalar: &Scalar) -> Result<Expression> {
@@ -137,8 +135,9 @@ where ExpressionBuilder<T>: FiledNameFormat
     }
 
     pub fn build_column_ref(&self, index: IndexType) -> Result<Expression> {
-        let name = self.metadata.read().column(index).name.clone();
-        Ok(Expression::Column(Self::format(name.as_str(), index)))
+        let metadata = self.metadata.read();
+        let name = metadata.column(index).name();
+        Ok(Expression::Column(Self::format(name, index)))
     }
 
     pub fn build_literal(
diff --git a/src/query/service/src/sql/executor/format.rs b/src/query/service/src/sql/executor/format.rs
@@ -135,7 +135,7 @@ fn project_to_format_tree(
         .columns
         .iter()
         .sorted()
-        .map(|column| format!("{} (#{})", metadata.read().column(*column).name, column))
+        .map(|column| format!("{} (#{})", metadata.read().column(*column).name(), column))
         .collect::<Vec<_>>()
         .join(", ");
     Ok(FormatTreeNode::with_children("Project".to_string(), vec![
@@ -173,7 +173,7 @@ fn aggregate_partial_to_format_tree(
         .map(|column| {
             let index = column.parse::<IndexType>()?;
             let column = metadata.read().column(index).clone();
-            Ok(column.name)
+            Ok(column.name().to_string())
         })
         .collect::<Result<Vec<_>>>()?
         .join(", ");
@@ -204,7 +204,7 @@ fn aggregate_final_to_format_tree(
         .map(|column| {
             let index = column.parse::<IndexType>()?;
             let column = metadata.read().column(index).clone();
-            Ok(column.name)
+            Ok(column.name().to_string())
         })
         .collect::<Result<Vec<_>>>()?
         .join(", ");
@@ -234,7 +234,7 @@ fn sort_to_format_tree(plan: &Sort, metadata: &MetadataRef) -> Result<FormatTree
             let column = metadata.read().column(index).clone();
             Ok(format!(
                 "{} {} {}",
-                column.name,
+                column.name(),
                 if sort_key.asc { "ASC" } else { "DESC" },
                 if sort_key.nulls_first {
                     "NULLS FIRST"
diff --git a/src/query/service/src/sql/executor/physical_plan_builder.rs b/src/query/service/src/sql/executor/physical_plan_builder.rs
@@ -26,6 +26,7 @@ use common_legacy_planners::Extras;
 use common_legacy_planners::PrewhereInfo;
 use common_legacy_planners::Projection;
 use common_legacy_planners::StageKind;
+use common_planner::IndexType;
 use common_planner::Metadata;
 use common_planner::MetadataRef;
 use common_planner::DUMMY_TABLE_INDEX;
@@ -83,7 +84,7 @@ impl PhysicalPlanBuilder {
             let col_indices = columns
                 .iter()
                 .map(|index| {
-                    let name = metadata.column(*index).name.as_str();
+                    let name = metadata.column(*index).name();
                     schema.index_of(name).unwrap()
                 })
                 .sorted()
@@ -94,17 +95,17 @@ impl PhysicalPlanBuilder {
                 .iter()
                 .map(|index| {
                     let column = metadata.column(*index);
-                    match &column.path_indices {
-                        Some(path_indices) => (column.column_index, path_indices.clone()),
+                    match &column.path_indices() {
+                        Some(path_indices) => (column.index(), path_indices.to_vec()),
                         None => {
-                            let name = metadata.column(*index).name.as_str();
+                            let name = metadata.column(*index).name();
                             let idx = schema.index_of(name).unwrap();
-                            (column.column_index, vec![idx])
+                            (column.index(), vec![idx])
                         }
                     }
                 })
                 .sorted()
-                .collect::<BTreeMap<_, _>>();
+                .collect::<BTreeMap<_, Vec<IndexType>>>();
             Projection::InnerColumns(col_indices)
         }
     }
@@ -120,17 +121,17 @@ impl PhysicalPlanBuilder {
                 let metadata = self.metadata.read().clone();
                 for index in scan.columns.iter() {
                     let column = metadata.column(*index);
-                    if column.path_indices.is_some() {
+                    if column.has_path_indices() {
                         has_inner_column = true;
                     }
                     if let Some(prewhere) = &scan.prewhere {
                         // if there is a prewhere optimization,
                         // we can prune `PhysicalScan`'s ouput schema.
                         if prewhere.output_columns.contains(index) {
-                            name_mapping.insert(column.name.clone(), index.to_string());
+                            name_mapping.insert(column.name().to_string(), index.to_string());
                         }
                     } else {
-                        let name = column.name.clone();
+                        let name = column.name().to_string();
                         name_mapping.insert(name, index.to_string());
                     }
                 }
diff --git a/src/query/service/src/sql/executor/physical_scalar.rs b/src/query/service/src/sql/executor/physical_scalar.rs
@@ -70,13 +70,13 @@ impl PhysicalScalar {
 
                 let index = column_id.parse::<IndexType>()?;
                 let column = metadata.read().column(index).clone();
-                let table_name = match column.table_index {
+                let table_name = match column.table_index() {
                     Some(table_index) => {
                         format!("{}.", metadata.read().table(table_index).name())
                     }
                     None => "".to_string(),
                 };
-                Ok(format!("{}{} (#{})", table_name, column.name, index))
+                Ok(format!("{}{} (#{})", table_name, column.name(), index))
             }
             PhysicalScalar::Constant { value, .. } => Ok(value.to_string()),
             PhysicalScalar::Function { name, args, .. } => {
@@ -114,7 +114,7 @@ impl AggregateFunctionDesc {
                 .map(|arg| {
                     let index = arg.parse::<IndexType>()?;
                     let column = metadata.read().column(index).clone();
-                    Ok(column.name)
+                    Ok(column.name().to_string())
                 })
                 .collect::<Result<Vec<_>>>()?
                 .join(", ")
diff --git a/src/query/service/src/sql/planner/binder/table.rs b/src/query/service/src/sql/planner/binder/table.rs
@@ -295,10 +295,10 @@ impl<'a> Binder {
             let column_binding = ColumnBinding {
                 database_name: Some(database_name.to_string()),
                 table_name: Some(table.name().to_string()),
-                column_name: column.name.clone(),
-                index: column.column_index,
-                data_type: Box::new(column.data_type.clone()),
-                visibility: if column.path_indices.is_some() {
+                column_name: column.name().to_string(),
+                index: column.index(),
+                data_type: Box::new(column.data_type().clone()),
+                visibility: if column.has_path_indices() {
                     Visibility::InVisible
                 } else {
                     Visibility::Visible
@@ -311,7 +311,7 @@ impl<'a> Binder {
             SExpr::create_leaf(
                 LogicalGet {
                     table_index,
-                    columns: columns.into_iter().map(|col| col.column_index).collect(),
+                    columns: columns.into_iter().map(|col| col.index()).collect(),
                     push_down_predicates: None,
                     limit: None,
                     order_by: None,
diff --git a/src/query/service/src/sql/planner/format/display_rel_operator.rs b/src/query/service/src/sql/planner/format/display_rel_operator.rs
@@ -270,7 +270,7 @@ fn physical_scan_to_format_tree(
                             .iter()
                             .map(|item| format!(
                                 "{} (#{}) {}",
-                                metadata.read().column(item.index).name.clone(),
+                                metadata.read().column(item.index).name(),
                                 item.index,
                                 if item.asc { "ASC" } else { "DESC" }
                             ))
@@ -328,7 +328,7 @@ fn logical_get_to_format_tree(
                             .iter()
                             .map(|item| format!(
                                 "{} (#{}) {}",
-                                metadata.read().column(item.index).name.clone(),
+                                metadata.read().column(item.index).name(),
                                 item.index,
                                 if item.asc { "ASC" } else { "DESC" }
                             ))
@@ -560,7 +560,7 @@ fn project_to_format_tree(
         .read()
         .columns()
         .iter()
-        .map(|entry| format!("{} (#{})", entry.name.clone(), entry.column_index))
+        .map(|entry| format!("{} (#{})", entry.name(), entry.index()))
         .collect::<Vec<String>>();
     // Sorted by column index to make display of Project stable
     let project_columns = op
@@ -595,7 +595,8 @@ fn sort_to_format_tree(
         .items
         .iter()
         .map(|item| {
-            let name = metadata.read().column(item.index).name.clone();
+            let metadata = metadata.read();
+            let name = metadata.column(item.index).name();
             format!(
                 "{} (#{}) {}",
                 name,
diff --git a/src/query/service/src/sql/planner/metadata.rs b/src/query/service/src/sql/planner/metadata.rs
@@ -30,16 +30,16 @@ pub fn find_smallest_column(entries: &[ColumnEntry]) -> usize {
     debug_assert!(!entries.is_empty());
     let mut column_indexes = entries
         .iter()
-        .map(|entry| entry.column_index)
+        .map(|entry| entry.index())
         .collect::<Vec<IndexType>>();
     column_indexes.sort();
     let mut smallest_index = column_indexes[0];
     let mut smallest_size = usize::MAX;
     for (idx, column_entry) in entries.iter().enumerate() {
-        if let Ok(bytes) = column_entry.data_type.data_type_id().numeric_byte_size() {
+        if let Ok(bytes) = column_entry.data_type().data_type_id().numeric_byte_size() {
             if smallest_size > bytes {
                 smallest_size = bytes;
-                smallest_index = entries[idx].column_index;
+                smallest_index = entries[idx].index();
             }
         }
     }
diff --git a/src/query/service/src/sql/planner/optimizer/heuristic/decorrelate.rs b/src/query/service/src/sql/planner/optimizer/heuristic/decorrelate.rs
@@ -443,12 +443,12 @@ impl SubqueryRewriter {
                 self.derived_columns.insert(
                     *correlated_column,
                     metadata.add_column(
-                        column_entry.name.clone(),
-                        if let DataTypeImpl::Nullable(_) = column_entry.data_type {
-                            column_entry.data_type.clone()
+                        column_entry.name().to_string(),
+                        if let DataTypeImpl::Nullable(_) = column_entry.data_type() {
+                            column_entry.data_type().clone()
                         } else {
                             DataTypeImpl::Nullable(NullableType::create(
-                                column_entry.data_type.clone(),
+                                column_entry.data_type().clone(),
                             ))
                         },
                         None,
@@ -495,8 +495,8 @@ impl SubqueryRewriter {
                                 database_name: None,
                                 table_name: None,
                                 column_name: "".to_string(),
-                                index: column_entry.column_index,
-                                data_type: Box::from(column_entry.data_type.clone()),
+                                index: column_entry.index(),
+                                data_type: Box::from(column_entry.data_type().clone()),
                                 visibility: Visibility::Visible,
                             },
                         })
@@ -529,7 +529,7 @@ impl SubqueryRewriter {
                         table_name: None,
                         column_name: format!("subquery_{}", derived_column),
                         index: *derived_column,
-                        data_type: Box::from(column_entry.data_type.clone()),
+                        data_type: Box::from(column_entry.data_type().clone()),
                         visibility: Visibility::Visible,
                     };
                     items.push(ScalarItem {
@@ -598,7 +598,7 @@ impl SubqueryRewriter {
                             table_name: None,
                             column_name: format!("subquery_{}", derived_column),
                             index: *derived_column,
-                            data_type: Box::from(column_entry.data_type.clone()),
+                            data_type: Box::from(column_entry.data_type().clone()),
                             visibility: Visibility::Visible,
                         }
                     };
@@ -767,7 +767,7 @@ impl SubqueryRewriter {
             let data_type = {
                 let metadata = self.metadata.read();
                 let column_entry = metadata.column(*correlated_column);
-                column_entry.data_type.clone()
+                column_entry.data_type().clone()
             };
             let right_column = Scalar::BoundColumnRef(BoundColumnRef {
                 column: ColumnBinding {
