From e920f95902c83ae1e2e4aac91da4dc50cf25c76a Mon Sep 17 00:00:00 2001 From: tison Date: Mon, 8 Apr 2024 15:28:55 +0800 Subject: [PATCH] refactor: drop Table trait (#3654) * refactor: drop Table trait Signed-off-by: tison * finish rename Signed-off-by: tison * Apply suggestions from code review Co-authored-by: Zhenchi * Update time_range_filter_test.rs * Update src/query/src/tests/time_range_filter_test.rs * apply comments Signed-off-by: tison --------- Signed-off-by: tison Co-authored-by: Zhenchi --- src/catalog/src/information_schema.rs | 8 +- src/query/src/tests/time_range_filter_test.rs | 12 +-- src/table/src/dist_table.rs | 7 +- src/table/src/lib.rs | 1 - src/table/src/table.rs | 87 ++++++++++++------- src/table/src/table/numbers.rs | 10 +-- src/table/src/test_util/empty_table.rs | 11 ++- src/table/src/test_util/memtable.rs | 7 +- src/table/src/thin_table.rs | 87 ------------------- 9 files changed, 82 insertions(+), 148 deletions(-) delete mode 100644 src/table/src/thin_table.rs diff --git a/src/catalog/src/information_schema.rs b/src/catalog/src/information_schema.rs index ec109b8252ee..3ece0acee7a2 100644 --- a/src/catalog/src/information_schema.rs +++ b/src/catalog/src/information_schema.rs @@ -41,8 +41,7 @@ use table::error::{SchemaConversionSnafu, TablesRecordBatchSnafu}; use table::metadata::{ FilterPushDownType, TableInfoBuilder, TableInfoRef, TableMetaBuilder, TableType, }; -use table::thin_table::{ThinTable, ThinTableAdapter}; -use table::TableRef; +use table::{Table, TableRef}; pub use table_names::*; use self::columns::InformationSchemaColumns; @@ -187,10 +186,9 @@ impl InformationSchemaProvider { self.information_table(name).map(|table| { let table_info = Self::table_info(self.catalog_name.clone(), &table); let filter_pushdown = FilterPushDownType::Inexact; - let thin_table = ThinTable::new(table_info, filter_pushdown); - let data_source = Arc::new(InformationTableDataSource::new(table)); - Arc::new(ThinTableAdapter::new(thin_table, data_source)) as _ + let table = Table::new(table_info, filter_pushdown, data_source); + Arc::new(table) }) } diff --git a/src/query/src/tests/time_range_filter_test.rs b/src/query/src/tests/time_range_filter_test.rs index c47b4e817c0a..b47ecce99f65 100644 --- a/src/query/src/tests/time_range_filter_test.rs +++ b/src/query/src/tests/time_range_filter_test.rs @@ -31,8 +31,7 @@ use store_api::storage::ScanRequest; use table::metadata::FilterPushDownType; use table::predicate::TimeRangePredicateBuilder; use table::test_util::MemTable; -use table::thin_table::{ThinTable, ThinTableAdapter}; -use table::TableRef; +use table::{Table, TableRef}; use crate::tests::exec_selection; use crate::{QueryEngineFactory, QueryEngineRef}; @@ -42,16 +41,13 @@ struct MemTableWrapper; impl MemTableWrapper { pub fn table(table: TableRef, filter: Arc>>) -> TableRef { let table_info = table.table_info(); - let thin_table_adapter = table.as_any().downcast_ref::().unwrap(); - let data_source = thin_table_adapter.data_source(); - - let thin_table = ThinTable::new(table_info, FilterPushDownType::Exact); + let data_source = table.data_source(); let data_source = Arc::new(DataSourceWrapper { inner: data_source, filter, }); - - Arc::new(ThinTableAdapter::new(thin_table, data_source)) + let table = Table::new(table_info, FilterPushDownType::Exact, data_source); + Arc::new(table) } } diff --git a/src/table/src/dist_table.rs b/src/table/src/dist_table.rs index dabcc6503c65..da3de2893333 100644 --- a/src/table/src/dist_table.rs +++ b/src/table/src/dist_table.rs @@ -21,17 +21,16 @@ use store_api::storage::ScanRequest; use crate::error::UnsupportedSnafu; use crate::metadata::{FilterPushDownType, TableInfoRef}; -use crate::thin_table::{ThinTable, ThinTableAdapter}; -use crate::TableRef; +use crate::{Table, TableRef}; #[derive(Clone)] pub struct DistTable; impl DistTable { pub fn table(table_info: TableInfoRef) -> TableRef { - let thin_table = ThinTable::new(table_info, FilterPushDownType::Inexact); let data_source = Arc::new(DummyDataSource); - Arc::new(ThinTableAdapter::new(thin_table, data_source)) + let table = Table::new(table_info, FilterPushDownType::Inexact, data_source); + Arc::new(table) } } diff --git a/src/table/src/lib.rs b/src/table/src/lib.rs index 8a5929fbd533..857d529e8add 100644 --- a/src/table/src/lib.rs +++ b/src/table/src/lib.rs @@ -23,7 +23,6 @@ pub mod stats; pub mod table; pub mod table_reference; pub mod test_util; -pub mod thin_table; pub use crate::error::{Error, Result}; pub use crate::stats::{ColumnStatistics, TableStatistics}; diff --git a/src/table/src/table.rs b/src/table/src/table.rs index d2ea65cf840b..27e410d27c8e 100644 --- a/src/table/src/table.rs +++ b/src/table/src/table.rs @@ -12,53 +12,80 @@ // See the License for the specific language governing permissions and // limitations under the License. -pub mod adapter; -mod metrics; -pub mod numbers; -pub mod scan; - -use std::any::Any; use std::sync::Arc; -use async_trait::async_trait; use common_query::logical_plan::Expr; use common_recordbatch::SendableRecordBatchStream; use datatypes::schema::SchemaRef; +use snafu::ResultExt; +use store_api::data_source::DataSourceRef; use store_api::storage::ScanRequest; -use crate::error::Result; +use crate::error::{Result, TablesRecordBatchSnafu}; use crate::metadata::{FilterPushDownType, TableId, TableInfoRef, TableType}; -/// Table abstraction. -#[async_trait] -pub trait Table: Send + Sync { - /// Returns the table as [`Any`](std::any::Any) so that it can be - /// downcast to a specific implementation. - fn as_any(&self) -> &dyn Any; +pub mod adapter; +mod metrics; +pub mod numbers; +pub mod scan; + +pub type TableRef = Arc; - /// Get a reference to the schema for this table - fn schema(&self) -> SchemaRef; +#[async_trait::async_trait] +pub trait TableIdProvider { + async fn next_table_id(&self) -> Result; +} + +pub type TableIdProviderRef = Arc; + +/// Table handle. +pub struct Table { + table_info: TableInfoRef, + filter_pushdown: FilterPushDownType, + data_source: DataSourceRef, +} + +impl Table { + pub fn new( + table_info: TableInfoRef, + filter_pushdown: FilterPushDownType, + data_source: DataSourceRef, + ) -> Self { + Self { + table_info, + filter_pushdown, + data_source, + } + } + + pub fn data_source(&self) -> DataSourceRef { + self.data_source.clone() + } + + /// Get a reference to the schema for this table. + pub fn schema(&self) -> SchemaRef { + self.table_info.meta.schema.clone() + } /// Get a reference to the table info. - fn table_info(&self) -> TableInfoRef; + pub fn table_info(&self) -> TableInfoRef { + self.table_info.clone() + } /// Get the type of this table for metadata/catalog purposes. - fn table_type(&self) -> TableType; + pub fn table_type(&self) -> TableType { + self.table_info.table_type + } - async fn scan_to_stream(&self, request: ScanRequest) -> Result; + pub async fn scan_to_stream(&self, request: ScanRequest) -> Result { + self.data_source + .get_stream(request) + .context(TablesRecordBatchSnafu) + } /// Tests whether the table provider can make use of any or all filter expressions /// to optimise data retrieval. - fn supports_filters_pushdown(&self, filters: &[&Expr]) -> Result> { - Ok(vec![FilterPushDownType::Unsupported; filters.len()]) + pub fn supports_filters_pushdown(&self, filters: &[&Expr]) -> Result> { + Ok(vec![self.filter_pushdown; filters.len()]) } } - -pub type TableRef = Arc; - -#[async_trait::async_trait] -pub trait TableIdProvider { - async fn next_table_id(&self) -> Result; -} - -pub type TableIdProviderRef = Arc; diff --git a/src/table/src/table/numbers.rs b/src/table/src/table/numbers.rs index 3ae0bf543831..b3a1cc0ab097 100644 --- a/src/table/src/table/numbers.rs +++ b/src/table/src/table/numbers.rs @@ -32,8 +32,7 @@ use store_api::storage::ScanRequest; use crate::metadata::{ FilterPushDownType, TableId, TableInfoBuilder, TableInfoRef, TableMetaBuilder, TableType, }; -use crate::thin_table::{ThinTable, ThinTableAdapter}; -use crate::TableRef; +use crate::{Table, TableRef}; const NUMBER_COLUMN: &str = "number"; @@ -49,12 +48,13 @@ impl NumbersTable { } pub fn table_with_name(table_id: TableId, name: String) -> TableRef { - let thin_table = ThinTable::new( + let data_source = Arc::new(NumbersDataSource::new(Self::schema())); + let table = Table::new( Self::table_info(table_id, name, "test_engine".to_string()), FilterPushDownType::Unsupported, + data_source, ); - let data_source = Arc::new(NumbersDataSource::new(Self::schema())); - Arc::new(ThinTableAdapter::new(thin_table, data_source)) + Arc::new(table) } pub fn schema() -> SchemaRef { diff --git a/src/table/src/test_util/empty_table.rs b/src/table/src/test_util/empty_table.rs index bf5d68c2bd7c..000614b469f7 100644 --- a/src/table/src/test_util/empty_table.rs +++ b/src/table/src/test_util/empty_table.rs @@ -21,18 +21,21 @@ use store_api::data_source::DataSource; use store_api::storage::ScanRequest; use crate::metadata::{FilterPushDownType, TableInfo}; -use crate::thin_table::{ThinTable, ThinTableAdapter}; -use crate::TableRef; +use crate::{Table, TableRef}; pub struct EmptyTable; impl EmptyTable { pub fn from_table_info(info: &TableInfo) -> TableRef { - let thin_table = ThinTable::new(Arc::new(info.clone()), FilterPushDownType::Unsupported); let data_source = Arc::new(EmptyDataSource { schema: info.meta.schema.clone(), }); - Arc::new(ThinTableAdapter::new(thin_table, data_source)) + let table = Table::new( + Arc::new(info.clone()), + FilterPushDownType::Unsupported, + data_source, + ); + Arc::new(table) } } diff --git a/src/table/src/test_util/memtable.rs b/src/table/src/test_util/memtable.rs index 837a6055b516..22562fa1a719 100644 --- a/src/table/src/test_util/memtable.rs +++ b/src/table/src/test_util/memtable.rs @@ -33,8 +33,7 @@ use crate::error::{SchemaConversionSnafu, TableProjectionSnafu, TablesRecordBatc use crate::metadata::{ FilterPushDownType, TableId, TableInfoBuilder, TableMetaBuilder, TableType, TableVersion, }; -use crate::thin_table::{ThinTable, ThinTableAdapter}; -use crate::TableRef; +use crate::{Table, TableRef}; pub struct MemTable; @@ -94,9 +93,9 @@ impl MemTable { .unwrap(), ); - let thin_table = ThinTable::new(info, FilterPushDownType::Unsupported); let data_source = Arc::new(MemtableDataSource { recordbatch }); - Arc::new(ThinTableAdapter::new(thin_table, data_source)) + let table = Table::new(info, FilterPushDownType::Unsupported, data_source); + Arc::new(table) } /// Creates a 1 column 100 rows table, with table name "numbers", column name "uint32s" and diff --git a/src/table/src/thin_table.rs b/src/table/src/thin_table.rs deleted file mode 100644 index df8c678a7e7e..000000000000 --- a/src/table/src/thin_table.rs +++ /dev/null @@ -1,87 +0,0 @@ -// Copyright 2023 Greptime Team -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use std::any::Any; - -use async_trait::async_trait; -use common_query::prelude::Expr; -use common_recordbatch::SendableRecordBatchStream; -use datatypes::schema::SchemaRef; -use snafu::ResultExt; -use store_api::data_source::DataSourceRef; -use store_api::storage::ScanRequest; - -use crate::error::{Result, TablesRecordBatchSnafu}; -use crate::metadata::{FilterPushDownType, TableInfoRef, TableType}; -use crate::Table; - -/// The `ThinTable` struct will replace the `Table` trait. -/// TODO(zhongzc): After completion, perform renaming and documentation work. -pub struct ThinTable { - table_info: TableInfoRef, - filter_pushdown: FilterPushDownType, -} - -impl ThinTable { - pub fn new(table_info: TableInfoRef, filter_pushdown: FilterPushDownType) -> Self { - Self { - table_info, - filter_pushdown, - } - } -} - -pub struct ThinTableAdapter { - table: ThinTable, - data_source: DataSourceRef, -} - -impl ThinTableAdapter { - pub fn new(table: ThinTable, data_source: DataSourceRef) -> Self { - Self { table, data_source } - } - - pub fn data_source(&self) -> DataSourceRef { - self.data_source.clone() - } -} - -#[async_trait] -impl Table for ThinTableAdapter { - fn as_any(&self) -> &dyn Any { - self - } - - fn schema(&self) -> SchemaRef { - self.table.table_info.meta.schema.clone() - } - - fn table_info(&self) -> TableInfoRef { - self.table.table_info.clone() - } - - fn table_type(&self) -> TableType { - self.table.table_info.table_type - } - - async fn scan_to_stream(&self, request: ScanRequest) -> Result { - self.data_source - .get_stream(request) - .context(TablesRecordBatchSnafu) - } - - fn supports_filters_pushdown(&self, filters: &[&Expr]) -> Result> { - Ok(vec![self.table.filter_pushdown; filters.len()]) - } -}