From a95f8767a80d1c3fc8e3d187f89a5a6aa5fd5440 Mon Sep 17 00:00:00 2001 From: Ruihang Xia Date: Mon, 26 Jun 2023 15:08:59 +0800 Subject: [PATCH] refactor: merge catalog provider & schema provider into catalog manager (#1803) * move to expr_factory Signed-off-by: Ruihang Xia * move configs into service_config Signed-off-by: Ruihang Xia * move GrpcQueryHandler into distributed.rs Signed-off-by: Ruihang Xia * fix compile and test in catalog sub-crate Signed-off-by: Ruihang Xia * clean up Signed-off-by: Ruihang Xia * fix table-procedure compile and test Signed-off-by: Ruihang Xia * fix query compile and tests Signed-off-by: Ruihang Xia * fix datanode compile and tests Signed-off-by: Ruihang Xia * fix catalog/query/script/servers compile Signed-off-by: Ruihang Xia * fix frontend compile Signed-off-by: Ruihang Xia * fix nextest except information_schema Signed-off-by: Ruihang Xia * support information_schema Signed-off-by: Ruihang Xia * fix sqlness test Signed-off-by: Ruihang Xia * fix merge errors Signed-off-by: Ruihang Xia * remove other structs Signed-off-by: Ruihang Xia * clean up Signed-off-by: Ruihang Xia * fix format Signed-off-by: Ruihang Xia * change deregister_table's return type to empty tuple Signed-off-by: Ruihang Xia --------- Signed-off-by: Ruihang Xia --- src/catalog/src/error.rs | 11 +- src/catalog/src/helper.rs | 2 + src/catalog/src/information_schema.rs | 36 +- src/catalog/src/information_schema/columns.rs | 50 +- src/catalog/src/information_schema/tables.rs | 48 +- src/catalog/src/lib.rs | 86 +- src/catalog/src/local.rs | 4 +- src/catalog/src/local/manager.rs | 349 +++--- src/catalog/src/local/memory.rs | 637 +++++----- src/catalog/src/remote.rs | 2 +- src/catalog/src/remote/manager.rs | 1064 +++++++---------- src/catalog/src/schema.rs | 69 -- src/catalog/src/table_source.rs | 42 +- src/catalog/src/tables.rs | 59 +- src/catalog/tests/remote_catalog_tests.rs | 109 +- src/common/catalog/src/consts.rs | 2 + .../src/heartbeat/handler/close_region.rs | 14 +- src/datanode/src/instance.rs | 10 +- src/datanode/src/instance/grpc.rs | 21 +- src/datanode/src/sql/create.rs | 3 +- src/datanode/src/sql/flush_table.rs | 48 +- src/datanode/src/tests/test_util.rs | 19 +- src/frontend/src/catalog.rs | 218 ++-- src/frontend/src/instance.rs | 3 +- src/frontend/src/instance/distributed.rs | 16 +- src/frontend/src/statement.rs | 7 +- src/frontend/src/statement/backup.rs | 23 +- src/frontend/src/statement/show.rs | 8 +- src/query/src/datafusion.rs | 60 +- src/query/src/query_engine.rs | 2 +- src/query/src/sql.rs | 23 +- src/query/src/tests.rs | 13 +- src/query/src/tests/function.rs | 27 +- src/query/src/tests/my_sum_udaf_example.rs | 29 +- src/query/src/tests/percentile_test.rs | 26 +- src/query/src/tests/query_engine_test.rs | 40 +- src/query/src/tests/time_range_filter_test.rs | 30 +- src/script/Cargo.toml | 1 + src/script/benches/py_benchmark.rs | 23 +- src/script/src/manager.rs | 1 + src/script/src/python/engine.rs | 23 +- src/servers/tests/mod.rs | 21 +- src/table-procedure/src/alter.rs | 47 +- src/table-procedure/src/create.rs | 71 +- src/table-procedure/src/drop.rs | 24 +- src/table/src/table/numbers.rs | 11 +- tests-integration/src/cluster.rs | 2 +- tests-integration/src/test_util.rs | 23 +- tests-integration/src/tests.rs | 21 +- tests-integration/src/tests/instance_test.rs | 4 +- 50 files changed, 1397 insertions(+), 2085 deletions(-) delete mode 100644 src/catalog/src/schema.rs diff --git a/src/catalog/src/error.rs b/src/catalog/src/error.rs index 64e858b754bf..34143793ea5c 100644 --- a/src/catalog/src/error.rs +++ b/src/catalog/src/error.rs @@ -192,11 +192,18 @@ pub enum Error { source: BoxedError, }, + #[snafu(display( + "Failed to upgrade weak catalog manager reference. location: {}", + location + ))] + UpgradeWeakCatalogManagerRef { location: Location }, + #[snafu(display("Failed to execute system catalog table scan, source: {}", source))] SystemCatalogTableScanExec { location: Location, source: common_query::error::Error, }, + #[snafu(display("Cannot parse catalog value, source: {}", source))] InvalidCatalogValue { location: Location, @@ -256,7 +263,9 @@ impl ErrorExt for Error { | Error::EmptyValue { .. } | Error::ValueDeserialize { .. } => StatusCode::StorageUnavailable, - Error::Generic { .. } | Error::SystemCatalogTypeMismatch { .. } => StatusCode::Internal, + Error::Generic { .. } + | Error::SystemCatalogTypeMismatch { .. } + | Error::UpgradeWeakCatalogManagerRef { .. } => StatusCode::Internal, Error::ReadSystemCatalog { source, .. } | Error::CreateRecordBatch { source, .. } => { source.status_code() diff --git a/src/catalog/src/helper.rs b/src/catalog/src/helper.rs index c95fc1b4d01c..dfad2b76d63e 100644 --- a/src/catalog/src/helper.rs +++ b/src/catalog/src/helper.rs @@ -67,6 +67,7 @@ pub fn build_schema_prefix(catalog_name: impl AsRef) -> String { format!("{SCHEMA_KEY_PREFIX}-{}-", catalog_name.as_ref()) } +/// Global table info has only one key across all datanodes so it does not have `node_id` field. pub fn build_table_global_prefix( catalog_name: impl AsRef, schema_name: impl AsRef, @@ -78,6 +79,7 @@ pub fn build_table_global_prefix( ) } +/// Regional table info varies between datanode, so it contains a `node_id` field. pub fn build_table_regional_prefix( catalog_name: impl AsRef, schema_name: impl AsRef, diff --git a/src/catalog/src/information_schema.rs b/src/catalog/src/information_schema.rs index 4952d52d8f14..80a3577cafd4 100644 --- a/src/catalog/src/information_schema.rs +++ b/src/catalog/src/information_schema.rs @@ -16,7 +16,7 @@ mod columns; mod tables; use std::any::Any; -use std::sync::Arc; +use std::sync::{Arc, Weak}; use async_trait::async_trait; use common_error::prelude::BoxedError; @@ -33,46 +33,35 @@ use table::{Result as TableResult, Table, TableRef}; use self::columns::InformationSchemaColumns; use crate::error::Result; use crate::information_schema::tables::InformationSchemaTables; -use crate::{CatalogProviderRef, SchemaProvider}; +use crate::CatalogManager; const TABLES: &str = "tables"; const COLUMNS: &str = "columns"; -pub(crate) struct InformationSchemaProvider { +pub struct InformationSchemaProvider { catalog_name: String, - catalog_provider: CatalogProviderRef, - tables: Vec, + catalog_manager: Weak, } impl InformationSchemaProvider { - pub(crate) fn new(catalog_name: String, catalog_provider: CatalogProviderRef) -> Self { + pub fn new(catalog_name: String, catalog_manager: Weak) -> Self { Self { catalog_name, - catalog_provider, - tables: vec![TABLES.to_string(), COLUMNS.to_string()], + catalog_manager, } } } -#[async_trait] -impl SchemaProvider for InformationSchemaProvider { - fn as_any(&self) -> &dyn Any { - self - } - - async fn table_names(&self) -> Result> { - Ok(self.tables.clone()) - } - - async fn table(&self, name: &str) -> Result> { +impl InformationSchemaProvider { + pub fn table(&self, name: &str) -> Result> { let stream_builder = match name.to_ascii_lowercase().as_ref() { TABLES => Arc::new(InformationSchemaTables::new( self.catalog_name.clone(), - self.catalog_provider.clone(), + self.catalog_manager.clone(), )) as _, COLUMNS => Arc::new(InformationSchemaColumns::new( self.catalog_name.clone(), - self.catalog_provider.clone(), + self.catalog_manager.clone(), )) as _, _ => { return Ok(None); @@ -81,11 +70,6 @@ impl SchemaProvider for InformationSchemaProvider { Ok(Some(Arc::new(InformationTable::new(stream_builder)))) } - - async fn table_exist(&self, name: &str) -> Result { - let normalized_name = name.to_ascii_lowercase(); - Ok(self.tables.contains(&normalized_name)) - } } // TODO(ruihang): make it a more generic trait: diff --git a/src/catalog/src/information_schema/columns.rs b/src/catalog/src/information_schema/columns.rs index 2331aa65d3a0..15d7dfa4e36d 100644 --- a/src/catalog/src/information_schema/columns.rs +++ b/src/catalog/src/information_schema/columns.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::sync::Arc; +use std::sync::{Arc, Weak}; use arrow_schema::SchemaRef as ArrowSchemaRef; use common_catalog::consts::{ @@ -29,16 +29,18 @@ use datatypes::prelude::{ConcreteDataType, DataType}; use datatypes::scalars::ScalarVectorBuilder; use datatypes::schema::{ColumnSchema, Schema, SchemaRef}; use datatypes::vectors::{StringVectorBuilder, VectorRef}; -use snafu::ResultExt; +use snafu::{OptionExt, ResultExt}; use super::InformationStreamBuilder; -use crate::error::{CreateRecordBatchSnafu, InternalSnafu, Result}; -use crate::CatalogProviderRef; +use crate::error::{ + CreateRecordBatchSnafu, InternalSnafu, Result, UpgradeWeakCatalogManagerRefSnafu, +}; +use crate::CatalogManager; pub(super) struct InformationSchemaColumns { schema: SchemaRef, catalog_name: String, - catalog_provider: CatalogProviderRef, + catalog_manager: Weak, } const TABLE_CATALOG: &str = "table_catalog"; @@ -49,7 +51,7 @@ const DATA_TYPE: &str = "data_type"; const SEMANTIC_TYPE: &str = "semantic_type"; impl InformationSchemaColumns { - pub(super) fn new(catalog_name: String, catalog_provider: CatalogProviderRef) -> Self { + pub(super) fn new(catalog_name: String, catalog_manager: Weak) -> Self { let schema = Arc::new(Schema::new(vec![ ColumnSchema::new(TABLE_CATALOG, ConcreteDataType::string_datatype(), false), ColumnSchema::new(TABLE_SCHEMA, ConcreteDataType::string_datatype(), false), @@ -61,7 +63,7 @@ impl InformationSchemaColumns { Self { schema, catalog_name, - catalog_provider, + catalog_manager, } } @@ -69,7 +71,7 @@ impl InformationSchemaColumns { InformationSchemaColumnsBuilder::new( self.schema.clone(), self.catalog_name.clone(), - self.catalog_provider.clone(), + self.catalog_manager.clone(), ) } } @@ -103,7 +105,7 @@ impl InformationStreamBuilder for InformationSchemaColumns { struct InformationSchemaColumnsBuilder { schema: SchemaRef, catalog_name: String, - catalog_provider: CatalogProviderRef, + catalog_manager: Weak, catalog_names: StringVectorBuilder, schema_names: StringVectorBuilder, @@ -114,11 +116,15 @@ struct InformationSchemaColumnsBuilder { } impl InformationSchemaColumnsBuilder { - fn new(schema: SchemaRef, catalog_name: String, catalog_provider: CatalogProviderRef) -> Self { + fn new( + schema: SchemaRef, + catalog_name: String, + catalog_manager: Weak, + ) -> Self { Self { schema, catalog_name, - catalog_provider, + catalog_manager, catalog_names: StringVectorBuilder::with_capacity(42), schema_names: StringVectorBuilder::with_capacity(42), table_names: StringVectorBuilder::with_capacity(42), @@ -131,11 +137,23 @@ impl InformationSchemaColumnsBuilder { /// Construct the `information_schema.tables` virtual table async fn make_tables(&mut self) -> Result { let catalog_name = self.catalog_name.clone(); - - for schema_name in self.catalog_provider.schema_names().await? { - let Some(schema) = self.catalog_provider.schema(&schema_name).await? else { continue }; - for table_name in schema.table_names().await? { - let Some(table) = schema.table(&table_name).await? else { continue }; + let catalog_manager = self + .catalog_manager + .upgrade() + .context(UpgradeWeakCatalogManagerRefSnafu)?; + + for schema_name in catalog_manager.schema_names(&catalog_name).await? { + if !catalog_manager + .schema_exist(&catalog_name, &schema_name) + .await? + { + continue; + } + for table_name in catalog_manager + .table_names(&catalog_name, &schema_name) + .await? + { + let Some(table) = catalog_manager.table(&catalog_name, &schema_name, &table_name).await? else { continue }; let keys = &table.table_info().meta.primary_key_indices; let schema = table.schema(); for (idx, column) in schema.column_schemas().iter().enumerate() { diff --git a/src/catalog/src/information_schema/tables.rs b/src/catalog/src/information_schema/tables.rs index 92e0eaeb9f53..ff24ac1c2a76 100644 --- a/src/catalog/src/information_schema/tables.rs +++ b/src/catalog/src/information_schema/tables.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::sync::Arc; +use std::sync::{Arc, Weak}; use arrow_schema::SchemaRef as ArrowSchemaRef; use common_catalog::consts::INFORMATION_SCHEMA_NAME; @@ -26,21 +26,23 @@ use datafusion::physical_plan::SendableRecordBatchStream as DfSendableRecordBatc use datatypes::prelude::{ConcreteDataType, ScalarVectorBuilder, VectorRef}; use datatypes::schema::{ColumnSchema, Schema, SchemaRef}; use datatypes::vectors::{StringVectorBuilder, UInt32VectorBuilder}; -use snafu::ResultExt; +use snafu::{OptionExt, ResultExt}; use table::metadata::TableType; -use crate::error::{CreateRecordBatchSnafu, InternalSnafu, Result}; +use crate::error::{ + CreateRecordBatchSnafu, InternalSnafu, Result, UpgradeWeakCatalogManagerRefSnafu, +}; use crate::information_schema::InformationStreamBuilder; -use crate::CatalogProviderRef; +use crate::CatalogManager; pub(super) struct InformationSchemaTables { schema: SchemaRef, catalog_name: String, - catalog_provider: CatalogProviderRef, + catalog_manager: Weak, } impl InformationSchemaTables { - pub(super) fn new(catalog_name: String, catalog_provider: CatalogProviderRef) -> Self { + pub(super) fn new(catalog_name: String, catalog_manager: Weak) -> Self { let schema = Arc::new(Schema::new(vec![ ColumnSchema::new("table_catalog", ConcreteDataType::string_datatype(), false), ColumnSchema::new("table_schema", ConcreteDataType::string_datatype(), false), @@ -52,7 +54,7 @@ impl InformationSchemaTables { Self { schema, catalog_name, - catalog_provider, + catalog_manager, } } @@ -60,7 +62,7 @@ impl InformationSchemaTables { InformationSchemaTablesBuilder::new( self.schema.clone(), self.catalog_name.clone(), - self.catalog_provider.clone(), + self.catalog_manager.clone(), ) } } @@ -97,7 +99,7 @@ impl InformationStreamBuilder for InformationSchemaTables { struct InformationSchemaTablesBuilder { schema: SchemaRef, catalog_name: String, - catalog_provider: CatalogProviderRef, + catalog_manager: Weak, catalog_names: StringVectorBuilder, schema_names: StringVectorBuilder, @@ -108,11 +110,15 @@ struct InformationSchemaTablesBuilder { } impl InformationSchemaTablesBuilder { - fn new(schema: SchemaRef, catalog_name: String, catalog_provider: CatalogProviderRef) -> Self { + fn new( + schema: SchemaRef, + catalog_name: String, + catalog_manager: Weak, + ) -> Self { Self { schema, catalog_name, - catalog_provider, + catalog_manager, catalog_names: StringVectorBuilder::with_capacity(42), schema_names: StringVectorBuilder::with_capacity(42), table_names: StringVectorBuilder::with_capacity(42), @@ -125,15 +131,27 @@ impl InformationSchemaTablesBuilder { /// Construct the `information_schema.tables` virtual table async fn make_tables(&mut self) -> Result { let catalog_name = self.catalog_name.clone(); + let catalog_manager = self + .catalog_manager + .upgrade() + .context(UpgradeWeakCatalogManagerRefSnafu)?; - for schema_name in self.catalog_provider.schema_names().await? { + for schema_name in catalog_manager.schema_names(&catalog_name).await? { if schema_name == INFORMATION_SCHEMA_NAME { continue; } + if !catalog_manager + .schema_exist(&catalog_name, &schema_name) + .await? + { + continue; + } - let Some(schema) = self.catalog_provider.schema(&schema_name).await? else { continue }; - for table_name in schema.table_names().await? { - let Some(table) = schema.table(&table_name).await? else { continue }; + for table_name in catalog_manager + .table_names(&catalog_name, &schema_name) + .await? + { + let Some(table) = catalog_manager.table(&catalog_name, &schema_name, &table_name).await? else { continue }; let table_info = table.table_info(); self.add_table( &catalog_name, diff --git a/src/catalog/src/lib.rs b/src/catalog/src/lib.rs index 09876a2dc005..54f69b5d41c9 100644 --- a/src/catalog/src/lib.rs +++ b/src/catalog/src/lib.rs @@ -14,6 +14,7 @@ #![feature(trait_upcasting)] #![feature(assert_matches)] +#![feature(try_blocks)] use std::any::Any; use std::collections::HashMap; @@ -29,64 +30,45 @@ use table::requests::CreateTableRequest; use table::TableRef; use crate::error::{CreateTableSnafu, Result}; -pub use crate::schema::{SchemaProvider, SchemaProviderRef}; pub mod error; pub mod helper; -pub(crate) mod information_schema; +pub mod information_schema; pub mod local; mod metrics; pub mod remote; -pub mod schema; pub mod system; pub mod table_source; pub mod tables; -/// Represents a catalog, comprising a number of named schemas. #[async_trait::async_trait] -pub trait CatalogProvider: Sync + Send { - /// Returns the catalog provider as [`Any`](std::any::Any) - /// so that it can be downcast to a specific implementation. +pub trait CatalogManager: Send + Sync { fn as_any(&self) -> &dyn Any; - /// Retrieves the list of available schema names in this catalog. - async fn schema_names(&self) -> Result>; - - /// Registers schema to this catalog. - async fn register_schema( - &self, - name: String, - schema: SchemaProviderRef, - ) -> Result>; - - /// Retrieves a specific schema from the catalog by name, provided it exists. - async fn schema(&self, name: &str) -> Result>; -} - -pub type CatalogProviderRef = Arc; - -#[async_trait::async_trait] -pub trait CatalogManager: Send + Sync { /// Starts a catalog manager. async fn start(&self) -> Result<()>; - async fn register_catalog( - &self, - name: String, - catalog: CatalogProviderRef, - ) -> Result>; + /// Registers a catalog to catalog manager, returns whether the catalog exist before. + async fn register_catalog(&self, name: String) -> Result; + + /// Register a schema with catalog name and schema name. Retuens whether the + /// schema registered. + /// + /// # Errors + /// + /// This method will/should fail if catalog not exist + async fn register_schema(&self, request: RegisterSchemaRequest) -> Result; /// Registers a table within given catalog/schema to catalog manager, /// returns whether the table registered. + /// + /// # Errors + /// + /// This method will/should fail if catalog or schema not exist async fn register_table(&self, request: RegisterTableRequest) -> Result; - /// Deregisters a table within given catalog/schema to catalog manager, - /// returns whether the table deregistered. - async fn deregister_table(&self, request: DeregisterTableRequest) -> Result; - - /// Register a schema with catalog name and schema name. Retuens whether the - /// schema registered. - async fn register_schema(&self, request: RegisterSchemaRequest) -> Result; + /// Deregisters a table within given catalog/schema to catalog manager + async fn deregister_table(&self, request: DeregisterTableRequest) -> Result<()>; /// Rename a table to [RenameTableRequest::new_table_name], returns whether the table is renamed. async fn rename_table(&self, request: RenameTableRequest) -> Result; @@ -97,9 +79,15 @@ pub trait CatalogManager: Send + Sync { async fn catalog_names(&self) -> Result>; - async fn catalog(&self, catalog: &str) -> Result>; + async fn schema_names(&self, catalog: &str) -> Result>; + + async fn table_names(&self, catalog: &str, schema: &str) -> Result>; - async fn schema(&self, catalog: &str, schema: &str) -> Result>; + async fn catalog_exist(&self, catalog: &str) -> Result; + + async fn schema_exist(&self, catalog: &str, schema: &str) -> Result; + + async fn table_exist(&self, catalog: &str, schema: &str, table: &str) -> Result; /// Returns the table by catalog, schema and table name. async fn table( @@ -108,8 +96,6 @@ pub trait CatalogManager: Send + Sync { schema: &str, table_name: &str, ) -> Result>; - - fn as_any(&self) -> &dyn Any; } pub type CatalogManagerRef = Arc; @@ -169,14 +155,6 @@ pub struct RegisterSchemaRequest { pub schema: String, } -pub trait CatalogProviderFactory { - fn create(&self, catalog_name: String) -> CatalogProviderRef; -} - -pub trait SchemaProviderFactory { - fn create(&self, catalog_name: String, schema_name: String) -> SchemaProviderRef; -} - pub(crate) async fn handle_system_table_request<'a, M: CatalogManager>( manager: &'a M, engine: TableEngineRef, @@ -233,15 +211,11 @@ pub async fn datanode_stat(catalog_manager: &CatalogManagerRef) -> (u64, Vec Result<()> { - let system_schema = Arc::new(MemorySchemaProvider::new()); - system_schema.register_table_sync( - SYSTEM_CATALOG_TABLE_NAME.to_string(), - self.system.information_schema.system.clone(), - )?; - let system_catalog = Arc::new(MemoryCatalogProvider::new()); - system_catalog.register_schema_sync(INFORMATION_SCHEMA_NAME.to_string(), system_schema)?; + // register SystemCatalogTable self.catalogs - .register_catalog_sync(SYSTEM_CATALOG_NAME.to_string(), system_catalog)?; + .register_catalog_sync(SYSTEM_CATALOG_NAME.to_string())?; + self.catalogs.register_schema_sync(RegisterSchemaRequest { + catalog: SYSTEM_CATALOG_NAME.to_string(), + schema: INFORMATION_SCHEMA_NAME.to_string(), + })?; + let register_table_req = RegisterTableRequest { + catalog: SYSTEM_CATALOG_NAME.to_string(), + schema: INFORMATION_SCHEMA_NAME.to_string(), + table_name: SYSTEM_CATALOG_TABLE_NAME.to_string(), + table_id: SYSTEM_CATALOG_TABLE_ID, + table: self.system.information_schema.system.clone(), + }; + self.catalogs.register_table(register_table_req).await?; - let default_catalog = Arc::new(MemoryCatalogProvider::new()); - let default_schema = Arc::new(MemorySchemaProvider::new()); + // register default catalog and default schema + self.catalogs + .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string())?; + self.catalogs.register_schema_sync(RegisterSchemaRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + })?; // Add numbers table for test - let table = Arc::new(NumbersTable::default()); - default_schema.register_table_sync("numbers".to_string(), table)?; + let numbers_table = Arc::new(NumbersTable::default()); + let register_number_table_req = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: NUMBERS_TABLE_NAME.to_string(), + table_id: NUMBERS_TABLE_ID, + table: numbers_table, + }; - default_catalog.register_schema_sync(DEFAULT_SCHEMA_NAME.to_string(), default_schema)?; self.catalogs - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), default_catalog)?; + .register_table(register_number_table_req) + .await?; + Ok(()) } @@ -207,24 +226,16 @@ impl LocalCatalogManager { for entry in entries { match entry { Entry::Catalog(c) => { - self.catalogs.register_catalog_if_absent( - c.catalog_name.clone(), - Arc::new(MemoryCatalogProvider::new()), - ); + self.catalogs + .register_catalog_if_absent(c.catalog_name.clone()); info!("Register catalog: {}", c.catalog_name); } Entry::Schema(s) => { - self.catalogs - .catalog(&s.catalog_name) - .await? - .context(CatalogNotFoundSnafu { - catalog_name: &s.catalog_name, - })? - .register_schema( - s.schema_name.clone(), - Arc::new(MemorySchemaProvider::new()), - ) - .await?; + let req = RegisterSchemaRequest { + catalog: s.catalog_name.clone(), + schema: s.schema_name.clone(), + }; + self.catalogs.register_schema_sync(req)?; info!("Registered schema: {:?}", s); } Entry::Table(t) => { @@ -245,23 +256,11 @@ impl LocalCatalogManager { } async fn open_and_register_table(&self, t: &TableEntry) -> Result<()> { - let catalog = - self.catalogs - .catalog(&t.catalog_name) - .await? - .context(CatalogNotFoundSnafu { - catalog_name: &t.catalog_name, - })?; - let schema = catalog - .schema(&t.schema_name) - .await? - .context(SchemaNotFoundSnafu { - catalog: &t.catalog_name, - schema: &t.schema_name, - })?; + self.check_catalog_schema_exist(&t.catalog_name, &t.schema_name) + .await?; let context = EngineContext {}; - let request = OpenTableRequest { + let open_request = OpenTableRequest { catalog_name: t.catalog_name.clone(), schema_name: t.schema_name.clone(), table_name: t.table_name.clone(), @@ -275,8 +274,8 @@ impl LocalCatalogManager { engine_name: &t.engine, })?; - let option = engine - .open_table(&context, request) + let table_ref = engine + .open_table(&context, open_request) .await .with_context(|_| OpenTableSnafu { table_info: format!( @@ -291,7 +290,48 @@ impl LocalCatalogManager { ), })?; - schema.register_table(t.table_name.clone(), option).await?; + let register_request = RegisterTableRequest { + catalog: t.catalog_name.clone(), + schema: t.schema_name.clone(), + table_name: t.table_name.clone(), + table_id: t.table_id, + table: table_ref, + }; + self.catalogs.register_table(register_request).await?; + + Ok(()) + } + + async fn check_state(&self) -> Result<()> { + let started = self.init_lock.lock().await; + ensure!( + *started, + IllegalManagerStateSnafu { + msg: "Catalog manager not started", + } + ); + Ok(()) + } + + async fn check_catalog_schema_exist( + &self, + catalog_name: &str, + schema_name: &str, + ) -> Result<()> { + if !self.catalogs.catalog_exist(catalog_name).await? { + return CatalogNotFoundSnafu { catalog_name }.fail()?; + } + if !self + .catalogs + .schema_exist(catalog_name, schema_name) + .await? + { + return SchemaNotFoundSnafu { + catalog: catalog_name, + schema: schema_name, + } + .fail()?; + } Ok(()) } } @@ -312,34 +352,21 @@ impl CatalogManager for LocalCatalogManager { } async fn register_table(&self, request: RegisterTableRequest) -> Result { - let started = self.init_lock.lock().await; + self.check_state().await?; - ensure!( - *started, - IllegalManagerStateSnafu { - msg: "Catalog manager not started", - } - ); + let catalog_name = request.catalog.clone(); + let schema_name = request.schema.clone(); - let catalog_name = &request.catalog; - let schema_name = &request.schema; - - let catalog = self - .catalogs - .catalog(catalog_name) - .await? - .context(CatalogNotFoundSnafu { catalog_name })?; - let schema = catalog - .schema(schema_name) - .await? - .with_context(|| SchemaNotFoundSnafu { - catalog: catalog_name, - schema: schema_name, - })?; + self.check_catalog_schema_exist(&catalog_name, &schema_name) + .await?; { let _lock = self.register_lock.lock().await; - if let Some(existing) = schema.table(&request.table_name).await? { + if let Some(existing) = self + .catalogs + .table(&request.catalog, &request.schema, &request.table_name) + .await? + { if existing.table_info().ident.table_id != request.table_id { error!( "Unexpected table register request: {:?}, existing: {:?}", @@ -348,8 +375,8 @@ impl CatalogManager for LocalCatalogManager { ); return TableExistsSnafu { table: format_full_table_name( - catalog_name, - schema_name, + &catalog_name, + &schema_name, &request.table_name, ), } @@ -358,24 +385,24 @@ impl CatalogManager for LocalCatalogManager { // Try to register table with same table id, just ignore. Ok(false) } else { - let engine = request.table.table_info().meta.engine.to_string(); // table does not exist + let engine = request.table.table_info().meta.engine.to_string(); + let table_name = request.table_name.clone(); + let table_id = request.table_id; + self.catalogs.register_table(request).await?; self.system .register_table( catalog_name.clone(), schema_name.clone(), - request.table_name.clone(), - request.table_id, + table_name, + table_id, engine, ) .await?; - schema - .register_table(request.table_name, request.table) - .await?; increment_gauge!( crate::metrics::METRIC_CATALOG_MANAGER_TABLE_COUNT, 1.0, - &[crate::metrics::db_label(catalog_name, schema_name)], + &[crate::metrics::db_label(&catalog_name, &schema_name)], ); Ok(true) } @@ -383,41 +410,27 @@ impl CatalogManager for LocalCatalogManager { } async fn rename_table(&self, request: RenameTableRequest) -> Result { - let started = self.init_lock.lock().await; - - ensure!( - *started, - IllegalManagerStateSnafu { - msg: "Catalog manager not started", - } - ); + self.check_state().await?; let catalog_name = &request.catalog; let schema_name = &request.schema; - let catalog = self - .catalogs - .catalog(catalog_name) - .await? - .context(CatalogNotFoundSnafu { catalog_name })?; - - let schema = catalog - .schema(schema_name) - .await? - .with_context(|| SchemaNotFoundSnafu { - catalog: catalog_name, - schema: schema_name, - })?; - - let _lock = self.register_lock.lock().await; + self.check_catalog_schema_exist(catalog_name, schema_name) + .await?; ensure!( - !schema.table_exist(&request.new_table_name).await?, + self.catalogs + .table(catalog_name, schema_name, &request.new_table_name) + .await? + .is_none(), TableExistsSnafu { table: &request.new_table_name } ); - let old_table = schema - .table(&request.table_name) + + let _lock = self.register_lock.lock().await; + let old_table = self + .catalogs + .table(catalog_name, schema_name, &request.table_name) .await? .context(TableNotExistSnafu { table: &request.table_name, @@ -435,18 +448,11 @@ impl CatalogManager for LocalCatalogManager { ) .await?; - let renamed = schema - .rename_table(&request.table_name, request.new_table_name.clone()) - .await - .is_ok(); - Ok(renamed) + self.catalogs.rename_table(request).await } - async fn deregister_table(&self, request: DeregisterTableRequest) -> Result { - { - let started = *self.init_lock.lock().await; - ensure!(started, IllegalManagerStateSnafu { msg: "not started" }); - } + async fn deregister_table(&self, request: DeregisterTableRequest) -> Result<()> { + self.check_state().await?; { let _ = self.register_lock.lock().await; @@ -473,52 +479,39 @@ impl CatalogManager for LocalCatalogManager { } async fn register_schema(&self, request: RegisterSchemaRequest) -> Result { - let started = self.init_lock.lock().await; - ensure!( - *started, - IllegalManagerStateSnafu { - msg: "Catalog manager not started", - } - ); + self.check_state().await?; + let catalog_name = &request.catalog; let schema_name = &request.schema; - let catalog = self - .catalogs - .catalog(catalog_name) - .await? - .context(CatalogNotFoundSnafu { catalog_name })?; + if !self.catalogs.catalog_exist(catalog_name).await? { + return CatalogNotFoundSnafu { catalog_name }.fail()?; + } { let _lock = self.register_lock.lock().await; ensure!( - catalog.schema(schema_name).await?.is_none(), + !self + .catalogs + .schema_exist(catalog_name, schema_name) + .await?, SchemaExistsSnafu { schema: schema_name, } ); self.system - .register_schema(request.catalog, schema_name.clone()) - .await?; - catalog - .register_schema(request.schema, Arc::new(MemorySchemaProvider::new())) + .register_schema(request.catalog.clone(), schema_name.clone()) .await?; - - Ok(true) + self.catalogs.register_schema_sync(request) } } async fn register_system_table(&self, request: RegisterSystemTableRequest) -> Result<()> { + self.check_state().await?; + let catalog_name = request.create_table_request.catalog_name.clone(); let schema_name = request.create_table_request.schema_name.clone(); - ensure!( - !*self.init_lock.lock().await, - IllegalManagerStateSnafu { - msg: "Catalog manager already started", - } - ); - let mut sys_table_requests = self.system_table_requests.lock().await; sys_table_requests.push(request); increment_gauge!( @@ -529,15 +522,8 @@ impl CatalogManager for LocalCatalogManager { Ok(()) } - async fn schema(&self, catalog: &str, schema: &str) -> Result> { - self.catalogs - .catalog(catalog) - .await? - .context(CatalogNotFoundSnafu { - catalog_name: catalog, - })? - .schema(schema) - .await + async fn schema_exist(&self, catalog: &str, schema: &str) -> Result { + self.catalogs.schema_exist(catalog, schema).await } async fn table( @@ -546,39 +532,44 @@ impl CatalogManager for LocalCatalogManager { schema_name: &str, table_name: &str, ) -> Result> { - let catalog = self - .catalogs - .catalog(catalog_name) - .await? - .context(CatalogNotFoundSnafu { catalog_name })?; - let schema = catalog - .schema(schema_name) - .await? - .with_context(|| SchemaNotFoundSnafu { - catalog: catalog_name, - schema: schema_name, - })?; - schema.table(table_name).await + if schema_name == INFORMATION_SCHEMA_NAME { + let manager: CatalogManagerRef = self.catalogs.clone() as _; + let provider = + InformationSchemaProvider::new(catalog_name.to_string(), Arc::downgrade(&manager)); + return provider.table(table_name); + } + + self.catalogs + .table(catalog_name, schema_name, table_name) + .await } - async fn catalog(&self, catalog: &str) -> Result> { + async fn catalog_exist(&self, catalog: &str) -> Result { if catalog.eq_ignore_ascii_case(SYSTEM_CATALOG_NAME) { - Ok(Some(self.system.clone())) + Ok(true) } else { - self.catalogs.catalog(catalog).await + self.catalogs.catalog_exist(catalog).await } } + async fn table_exist(&self, catalog: &str, schema: &str, table: &str) -> Result { + self.catalogs.table_exist(catalog, schema, table).await + } + async fn catalog_names(&self) -> Result> { self.catalogs.catalog_names().await } - async fn register_catalog( - &self, - name: String, - catalog: CatalogProviderRef, - ) -> Result> { - self.catalogs.register_catalog(name, catalog).await + async fn schema_names(&self, catalog_name: &str) -> Result> { + self.catalogs.schema_names(catalog_name).await + } + + async fn table_names(&self, catalog_name: &str, schema_name: &str) -> Result> { + self.catalogs.table_names(catalog_name, schema_name).await + } + + async fn register_catalog(&self, name: String) -> Result { + self.catalogs.register_catalog(name).await } fn as_any(&self) -> &dyn Any { diff --git a/src/catalog/src/local/memory.rs b/src/catalog/src/local/memory.rs index a5c1b84591c2..799d93a71f2b 100644 --- a/src/catalog/src/local/memory.rs +++ b/src/catalog/src/local/memory.rs @@ -18,29 +18,27 @@ use std::collections::HashMap; use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::{Arc, RwLock}; -use async_trait::async_trait; -use common_catalog::consts::MIN_USER_TABLE_ID; -use common_telemetry::error; +use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, MIN_USER_TABLE_ID}; use metrics::{decrement_gauge, increment_gauge}; -use snafu::{ensure, OptionExt}; +use snafu::OptionExt; use table::metadata::TableId; use table::table::TableIdProvider; use table::TableRef; use crate::error::{ - self, CatalogNotFoundSnafu, Result, SchemaNotFoundSnafu, TableExistsSnafu, TableNotFoundSnafu, + CatalogNotFoundSnafu, Result, SchemaNotFoundSnafu, TableExistsSnafu, TableNotFoundSnafu, }; -use crate::schema::SchemaProvider; use crate::{ - CatalogManager, CatalogProvider, CatalogProviderRef, DeregisterTableRequest, - RegisterSchemaRequest, RegisterSystemTableRequest, RegisterTableRequest, RenameTableRequest, - SchemaProviderRef, + CatalogManager, DeregisterTableRequest, RegisterSchemaRequest, RegisterSystemTableRequest, + RegisterTableRequest, RenameTableRequest, }; +type SchemaEntries = HashMap>; + /// Simple in-memory list of catalogs pub struct MemoryCatalogManager { /// Collection of catalogs containing schemas and ultimately Tables - pub catalogs: RwLock>, + pub catalogs: RwLock>, pub table_id: AtomicU32, } @@ -50,13 +48,15 @@ impl Default for MemoryCatalogManager { table_id: AtomicU32::new(MIN_USER_TABLE_ID), catalogs: Default::default(), }; - let default_catalog = Arc::new(MemoryCatalogProvider::new()); + + let mut catalog = HashMap::with_capacity(1); + catalog.insert(DEFAULT_SCHEMA_NAME.to_string(), HashMap::new()); manager - .register_catalog_sync("greptime".to_string(), default_catalog.clone()) - .unwrap(); - default_catalog - .register_schema_sync("public".to_string(), Arc::new(MemorySchemaProvider::new())) - .unwrap(); + .catalogs + .write() + .unwrap() + .insert(DEFAULT_CATALOG_NAME.to_string(), catalog); + manager } } @@ -76,80 +76,75 @@ impl CatalogManager for MemoryCatalogManager { } async fn register_table(&self, request: RegisterTableRequest) -> Result { - let schema = self - .catalog(&request.catalog) - .context(CatalogNotFoundSnafu { - catalog_name: &request.catalog, - })? - .schema(&request.schema) - .await? - .context(SchemaNotFoundSnafu { - catalog: &request.catalog, - schema: &request.schema, - })?; + let catalog = request.catalog.clone(); + let schema = request.schema.clone(); + let result = self.register_table_sync(request); increment_gauge!( crate::metrics::METRIC_CATALOG_MANAGER_TABLE_COUNT, 1.0, - &[crate::metrics::db_label(&request.catalog, &request.schema)], + &[crate::metrics::db_label(&catalog, &schema)], ); - schema - .register_table(request.table_name, request.table) - .await - .map(|v| v.is_none()) + result } async fn rename_table(&self, request: RenameTableRequest) -> Result { - let catalog = self - .catalog(&request.catalog) - .context(CatalogNotFoundSnafu { + let mut catalogs = self.catalogs.write().unwrap(); + let schema = catalogs + .get_mut(&request.catalog) + .with_context(|| CatalogNotFoundSnafu { catalog_name: &request.catalog, + })? + .get_mut(&request.schema) + .with_context(|| SchemaNotFoundSnafu { + catalog: &request.catalog, + schema: &request.schema, })?; - let schema = - catalog - .schema(&request.schema) - .await? - .with_context(|| SchemaNotFoundSnafu { - catalog: &request.catalog, - schema: &request.schema, - })?; - Ok(schema - .rename_table(&request.table_name, request.new_table_name) - .await - .is_ok()) + + // check old and new table names + if !schema.contains_key(&request.table_name) { + return TableNotFoundSnafu { + table_info: request.table_name.to_string(), + } + .fail()?; + } + if schema.contains_key(&request.new_table_name) { + return TableExistsSnafu { + table: &request.new_table_name, + } + .fail(); + } + + let table = schema.remove(&request.table_name).unwrap(); + schema.insert(request.new_table_name, table); + + Ok(true) } - async fn deregister_table(&self, request: DeregisterTableRequest) -> Result { - let schema = self - .catalog(&request.catalog) - .context(CatalogNotFoundSnafu { + async fn deregister_table(&self, request: DeregisterTableRequest) -> Result<()> { + let mut catalogs = self.catalogs.write().unwrap(); + let schema = catalogs + .get_mut(&request.catalog) + .with_context(|| CatalogNotFoundSnafu { catalog_name: &request.catalog, })? - .schema(&request.schema) - .await? + .get_mut(&request.schema) .with_context(|| SchemaNotFoundSnafu { catalog: &request.catalog, schema: &request.schema, })?; - decrement_gauge!( - crate::metrics::METRIC_CATALOG_MANAGER_TABLE_COUNT, - 1.0, - &[crate::metrics::db_label(&request.catalog, &request.schema)], - ); - schema - .deregister_table(&request.table_name) - .await - .map(|v| v.is_some()) + let result = schema.remove(&request.table_name); + if result.is_some() { + decrement_gauge!( + crate::metrics::METRIC_CATALOG_MANAGER_TABLE_COUNT, + 1.0, + &[crate::metrics::db_label(&request.catalog, &request.schema)], + ); + } + Ok(()) } async fn register_schema(&self, request: RegisterSchemaRequest) -> Result { - let catalog = self - .catalog(&request.catalog) - .context(CatalogNotFoundSnafu { - catalog_name: &request.catalog, - })?; - catalog - .register_schema(request.schema, Arc::new(MemorySchemaProvider::new())) - .await?; + self.register_schema_sync(request)?; increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0); Ok(true) } @@ -159,12 +154,16 @@ impl CatalogManager for MemoryCatalogManager { Ok(()) } - async fn schema(&self, catalog: &str, schema: &str) -> Result> { - if let Some(c) = self.catalog(catalog) { - c.schema(schema).await - } else { - Ok(None) - } + async fn schema_exist(&self, catalog: &str, schema: &str) -> Result { + Ok(self + .catalogs + .read() + .unwrap() + .get(catalog) + .with_context(|| CatalogNotFoundSnafu { + catalog_name: catalog, + })? + .contains_key(schema)) } async fn table( @@ -173,27 +172,71 @@ impl CatalogManager for MemoryCatalogManager { schema: &str, table_name: &str, ) -> Result> { - let Some(catalog) = self - .catalog(catalog) else { return Ok(None)}; - let Some(s) = catalog.schema(schema).await? else { return Ok(None) }; - s.table(table_name).await + let result = try { + self.catalogs + .read() + .unwrap() + .get(catalog)? + .get(schema)? + .get(table_name) + .cloned()? + }; + Ok(result) } - async fn catalog(&self, catalog: &str) -> Result> { - Ok(self.catalogs.read().unwrap().get(catalog).cloned()) + async fn catalog_exist(&self, catalog: &str) -> Result { + Ok(self.catalogs.read().unwrap().get(catalog).is_some()) + } + + async fn table_exist(&self, catalog: &str, schema: &str, table: &str) -> Result { + let catalogs = self.catalogs.read().unwrap(); + Ok(catalogs + .get(catalog) + .with_context(|| CatalogNotFoundSnafu { + catalog_name: catalog, + })? + .get(schema) + .with_context(|| SchemaNotFoundSnafu { catalog, schema })? + .contains_key(table)) } async fn catalog_names(&self) -> Result> { Ok(self.catalogs.read().unwrap().keys().cloned().collect()) } - async fn register_catalog( - &self, - name: String, - catalog: CatalogProviderRef, - ) -> Result> { + async fn schema_names(&self, catalog_name: &str) -> Result> { + Ok(self + .catalogs + .read() + .unwrap() + .get(catalog_name) + .with_context(|| CatalogNotFoundSnafu { catalog_name })? + .keys() + .cloned() + .collect()) + } + + async fn table_names(&self, catalog_name: &str, schema_name: &str) -> Result> { + Ok(self + .catalogs + .read() + .unwrap() + .get(catalog_name) + .with_context(|| CatalogNotFoundSnafu { catalog_name })? + .get(schema_name) + .with_context(|| SchemaNotFoundSnafu { + catalog: catalog_name, + schema: schema_name, + })? + .keys() + .cloned() + .collect()) + } + + async fn register_catalog(&self, name: String) -> Result { + self.register_catalog_sync(name)?; increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_CATALOG_COUNT, 1.0); - self.register_catalog_sync(name, catalog) + Ok(true) } fn as_any(&self) -> &dyn Any { @@ -202,206 +245,78 @@ impl CatalogManager for MemoryCatalogManager { } impl MemoryCatalogManager { - /// Registers a catalog and return `None` if no catalog with the same name was already - /// registered, or `Some` with the previously registered catalog. - pub fn register_catalog_if_absent( - &self, - name: String, - catalog: CatalogProviderRef, - ) -> Option { + /// Registers a catalog and return the catalog already exist + pub fn register_catalog_if_absent(&self, name: String) -> bool { let mut catalogs = self.catalogs.write().unwrap(); let entry = catalogs.entry(name); match entry { - Entry::Occupied(v) => Some(v.get().clone()), + Entry::Occupied(_) => true, Entry::Vacant(v) => { - v.insert(catalog); - None + v.insert(HashMap::new()); + false } } } - pub fn register_catalog_sync( - &self, - name: String, - catalog: CatalogProviderRef, - ) -> Result> { + pub fn register_catalog_sync(&self, name: String) -> Result { let mut catalogs = self.catalogs.write().unwrap(); - Ok(catalogs.insert(name, catalog)) - } - - fn catalog(&self, catalog_name: &str) -> Option { - self.catalogs.read().unwrap().get(catalog_name).cloned() - } -} - -impl Default for MemoryCatalogProvider { - fn default() -> Self { - Self::new() + Ok(catalogs.insert(name, HashMap::new()).is_some()) } -} - -/// Simple in-memory implementation of a catalog. -pub struct MemoryCatalogProvider { - schemas: RwLock>>, -} -impl MemoryCatalogProvider { - /// Instantiates a new MemoryCatalogProvider with an empty collection of schemas. - pub fn new() -> Self { - Self { - schemas: RwLock::new(HashMap::new()), + pub fn register_schema_sync(&self, request: RegisterSchemaRequest) -> Result { + let mut catalogs = self.catalogs.write().unwrap(); + let catalog = catalogs + .get_mut(&request.catalog) + .with_context(|| CatalogNotFoundSnafu { + catalog_name: &request.catalog, + })?; + if catalog.contains_key(&request.schema) { + return Ok(false); } + catalog.insert(request.schema, HashMap::new()); + Ok(true) } - pub fn schema_names_sync(&self) -> Result> { - let schemas = self.schemas.read().unwrap(); - Ok(schemas.keys().cloned().collect()) - } - - pub fn register_schema_sync( - &self, - name: String, - schema: SchemaProviderRef, - ) -> Result> { - let mut schemas = self.schemas.write().unwrap(); - ensure!( - !schemas.contains_key(&name), - error::SchemaExistsSnafu { schema: &name } - ); - increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0); - Ok(schemas.insert(name, schema)) - } - - pub fn schema_sync(&self, name: &str) -> Result>> { - let schemas = self.schemas.read().unwrap(); - Ok(schemas.get(name).cloned()) - } -} - -#[async_trait::async_trait] -impl CatalogProvider for MemoryCatalogProvider { - fn as_any(&self) -> &dyn Any { - self - } - - async fn schema_names(&self) -> Result> { - self.schema_names_sync() - } - - async fn register_schema( - &self, - name: String, - schema: SchemaProviderRef, - ) -> Result> { - self.register_schema_sync(name, schema) - } - - async fn schema(&self, name: &str) -> Result>> { - self.schema_sync(name) - } -} - -/// Simple in-memory implementation of a schema. -pub struct MemorySchemaProvider { - tables: RwLock>, -} - -impl MemorySchemaProvider { - /// Instantiates a new MemorySchemaProvider with an empty collection of tables. - pub fn new() -> Self { - Self { - tables: RwLock::new(HashMap::new()), - } - } + pub fn register_table_sync(&self, request: RegisterTableRequest) -> Result { + let mut catalogs = self.catalogs.write().unwrap(); + let schema = catalogs + .get_mut(&request.catalog) + .with_context(|| CatalogNotFoundSnafu { + catalog_name: &request.catalog, + })? + .get_mut(&request.schema) + .with_context(|| SchemaNotFoundSnafu { + catalog: &request.catalog, + schema: &request.schema, + })?; - pub fn register_table_sync(&self, name: String, table: TableRef) -> Result> { - let mut tables = self.tables.write().unwrap(); - if let Some(existing) = tables.get(name.as_str()) { - // if table with the same name but different table id exists, then it's a fatal bug - if existing.table_info().ident.table_id != table.table_info().ident.table_id { - error!( - "Unexpected table register: {:?}, existing: {:?}", - table.table_info(), - existing.table_info() - ); - return TableExistsSnafu { table: name }.fail()?; + if schema.contains_key(&request.table_name) { + return TableExistsSnafu { + table: &request.table_name, } - Ok(Some(existing.clone())) - } else { - Ok(tables.insert(name, table)) + .fail(); } - } - - pub fn rename_table_sync(&self, name: &str, new_name: String) -> Result { - let mut tables = self.tables.write().unwrap(); - let Some(table) = tables.remove(name) else { - return TableNotFoundSnafu { - table_info: name.to_string(), - } - .fail()?; - }; - let e = match tables.entry(new_name) { - Entry::Vacant(e) => e, - Entry::Occupied(e) => { - return TableExistsSnafu { table: e.key() }.fail(); - } - }; - e.insert(table.clone()); - Ok(table) - } - - pub fn table_exist_sync(&self, name: &str) -> Result { - let tables = self.tables.read().unwrap(); - Ok(tables.contains_key(name)) - } - - pub fn deregister_table_sync(&self, name: &str) -> Result> { - let mut tables = self.tables.write().unwrap(); - Ok(tables.remove(name)) - } -} -impl Default for MemorySchemaProvider { - fn default() -> Self { - Self::new() + Ok(schema.insert(request.table_name, request.table).is_none()) } -} -#[async_trait] -impl SchemaProvider for MemorySchemaProvider { - fn as_any(&self) -> &dyn Any { - self - } - - async fn table_names(&self) -> Result> { - let tables = self.tables.read().unwrap(); - Ok(tables.keys().cloned().collect()) - } - - async fn table(&self, name: &str) -> Result> { - let tables = self.tables.read().unwrap(); - Ok(tables.get(name).cloned()) - } - - async fn register_table(&self, name: String, table: TableRef) -> Result> { - self.register_table_sync(name, table) - } - - async fn rename_table(&self, name: &str, new_name: String) -> Result { - self.rename_table_sync(name, new_name) - } - - async fn deregister_table(&self, name: &str) -> Result> { - self.deregister_table_sync(name) - } - - async fn table_exist(&self, name: &str) -> Result { - self.table_exist_sync(name) + #[cfg(any(test, feature = "testing"))] + pub fn new_with_table(table: TableRef) -> Self { + let manager = Self::default(); + let request = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: table.table_info().name.clone(), + table_id: table.table_info().ident.table_id, + table, + }; + manager.register_table_sync(request).unwrap(); + manager } } /// Create a memory catalog list contains a numbers table for test -pub fn new_memory_catalog_list() -> Result> { +pub fn new_memory_catalog_manager() -> Result> { Ok(Arc::new(MemoryCatalogManager::default())) } @@ -410,88 +325,99 @@ mod tests { use common_catalog::consts::*; use common_error::ext::ErrorExt; use common_error::prelude::StatusCode; - use table::table::numbers::NumbersTable; + use table::table::numbers::{NumbersTable, NUMBERS_TABLE_NAME}; use super::*; #[tokio::test] async fn test_new_memory_catalog_list() { - let catalog_list = new_memory_catalog_list().unwrap(); - let default_catalog = CatalogManager::catalog(&*catalog_list, DEFAULT_CATALOG_NAME) - .await - .unwrap() - .unwrap(); + let catalog_list = new_memory_catalog_manager().unwrap(); - let default_schema = default_catalog - .schema(DEFAULT_SCHEMA_NAME) - .await - .unwrap() - .unwrap(); + let register_request = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: NUMBERS_TABLE_NAME.to_string(), + table_id: NUMBERS_TABLE_ID, + table: Arc::new(NumbersTable::default()), + }; - default_schema - .register_table("numbers".to_string(), Arc::new(NumbersTable::default())) + catalog_list.register_table(register_request).await.unwrap(); + let table = catalog_list + .table( + DEFAULT_CATALOG_NAME, + DEFAULT_SCHEMA_NAME, + NUMBERS_TABLE_NAME, + ) .await .unwrap(); - - let table = default_schema.table("numbers").await.unwrap(); assert!(table.is_some()); - assert!(default_schema.table("not_exists").await.unwrap().is_none()); - } - - #[tokio::test] - async fn test_mem_provider() { - let provider = MemorySchemaProvider::new(); - let table_name = "numbers"; - assert!(!provider.table_exist_sync(table_name).unwrap()); - provider.deregister_table_sync(table_name).unwrap(); - let test_table = NumbersTable::default(); - // register table successfully - assert!(provider - .register_table_sync(table_name.to_string(), Arc::new(test_table)) + assert!(catalog_list + .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, "not_exists") + .await .unwrap() .is_none()); - assert!(provider.table_exist_sync(table_name).unwrap()); - let other_table = NumbersTable::new(12); - let result = provider.register_table_sync(table_name.to_string(), Arc::new(other_table)); - let err = result.err().unwrap(); - assert_eq!(StatusCode::TableAlreadyExists, err.status_code()); } #[tokio::test] - async fn test_mem_provider_rename_table() { - let provider = MemorySchemaProvider::new(); - let table_name = "num"; - assert!(!provider.table_exist_sync(table_name).unwrap()); - let test_table: TableRef = Arc::new(NumbersTable::default()); + async fn test_mem_manager_rename_table() { + let catalog = MemoryCatalogManager::default(); + let table_name = "test_table"; + assert!(!catalog + .table_exist(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) + .await + .unwrap()); // register test table - assert!(provider - .register_table_sync(table_name.to_string(), test_table.clone()) - .unwrap() - .is_none()); - assert!(provider.table_exist_sync(table_name).unwrap()); + let table_id = 2333; + let register_request = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: table_name.to_string(), + table_id, + table: Arc::new(NumbersTable::new(table_id)), + }; + assert!(catalog.register_table(register_request).await.unwrap()); + assert!(catalog + .table_exist(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) + .await + .unwrap()); // rename test table - let new_table_name = "numbers"; - provider - .rename_table_sync(table_name, new_table_name.to_string()) - .unwrap(); + let new_table_name = "test_table_renamed"; + let rename_request = RenameTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: table_name.to_string(), + new_table_name: new_table_name.to_string(), + table_id, + }; + catalog.rename_table(rename_request).await.unwrap(); // test old table name not exist - assert!(!provider.table_exist_sync(table_name).unwrap()); - provider.deregister_table_sync(table_name).unwrap(); + assert!(!catalog + .table_exist(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) + .await + .unwrap()); // test new table name exists - assert!(provider.table_exist_sync(new_table_name).unwrap()); - let registered_table = provider.table(new_table_name).await.unwrap().unwrap(); - assert_eq!( - registered_table.table_info().ident.table_id, - test_table.table_info().ident.table_id - ); + assert!(catalog + .table_exist(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, new_table_name) + .await + .unwrap()); + let registered_table = catalog + .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, new_table_name) + .await + .unwrap() + .unwrap(); + assert_eq!(registered_table.table_info().ident.table_id, table_id); - let other_table = Arc::new(NumbersTable::new(2)); - let result = provider - .register_table(new_table_name.to_string(), other_table) - .await; + let dup_register_request = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: new_table_name.to_string(), + table_id: table_id + 1, + table: Arc::new(NumbersTable::new(table_id + 1)), + }; + let result = catalog.register_table(dup_register_request).await; let err = result.err().unwrap(); assert_eq!(StatusCode::TableAlreadyExists, err.status_code()); } @@ -499,16 +425,11 @@ mod tests { #[tokio::test] async fn test_catalog_rename_table() { let catalog = MemoryCatalogManager::default(); - let schema = catalog - .schema(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME) - .await - .unwrap() - .unwrap(); - - // register table let table_name = "num"; let table_id = 2333; let table: TableRef = Arc::new(NumbersTable::new(table_id)); + + // register table let register_table_req = RegisterTableRequest { catalog: DEFAULT_CATALOG_NAME.to_string(), schema: DEFAULT_SCHEMA_NAME.to_string(), @@ -517,7 +438,11 @@ mod tests { table, }; assert!(catalog.register_table(register_table_req).await.unwrap()); - assert!(schema.table_exist(table_name).await.unwrap()); + assert!(catalog + .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) + .await + .unwrap() + .is_some()); // rename table let new_table_name = "numbers_new"; @@ -529,8 +454,16 @@ mod tests { table_id, }; assert!(catalog.rename_table(rename_table_req).await.unwrap()); - assert!(!schema.table_exist(table_name).await.unwrap()); - assert!(schema.table_exist(new_table_name).await.unwrap()); + assert!(catalog + .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) + .await + .unwrap() + .is_none()); + assert!(catalog + .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, new_table_name) + .await + .unwrap() + .is_some()); let registered_table = catalog .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, new_table_name) @@ -543,50 +476,42 @@ mod tests { #[test] pub fn test_register_if_absent() { let list = MemoryCatalogManager::default(); - assert!(list - .register_catalog_if_absent( - "test_catalog".to_string(), - Arc::new(MemoryCatalogProvider::new()) - ) - .is_none()); - list.register_catalog_if_absent( - "test_catalog".to_string(), - Arc::new(MemoryCatalogProvider::new()), - ) - .unwrap(); - list.as_any() - .downcast_ref::() - .unwrap(); + assert!(!list.register_catalog_if_absent("test_catalog".to_string(),)); + assert!(list.register_catalog_if_absent("test_catalog".to_string())); } #[tokio::test] pub async fn test_catalog_deregister_table() { let catalog = MemoryCatalogManager::default(); - let schema = catalog - .schema(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME) - .await - .unwrap() - .unwrap(); + let table_name = "foo_table"; let register_table_req = RegisterTableRequest { catalog: DEFAULT_CATALOG_NAME.to_string(), schema: DEFAULT_SCHEMA_NAME.to_string(), - table_name: "numbers".to_string(), + table_name: table_name.to_string(), table_id: 2333, table: Arc::new(NumbersTable::default()), }; catalog.register_table(register_table_req).await.unwrap(); - assert!(schema.table_exist("numbers").await.unwrap()); + assert!(catalog + .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) + .await + .unwrap() + .is_some()); let deregister_table_req = DeregisterTableRequest { catalog: DEFAULT_CATALOG_NAME.to_string(), schema: DEFAULT_SCHEMA_NAME.to_string(), - table_name: "numbers".to_string(), + table_name: table_name.to_string(), }; catalog .deregister_table(deregister_table_req) .await .unwrap(); - assert!(!schema.table_exist("numbers").await.unwrap()); + assert!(catalog + .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) + .await + .unwrap() + .is_none()); } } diff --git a/src/catalog/src/remote.rs b/src/catalog/src/remote.rs index da32e53bf173..e82dd327153d 100644 --- a/src/catalog/src/remote.rs +++ b/src/catalog/src/remote.rs @@ -20,7 +20,7 @@ use std::sync::Arc; pub use client::{CachedMetaKvBackend, MetaKvBackend}; use futures::Stream; use futures_util::StreamExt; -pub use manager::{RemoteCatalogManager, RemoteCatalogProvider, RemoteSchemaProvider}; +pub use manager::RemoteCatalogManager; use crate::error::Error; diff --git a/src/catalog/src/remote/manager.rs b/src/catalog/src/remote/manager.rs index 4bc5d4c7d881..e6cc824f0811 100644 --- a/src/catalog/src/remote/manager.rs +++ b/src/catalog/src/remote/manager.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::any::Any; -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use std::pin::Pin; use std::sync::Arc; @@ -22,12 +22,10 @@ use async_trait::async_trait; use common_catalog::consts::{MAX_SYS_TABLE_ID, MITO_ENGINE}; use common_meta::ident::TableIdent; use common_telemetry::{debug, error, info, warn}; -use dashmap::DashMap; use futures::Stream; use futures_util::{StreamExt, TryStreamExt}; use metrics::{decrement_gauge, increment_gauge}; -use parking_lot::RwLock; -use snafu::{ensure, OptionExt, ResultExt}; +use snafu::ResultExt; use table::engine::manager::TableEngineManagerRef; use table::engine::{EngineContext, TableReference}; use table::requests::{CreateTableRequest, OpenTableRequest}; @@ -37,7 +35,6 @@ use tokio::sync::Mutex; use crate::error::{ CatalogNotFoundSnafu, CreateTableSnafu, InvalidCatalogValueSnafu, OpenTableSnafu, ParallelOpenTableSnafu, Result, SchemaNotFoundSnafu, TableEngineNotFoundSnafu, - TableExistsSnafu, UnimplementedSnafu, }; use crate::helper::{ build_catalog_prefix, build_schema_prefix, build_table_global_prefix, @@ -47,16 +44,14 @@ use crate::helper::{ use crate::remote::region_alive_keeper::RegionAliveKeepers; use crate::remote::{Kv, KvBackendRef}; use crate::{ - handle_system_table_request, CatalogManager, CatalogProvider, CatalogProviderRef, - DeregisterTableRequest, RegisterSchemaRequest, RegisterSystemTableRequest, - RegisterTableRequest, RenameTableRequest, SchemaProvider, SchemaProviderRef, + handle_system_table_request, CatalogManager, DeregisterTableRequest, RegisterSchemaRequest, + RegisterSystemTableRequest, RegisterTableRequest, RenameTableRequest, }; /// Catalog manager based on metasrv. pub struct RemoteCatalogManager { node_id: u64, backend: KvBackendRef, - catalogs: Arc>>, engine_manager: TableEngineManagerRef, system_table_requests: Mutex>, region_alive_keepers: Arc, @@ -73,27 +68,15 @@ impl RemoteCatalogManager { engine_manager, node_id, backend, - catalogs: Default::default(), system_table_requests: Default::default(), region_alive_keepers, } } - fn new_catalog_provider(&self, catalog_name: &str) -> CatalogProviderRef { - Arc::new(RemoteCatalogProvider { - node_id: self.node_id, - catalog_name: catalog_name.to_string(), - backend: self.backend.clone(), - engine_manager: self.engine_manager.clone(), - region_alive_keepers: self.region_alive_keepers.clone(), - }) as _ - } - async fn iter_remote_catalogs( &self, ) -> Pin> + Send + '_>> { let catalog_range_prefix = build_catalog_prefix(); - info!("catalog_range_prefix: {}", catalog_range_prefix); let mut catalogs = self.backend.range(catalog_range_prefix.as_bytes()); Box::pin(stream!({ while let Some(r) = catalogs.next().await { @@ -114,72 +97,31 @@ impl RemoteCatalogManager { } /// Fetch catalogs/schemas/tables from remote catalog manager along with max table id allocated. - async fn initiate_catalogs(&self) -> Result> { - let mut res = HashMap::new(); - + async fn initiate_catalogs(&self) -> Result<()> { let mut catalogs = self.iter_remote_catalogs().await; let mut joins = Vec::new(); while let Some(r) = catalogs.next().await { let CatalogKey { catalog_name, .. } = r?; info!("Fetch catalog from metasrv: {}", catalog_name); - let catalog = res - .entry(catalog_name.clone()) - .or_insert_with(|| self.new_catalog_provider(&catalog_name)) - .clone(); let node_id = self.node_id; let backend = self.backend.clone(); let engine_manager = self.engine_manager.clone(); increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_CATALOG_COUNT, 1.0); - - let region_alive_keepers = self.region_alive_keepers.clone(); - joins.push(common_runtime::spawn_bg(async move { - let max_table_id = initiate_schemas( - node_id, - backend, - engine_manager, - &catalog_name, - catalog, - region_alive_keepers, - ) - .await?; - info!( - "Catalog name: {}, max table id allocated: {}", - &catalog_name, max_table_id - ); - Ok(()) - })); + joins.push(self.initiate_schemas(node_id, backend, engine_manager, catalog_name)); } - futures::future::try_join_all(joins) - .await - .context(ParallelOpenTableSnafu)? - .into_iter() - .collect::>>()?; + futures::future::try_join_all(joins).await?; - Ok(res) + Ok(()) } pub async fn create_catalog_and_schema( &self, catalog_name: &str, schema_name: &str, - ) -> Result { - let schema_provider = new_schema_provider( - self.node_id, - self.backend.clone(), - self.engine_manager.clone(), - catalog_name, - schema_name, - self.region_alive_keepers.clone(), - ); - - let catalog_provider = self.new_catalog_provider(catalog_name); - catalog_provider - .register_schema(schema_name.to_string(), schema_provider.clone()) - .await?; - + ) -> Result<()> { let schema_key = SchemaKey { catalog_name: catalog_name.to_string(), schema_name: schema_name.to_string(), @@ -208,26 +150,273 @@ impl RemoteCatalogManager { ) .await?; info!("Created catalog '{catalog_key}"); - Ok(catalog_provider) + Ok(()) } -} -fn new_schema_provider( - node_id: u64, - backend: KvBackendRef, - engine_manager: TableEngineManagerRef, - catalog_name: &str, - schema_name: &str, - region_alive_keepers: Arc, -) -> SchemaProviderRef { - Arc::new(RemoteSchemaProvider { - catalog_name: catalog_name.to_string(), - schema_name: schema_name.to_string(), - node_id, - backend, - engine_manager, - region_alive_keepers, - }) as _ + fn build_schema_key(&self, catalog_name: String, schema_name: String) -> SchemaKey { + SchemaKey { + catalog_name, + schema_name, + } + } + + /// Initiates all tables inside the catalog by fetching data from metasrv. + /// Return maximum table id in the schema. + async fn initiate_tables( + &self, + node_id: u64, + backend: KvBackendRef, + engine_manager: TableEngineManagerRef, + catalog_name: String, + schema_name: String, + ) -> Result { + info!("initializing tables in {}.{}", catalog_name, schema_name); + let tables = iter_remote_tables(node_id, &backend, &catalog_name, &schema_name).await; + + let kvs = tables.try_collect::>().await?; + let table_num = kvs.len(); + let joins = kvs + .into_iter() + .map(|(table_key, table_value)| { + let engine_manager = engine_manager.clone(); + let backend = backend.clone(); + common_runtime::spawn_bg(async move { + match open_or_create_table(node_id, engine_manager, &table_key, &table_value) + .await + { + Ok(table_ref) => { + let table_info = table_ref.table_info(); + let table_name = &table_info.name; + info!("Registered table {}", table_name); + Ok(Some(table_info.ident.table_id)) + } + Err(err) => { + warn!( + "Node id: {}, failed to open table: {}, source: {}", + node_id, table_key, err + ); + debug!( + "Node id: {}, TableGlobalKey: {}, value: {:?},", + node_id, table_key, table_value + ); + print_regional_key_debug_info(node_id, backend, &table_key).await; + + Ok(None) + } + } + }) + }) + .collect::>(); + + let opened_table_ids = futures::future::try_join_all(joins) + .await + .context(ParallelOpenTableSnafu)? + .into_iter() + .collect::>>()? + .into_iter() + .flatten() + .collect::>(); + + let opened = opened_table_ids.len(); + + let max_table_id = opened_table_ids + .into_iter() + .max() + .unwrap_or(MAX_SYS_TABLE_ID); + + increment_gauge!( + crate::metrics::METRIC_CATALOG_MANAGER_TABLE_COUNT, + table_num as f64, + &[crate::metrics::db_label(&catalog_name, &schema_name)], + ); + info!( + "initialized tables in {}.{}, total: {}, opened: {}, failed: {}", + catalog_name, + schema_name, + table_num, + opened, + table_num - opened + ); + + Ok(max_table_id) + } + + /// Initiates all schemas inside the catalog by fetching data from metasrv. + /// Return maximum table id in the catalog. + async fn initiate_schemas( + &self, + node_id: u64, + backend: KvBackendRef, + engine_manager: TableEngineManagerRef, + catalog_name: String, + ) -> Result<()> { + let mut schemas = iter_remote_schemas(&backend, &catalog_name).await; + let mut joins = Vec::new(); + while let Some(r) = schemas.next().await { + let SchemaKey { + catalog_name, + schema_name, + .. + } = r?; + + info!( + "Fetch schema from metasrv: {}.{}", + &catalog_name, &schema_name + ); + increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0); + + let backend = backend.clone(); + let engine_manager = engine_manager.clone(); + + joins.push(self.initiate_tables( + node_id, + backend, + engine_manager, + catalog_name, + schema_name, + )); + } + + let mut max_table_id = MAX_SYS_TABLE_ID; + if let Some(found_max_table_id) = futures::future::try_join_all(joins) + .await? + .into_iter() + .max() + { + max_table_id = max_table_id.max(found_max_table_id); + info!( + "Catalog name: {}, max table id allocated: {}", + catalog_name, max_table_id + ); + } + + Ok(()) + } + + async fn register_table( + &self, + catalog_name: String, + schema_name: String, + table_name: String, + table: TableRef, + ) -> Result> { + let table_info = table.table_info(); + let table_version = table_info.ident.version; + let table_value = TableRegionalValue { + table_id: Some(table.table_info().ident.table_id), + version: table_version, + regions_ids: table.table_info().meta.region_numbers.clone(), + engine_name: Some(table_info.meta.engine.clone()), + }; + let table_key = self + .build_regional_table_key(catalog_name, schema_name, table_name) + .to_string(); + self.backend + .set( + table_key.as_bytes(), + &table_value.as_bytes().context(InvalidCatalogValueSnafu)?, + ) + .await?; + debug!( + "Successfully set catalog table entry, key: {}, table value: {:?}", + table_key, table_value + ); + + // TODO(hl): retrieve prev table info using cas + Ok(None) + } + + async fn deregister_table( + &self, + catalog_name: String, + schema_name: String, + table_name: String, + ) -> Result> { + let table_key = self + .build_regional_table_key( + catalog_name.clone(), + schema_name.clone(), + table_name.clone(), + ) + .to_string(); + + let engine_opt = self + .backend + .get(table_key.as_bytes()) + .await? + .map(|Kv(_, v)| { + let TableRegionalValue { + table_id, + engine_name, + .. + } = TableRegionalValue::parse(String::from_utf8_lossy(&v)) + .context(InvalidCatalogValueSnafu)?; + Ok(engine_name.and_then(|name| table_id.map(|id| (name, id)))) + }) + .transpose()? + .flatten(); + + let Some((engine_name, table_id)) = engine_opt else { + warn!("Cannot find table id and engine name for {table_key}"); + return Ok(None); + }; + + self.backend.delete(table_key.as_bytes()).await?; + debug!( + "Successfully deleted catalog table entry, key: {}", + table_key + ); + + // deregistering table does not necessarily mean dropping the table + let table = self + .engine_manager + .engine(&engine_name) + .context(TableEngineNotFoundSnafu { engine_name })? + .get_table(&EngineContext {}, table_id) + .with_context(|_| { + let reference = TableReference { + catalog: &catalog_name, + schema: &schema_name, + table: &table_name, + }; + OpenTableSnafu { + table_info: reference.to_string(), + } + })?; + Ok(table) + } + + fn build_regional_table_key( + &self, + catalog_name: String, + schema_name: String, + table_name: String, + ) -> TableRegionalKey { + TableRegionalKey { + catalog_name, + schema_name, + table_name, + node_id: self.node_id, + } + } + + async fn check_catalog_schema_exist( + &self, + catalog_name: &str, + schema_name: &str, + ) -> Result<()> { + if !self.catalog_exist(catalog_name).await? { + return CatalogNotFoundSnafu { catalog_name }.fail()?; + } + if !self.schema_exist(catalog_name, schema_name).await? { + return SchemaNotFoundSnafu { + catalog: catalog_name, + schema: schema_name, + } + .fail()?; + } + Ok(()) + } } async fn iter_remote_schemas<'a>( @@ -252,82 +441,6 @@ async fn iter_remote_schemas<'a>( })) } -/// Initiates all schemas inside the catalog by fetching data from metasrv. -/// Return maximum table id in the catalog. -async fn initiate_schemas( - node_id: u64, - backend: KvBackendRef, - engine_manager: TableEngineManagerRef, - catalog_name: &str, - catalog: CatalogProviderRef, - region_alive_keepers: Arc, -) -> Result { - let mut schemas = iter_remote_schemas(&backend, catalog_name).await; - let mut joins = Vec::new(); - while let Some(r) = schemas.next().await { - let SchemaKey { - catalog_name, - schema_name, - .. - } = r?; - - info!("Found schema: {}.{}", catalog_name, schema_name); - let schema = match catalog.schema(&schema_name).await? { - None => { - let schema = new_schema_provider( - node_id, - backend.clone(), - engine_manager.clone(), - &catalog_name, - &schema_name, - region_alive_keepers.clone(), - ); - catalog - .register_schema(schema_name.clone(), schema.clone()) - .await?; - info!("Registered schema: {}", &schema_name); - schema - } - Some(schema) => schema, - }; - - info!( - "Fetch schema from metasrv: {}.{}", - &catalog_name, &schema_name - ); - increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0); - - let backend = backend.clone(); - let engine_manager = engine_manager.clone(); - - joins.push(common_runtime::spawn_bg(async move { - initiate_tables( - node_id, - backend, - engine_manager, - &catalog_name, - &schema_name, - schema, - ) - .await - })); - } - - let mut max_table_id = MAX_SYS_TABLE_ID; - if let Some(found_max_table_id) = futures::future::try_join_all(joins) - .await - .context(ParallelOpenTableSnafu)? - .into_iter() - .collect::>>()? - .into_iter() - .max() - { - max_table_id = max_table_id.max(found_max_table_id); - } - - Ok(max_table_id) -} - /// Iterate over all table entries on metasrv async fn iter_remote_tables<'a>( node_id: u64, @@ -402,88 +515,6 @@ async fn print_regional_key_debug_info( } } -/// Initiates all tables inside the catalog by fetching data from metasrv. -/// Return maximum table id in the schema. -async fn initiate_tables( - node_id: u64, - backend: KvBackendRef, - engine_manager: TableEngineManagerRef, - catalog_name: &str, - schema_name: &str, - schema: SchemaProviderRef, -) -> Result { - info!("initializing tables in {}.{}", catalog_name, schema_name); - let tables = iter_remote_tables(node_id, &backend, catalog_name, schema_name).await; - - let kvs = tables.try_collect::>().await?; - let table_num = kvs.len(); - let joins = kvs - .into_iter() - .map(|(table_key, table_value)| { - let engine_manager = engine_manager.clone(); - let schema = schema.clone(); - let backend = backend.clone(); - common_runtime::spawn_bg(async move { - match open_or_create_table(node_id, engine_manager, &table_key, &table_value).await - { - Ok(table_ref) => { - let table_info = table_ref.table_info(); - let table_name = &table_info.name; - schema.register_table(table_name.clone(), table_ref).await?; - info!("Registered table {}", table_name); - Ok(Some(table_info.ident.table_id)) - } - Err(err) => { - warn!( - "Node id: {}, failed to open table: {}, source: {}", - node_id, table_key, err - ); - debug!( - "Node id: {}, TableGlobalKey: {}, value: {:?},", - node_id, table_key, table_value - ); - print_regional_key_debug_info(node_id, backend, &table_key).await; - - Ok(None) - } - } - }) - }) - .collect::>(); - - let opened_table_ids = futures::future::try_join_all(joins) - .await - .context(ParallelOpenTableSnafu)? - .into_iter() - .collect::>>()? - .into_iter() - .flatten() - .collect::>(); - - let opened = opened_table_ids.len(); - - let max_table_id = opened_table_ids - .into_iter() - .max() - .unwrap_or(MAX_SYS_TABLE_ID); - - increment_gauge!( - crate::metrics::METRIC_CATALOG_MANAGER_TABLE_COUNT, - table_num as f64, - &[crate::metrics::db_label(catalog_name, schema_name)], - ); - info!( - "initialized tables in {}.{}, total: {}, opened: {}, failed: {}", - catalog_name, - schema_name, - table_num, - opened, - table_num - opened - ); - - Ok(max_table_id) -} - async fn open_or_create_table( node_id: u64, engine_manager: TableEngineManagerRef, @@ -569,21 +600,10 @@ async fn open_or_create_table( } } -#[async_trait::async_trait] +#[async_trait] impl CatalogManager for RemoteCatalogManager { async fn start(&self) -> Result<()> { - let catalogs = self.initiate_catalogs().await?; - info!( - "Initialized catalogs: {:?}", - catalogs.keys().cloned().collect::>() - ); - - { - let self_catalogs = self.catalogs.read(); - catalogs.into_iter().for_each(|(k, v)| { - self_catalogs.insert(k, v); - }); - } + self.initiate_catalogs().await?; let mut system_table_requests = self.system_table_requests.lock().await; let engine = self @@ -598,62 +618,65 @@ impl CatalogManager for RemoteCatalogManager { } async fn register_table(&self, request: RegisterTableRequest) -> Result { - let catalog = &request.catalog; - let schema = &request.schema; - let table_name = &request.table_name; + let catalog_name = request.catalog; + let schema_name = request.schema; + self.check_catalog_schema_exist(&catalog_name, &schema_name) + .await?; - let schema_provider = self - .catalog(catalog) - .await? - .context(CatalogNotFoundSnafu { - catalog_name: catalog, - })? - .schema(schema) - .await? - .context(SchemaNotFoundSnafu { catalog, schema })?; - ensure!( - !schema_provider.table_exist(table_name).await?, - TableExistsSnafu { - table: common_catalog::format_full_table_name(catalog, schema, table_name), - } - ); + self.register_table( + catalog_name.clone(), + schema_name.clone(), + request.table_name, + request.table.clone(), + ) + .await?; + + let table_info = request.table.table_info(); + let table_ident = TableIdent { + catalog: table_info.catalog_name.clone(), + schema: table_info.schema_name.clone(), + table: table_info.name.clone(), + table_id: table_info.ident.table_id, + engine: table_info.meta.engine.clone(), + }; + self.region_alive_keepers + .register_table(table_ident, request.table) + .await?; increment_gauge!( crate::metrics::METRIC_CATALOG_MANAGER_TABLE_COUNT, 1.0, - &[crate::metrics::db_label(catalog, schema)], + &[crate::metrics::db_label(&catalog_name, &schema_name)], ); - schema_provider - .register_table(table_name.to_string(), request.table) - .await?; - Ok(true) } - async fn deregister_table(&self, request: DeregisterTableRequest) -> Result { - let catalog_name = &request.catalog; - let schema_name = &request.schema; + async fn deregister_table(&self, request: DeregisterTableRequest) -> Result<()> { + let catalog_name = request.catalog; + let schema_name = request.schema; + let table_name = request.table_name; + self.check_catalog_schema_exist(&catalog_name, &schema_name) + .await?; + let result = self - .schema(catalog_name, schema_name) - .await? - .context(SchemaNotFoundSnafu { - catalog: catalog_name, - schema: schema_name, - })? - .deregister_table(&request.table_name) + .deregister_table( + catalog_name.clone(), + schema_name.clone(), + table_name.clone(), + ) .await?; decrement_gauge!( crate::metrics::METRIC_CATALOG_MANAGER_TABLE_COUNT, 1.0, - &[crate::metrics::db_label(catalog_name, schema_name)], + &[crate::metrics::db_label(&catalog_name, &schema_name)], ); if let Some(table) = result.as_ref() { let table_info = table.table_info(); - let table_ident = TableIdent { - catalog: request.catalog, - schema: request.schema, - table: request.table_name, + let table_ident: TableIdent = TableIdent { + catalog: catalog_name, + schema: schema_name, + table: table_name, table_id: table_info.ident.table_id, engine: table_info.meta.engine.clone(), }; @@ -662,34 +685,32 @@ impl CatalogManager for RemoteCatalogManager { .await; } - Ok(true) + Ok(()) } async fn register_schema(&self, request: RegisterSchemaRequest) -> Result { let catalog_name = request.catalog; let schema_name = request.schema; - let catalog_provider = - self.catalog(&catalog_name) - .await? - .context(CatalogNotFoundSnafu { - catalog_name: &catalog_name, - })?; - let schema_provider = new_schema_provider( - self.node_id, - self.backend.clone(), - self.engine_manager.clone(), - &catalog_name, - &schema_name, - self.region_alive_keepers.clone(), - ); - catalog_provider - .register_schema(schema_name, schema_provider) + let key = self.build_schema_key(catalog_name, schema_name).to_string(); + self.backend + .set( + key.as_bytes(), + &SchemaValue {} + .as_bytes() + .context(InvalidCatalogValueSnafu)?, + ) .await?; + increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_SCHEMA_COUNT, 1.0); Ok(true) } async fn rename_table(&self, request: RenameTableRequest) -> Result { + let catalog_name = request.catalog.clone(); + let schema_name = request.schema.clone(); + self.check_catalog_schema_exist(&catalog_name, &schema_name) + .await?; + let old_table_key = TableRegionalKey { catalog_name: request.catalog.clone(), schema_name: request.schema.clone(), @@ -729,14 +750,11 @@ impl CatalogManager for RemoteCatalogManager { Ok(()) } - async fn schema(&self, catalog: &str, schema: &str) -> Result> { - self.catalog(catalog) - .await? - .context(CatalogNotFoundSnafu { - catalog_name: catalog, - })? - .schema(schema) - .await + async fn schema_exist(&self, catalog: &str, schema: &str) -> Result { + let key = self + .build_schema_key(catalog.to_string(), schema.to_string()) + .to_string(); + Ok(self.backend.get(key.as_bytes()).await?.is_some()) } async fn table( @@ -745,240 +763,16 @@ impl CatalogManager for RemoteCatalogManager { schema_name: &str, table_name: &str, ) -> Result> { - let catalog = self - .catalog(catalog_name) - .await? - .with_context(|| CatalogNotFoundSnafu { catalog_name })?; - let schema = catalog - .schema(schema_name) - .await? - .with_context(|| SchemaNotFoundSnafu { - catalog: catalog_name, - schema: schema_name, - })?; - schema.table(table_name).await - } - - async fn catalog(&self, catalog: &str) -> Result> { - let key = CatalogKey { - catalog_name: catalog.to_string(), - }; - Ok(self - .backend - .get(key.to_string().as_bytes()) - .await? - .map(|_| self.new_catalog_provider(catalog))) - } - - async fn catalog_names(&self) -> Result> { - let mut stream = self.backend.range(CATALOG_KEY_PREFIX.as_bytes()); - let mut catalogs = HashSet::new(); - - while let Some(catalog) = stream.next().await { - if let Ok(catalog) = catalog { - let catalog_key = String::from_utf8_lossy(&catalog.0); - - if let Ok(key) = CatalogKey::parse(&catalog_key) { - catalogs.insert(key.catalog_name); - } - } - } - - Ok(catalogs.into_iter().collect()) - } - - async fn register_catalog( - &self, - name: String, - _catalog: CatalogProviderRef, - ) -> Result> { - let key = CatalogKey { catalog_name: name }.to_string(); - // TODO(hl): use compare_and_swap to prevent concurrent update - self.backend - .set( - key.as_bytes(), - &CatalogValue {} - .as_bytes() - .context(InvalidCatalogValueSnafu)?, - ) + self.check_catalog_schema_exist(catalog_name, schema_name) .await?; - increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_CATALOG_COUNT, 1.0); - Ok(None) - } - fn as_any(&self) -> &dyn Any { - self - } -} - -pub struct RemoteCatalogProvider { - node_id: u64, - catalog_name: String, - backend: KvBackendRef, - engine_manager: TableEngineManagerRef, - region_alive_keepers: Arc, -} - -impl RemoteCatalogProvider { - pub fn new( - catalog_name: String, - backend: KvBackendRef, - engine_manager: TableEngineManagerRef, - node_id: u64, - region_alive_keepers: Arc, - ) -> Self { - Self { - node_id, - catalog_name, - backend, - engine_manager, - region_alive_keepers, - } - } - - fn build_schema_key(&self, schema_name: impl AsRef) -> SchemaKey { - SchemaKey { - catalog_name: self.catalog_name.clone(), - schema_name: schema_name.as_ref().to_string(), - } - } - - fn build_schema_provider(&self, schema_name: &str) -> SchemaProviderRef { - let provider = RemoteSchemaProvider { - catalog_name: self.catalog_name.clone(), - schema_name: schema_name.to_string(), - node_id: self.node_id, - backend: self.backend.clone(), - engine_manager: self.engine_manager.clone(), - region_alive_keepers: self.region_alive_keepers.clone(), - }; - Arc::new(provider) as Arc<_> - } -} - -#[async_trait::async_trait] -impl CatalogProvider for RemoteCatalogProvider { - fn as_any(&self) -> &dyn Any { - self - } - - async fn schema_names(&self) -> Result> { - let schema_prefix = build_schema_prefix(&self.catalog_name); - - let remote_schemas = self.backend.range(schema_prefix.as_bytes()); - let res = remote_schemas - .map(|kv| { - let Kv(k, _) = kv?; - let schema_key = SchemaKey::parse(String::from_utf8_lossy(&k)) - .context(InvalidCatalogValueSnafu)?; - Ok(schema_key.schema_name) - }) - .try_collect() - .await?; - Ok(res) - } - - async fn register_schema( - &self, - name: String, - schema: SchemaProviderRef, - ) -> Result> { - let _ = schema; // we don't care about schema provider - let key = self.build_schema_key(&name).to_string(); - self.backend - .set( - key.as_bytes(), - &SchemaValue {} - .as_bytes() - .context(InvalidCatalogValueSnafu)?, + let key = self + .build_regional_table_key( + catalog_name.to_string(), + schema_name.to_string(), + table_name.to_string(), ) - .await?; - // TODO(hl): maybe return preview schema by cas - Ok(None) - } - - async fn schema(&self, name: &str) -> Result> { - let key = self.build_schema_key(name).to_string(); - Ok(self - .backend - .get(key.as_bytes()) - .await? - .map(|_| self.build_schema_provider(name))) - } -} - -pub struct RemoteSchemaProvider { - catalog_name: String, - schema_name: String, - node_id: u64, - backend: KvBackendRef, - engine_manager: TableEngineManagerRef, - region_alive_keepers: Arc, -} - -impl RemoteSchemaProvider { - pub fn new( - catalog_name: String, - schema_name: String, - node_id: u64, - engine_manager: TableEngineManagerRef, - backend: KvBackendRef, - region_alive_keepers: Arc, - ) -> Self { - Self { - catalog_name, - schema_name, - node_id, - backend, - engine_manager, - region_alive_keepers, - } - } - - fn build_regional_table_key(&self, table_name: impl AsRef) -> TableRegionalKey { - TableRegionalKey { - catalog_name: self.catalog_name.clone(), - schema_name: self.schema_name.clone(), - table_name: table_name.as_ref().to_string(), - node_id: self.node_id, - } - } -} - -#[async_trait] -impl SchemaProvider for RemoteSchemaProvider { - fn as_any(&self) -> &dyn Any { - self - } - - async fn table_names(&self) -> Result> { - let key_prefix = build_table_regional_prefix(&self.catalog_name, &self.schema_name); - let iter = self.backend.range(key_prefix.as_bytes()); - let regional_keys = iter - .map(|kv| { - let Kv(key, _) = kv?; - let regional_key = TableRegionalKey::parse(String::from_utf8_lossy(&key)) - .context(InvalidCatalogValueSnafu)?; - Ok(regional_key) - }) - .try_collect::>() - .await?; - - let table_names = regional_keys - .into_iter() - .filter_map(|x| { - if x.node_id == self.node_id { - Some(x.table_name) - } else { - None - } - }) - .collect(); - Ok(table_names) - } - - async fn table(&self, name: &str) -> Result> { - let key = self.build_regional_table_key(name).to_string(); + .to_string(); let table_opt = self .backend .get(key.as_bytes()) @@ -1001,9 +795,9 @@ impl SchemaProvider for RemoteSchemaProvider { .engine(engine_name) .context(TableEngineNotFoundSnafu { engine_name })?; let reference = TableReference { - catalog: &self.catalog_name, - schema: &self.schema_name, - table: name, + catalog: catalog_name, + schema: schema_name, + table: table_name, }; let table = engine .get_table(&EngineContext {}, table_id) @@ -1018,101 +812,103 @@ impl SchemaProvider for RemoteSchemaProvider { Ok(table_opt) } - async fn register_table(&self, name: String, table: TableRef) -> Result> { - // Currently, initiate_tables() always call this method to register the table to the schema thus we - // always update the region value. - let table_info = table.table_info(); - let table_version = table_info.ident.version; - let table_value = TableRegionalValue { - table_id: Some(table_info.ident.table_id), - version: table_version, - regions_ids: table.table_info().meta.region_numbers.clone(), - engine_name: Some(table_info.meta.engine.clone()), + async fn catalog_exist(&self, catalog: &str) -> Result { + let key = CatalogKey { + catalog_name: catalog.to_string(), }; - let table_key = self.build_regional_table_key(&name).to_string(); - self.backend - .set( - table_key.as_bytes(), - &table_value.as_bytes().context(InvalidCatalogValueSnafu)?, - ) - .await?; + Ok(self + .backend + .get(key.to_string().as_bytes()) + .await? + .is_some()) + } - let table_ident = TableIdent { - catalog: table_info.catalog_name.clone(), - schema: table_info.schema_name.clone(), - table: table_info.name.clone(), - table_id: table_info.ident.table_id, - engine: table_info.meta.engine.clone(), - }; - self.region_alive_keepers - .register_table(table_ident, table) - .await?; + async fn table_exist(&self, catalog: &str, schema: &str, table: &str) -> Result { + self.check_catalog_schema_exist(catalog, schema).await?; - debug!( - "Successfully set catalog table entry, key: {}, table value: {:?}", - table_key, table_value - ); + let key = TableRegionalKey { + catalog_name: catalog.to_string(), + schema_name: schema.to_string(), + table_name: table.to_string(), + node_id: self.node_id, + } + .to_string(); - // TODO(hl): retrieve prev table info using cas - Ok(None) + Ok(self.backend.get(key.as_bytes()).await?.is_some()) + } + + async fn catalog_names(&self) -> Result> { + let mut stream = self.backend.range(CATALOG_KEY_PREFIX.as_bytes()); + let mut catalogs = HashSet::new(); + + while let Some(catalog) = stream.next().await { + if let Ok(catalog) = catalog { + let catalog_key = String::from_utf8_lossy(&catalog.0); + + if let Ok(key) = CatalogKey::parse(&catalog_key) { + catalogs.insert(key.catalog_name); + } + } + } + + Ok(catalogs.into_iter().collect()) } - async fn rename_table(&self, _name: &str, _new_name: String) -> Result { - UnimplementedSnafu { - operation: "rename table", + async fn schema_names(&self, catalog_name: &str) -> Result> { + let mut stream = self + .backend + .range(build_schema_prefix(catalog_name).as_bytes()); + let mut schemas = HashSet::new(); + + while let Some(schema) = stream.next().await { + if let Ok(schema) = schema { + let schema_key = String::from_utf8_lossy(&schema.0); + + if let Ok(key) = SchemaKey::parse(&schema_key) { + schemas.insert(key.schema_name); + } + } } - .fail() + Ok(schemas.into_iter().collect()) } - async fn deregister_table(&self, name: &str) -> Result> { - let table_key = self.build_regional_table_key(name).to_string(); + async fn table_names(&self, catalog_name: &str, schema_name: &str) -> Result> { + self.check_catalog_schema_exist(catalog_name, schema_name) + .await?; - let engine_opt = self + let mut stream = self .backend - .get(table_key.as_bytes()) - .await? - .map(|Kv(_, v)| { - let TableRegionalValue { - table_id, - engine_name, - .. - } = TableRegionalValue::parse(String::from_utf8_lossy(&v)) - .context(InvalidCatalogValueSnafu)?; - Ok(engine_name.and_then(|name| table_id.map(|id| (name, id)))) - }) - .transpose()? - .flatten(); + .range(build_table_regional_prefix(catalog_name, schema_name).as_bytes()); + let mut tables = HashSet::new(); - self.backend.delete(table_key.as_bytes()).await?; - debug!( - "Successfully deleted catalog table entry, key: {}", - table_key - ); + while let Some(table) = stream.next().await { + if let Ok(table) = table { + let table_key = String::from_utf8_lossy(&table.0); - let Some((engine_name, table_id)) = engine_opt else { - warn!("Cannot find table id and engine name for {table_key}"); - return Ok(None); - }; - let reference = TableReference { - catalog: &self.catalog_name, - schema: &self.schema_name, - table: name, - }; - // deregistering table does not necessarily mean dropping the table - let table = self - .engine_manager - .engine(&engine_name) - .context(TableEngineNotFoundSnafu { engine_name })? - .get_table(&EngineContext {}, table_id) - .with_context(|_| OpenTableSnafu { - table_info: reference.to_string(), - })?; - Ok(table) + if let Ok(key) = TableRegionalKey::parse(&table_key) { + tables.insert(key.table_name); + } + } + } + Ok(tables.into_iter().collect()) } - /// Checks if table exists in schema provider based on locally opened table map. - async fn table_exist(&self, name: &str) -> Result { - let key = self.build_regional_table_key(name).to_string(); - Ok(self.backend.get(key.as_bytes()).await?.is_some()) + async fn register_catalog(&self, name: String) -> Result { + let key = CatalogKey { catalog_name: name }.to_string(); + // TODO(hl): use compare_and_swap to prevent concurrent update + self.backend + .set( + key.as_bytes(), + &CatalogValue {} + .as_bytes() + .context(InvalidCatalogValueSnafu)?, + ) + .await?; + increment_gauge!(crate::metrics::METRIC_CATALOG_MANAGER_CATALOG_COUNT, 1.0); + Ok(false) + } + + fn as_any(&self) -> &dyn Any { + self } } diff --git a/src/catalog/src/schema.rs b/src/catalog/src/schema.rs deleted file mode 100644 index f9e6e2a6c22c..000000000000 --- a/src/catalog/src/schema.rs +++ /dev/null @@ -1,69 +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 std::sync::Arc; - -use async_trait::async_trait; -use table::TableRef; - -use crate::error::{NotSupportedSnafu, Result}; - -/// Represents a schema, comprising a number of named tables. -#[async_trait] -pub trait SchemaProvider: Sync + Send { - /// Returns the schema provider as [`Any`](std::any::Any) - /// so that it can be downcast to a specific implementation. - fn as_any(&self) -> &dyn Any; - - /// Retrieves the list of available table names in this schema. - async fn table_names(&self) -> Result>; - - /// Retrieves a specific table from the schema by name, provided it exists. - async fn table(&self, name: &str) -> Result>; - - /// If supported by the implementation, adds a new table to this schema. - /// If a table of the same name existed before, it returns "Table already exists" error. - async fn register_table(&self, name: String, _table: TableRef) -> Result> { - NotSupportedSnafu { - op: format!("register_table({name}, )"), - } - .fail() - } - - /// If supported by the implementation, renames an existing table from this schema and returns it. - /// If no table of that name exists, returns "Table not found" error. - async fn rename_table(&self, name: &str, new_name: String) -> Result { - NotSupportedSnafu { - op: format!("rename_table({name}, {new_name})"), - } - .fail() - } - - /// If supported by the implementation, removes an existing table from this schema and returns it. - /// If no table of that name exists, returns Ok(None). - async fn deregister_table(&self, name: &str) -> Result> { - NotSupportedSnafu { - op: format!("deregister_table({name})"), - } - .fail() - } - - /// If supported by the implementation, checks the table exist in the schema provider or not. - /// If no matched table in the schema provider, return false. - /// Otherwise, return true. - async fn table_exist(&self, name: &str) -> Result; -} - -pub type SchemaProviderRef = Arc; diff --git a/src/catalog/src/table_source.rs b/src/catalog/src/table_source.rs index 72a0c8fe91c7..d5b3423c8126 100644 --- a/src/catalog/src/table_source.rs +++ b/src/catalog/src/table_source.rs @@ -24,10 +24,7 @@ use session::context::QueryContext; use snafu::{ensure, OptionExt}; use table::table::adapter::DfTableProviderAdapter; -use crate::error::{ - CatalogNotFoundSnafu, QueryAccessDeniedSnafu, Result, SchemaNotFoundSnafu, TableNotExistSnafu, -}; -use crate::information_schema::InformationSchemaProvider; +use crate::error::{QueryAccessDeniedSnafu, Result, TableNotExistSnafu}; use crate::CatalogManagerRef; pub struct DfTableSourceProvider { @@ -104,41 +101,18 @@ impl DfTableSourceProvider { let schema_name = table_ref.schema.as_ref(); let table_name = table_ref.table.as_ref(); - let schema = if schema_name != INFORMATION_SCHEMA_NAME { - let catalog = self - .catalog_manager - .catalog(catalog_name) - .await? - .context(CatalogNotFoundSnafu { catalog_name })?; - catalog - .schema(schema_name) - .await? - .context(SchemaNotFoundSnafu { - catalog: catalog_name, - schema: schema_name, - })? - } else { - let catalog_provider = self - .catalog_manager - .catalog(catalog_name) - .await? - .context(CatalogNotFoundSnafu { catalog_name })?; - Arc::new(InformationSchemaProvider::new( - catalog_name.to_string(), - catalog_provider, - )) - }; - let table = schema - .table(table_name) + let table = self + .catalog_manager + .table(catalog_name, schema_name, table_name) .await? .with_context(|| TableNotExistSnafu { table: format_full_table_name(catalog_name, schema_name, table_name), })?; - let table = DfTableProviderAdapter::new(table); - let table = provider_as_source(Arc::new(table)); - self.resolved_tables.insert(resolved_name, table.clone()); - Ok(table) + let provider = DfTableProviderAdapter::new(table); + let source = provider_as_source(Arc::new(provider)); + self.resolved_tables.insert(resolved_name, source.clone()); + Ok(source) } } diff --git a/src/catalog/src/tables.rs b/src/catalog/src/tables.rs index 175e0799a791..cd74c326d174 100644 --- a/src/catalog/src/tables.rs +++ b/src/catalog/src/tables.rs @@ -14,50 +14,24 @@ // The `tables` table in system catalog keeps a record of all tables created by user. -use std::any::Any; use std::sync::Arc; -use async_trait::async_trait; -use common_catalog::consts::{INFORMATION_SCHEMA_NAME, SYSTEM_CATALOG_TABLE_NAME}; use common_telemetry::logging; use snafu::ResultExt; use table::metadata::TableId; -use table::{Table, TableRef}; +use table::Table; -use crate::error::{self, Error, InsertCatalogRecordSnafu, Result as CatalogResult}; +use crate::error::{self, InsertCatalogRecordSnafu, Result as CatalogResult}; use crate::system::{ build_schema_insert_request, build_table_deletion_request, build_table_insert_request, SystemCatalogTable, }; -use crate::{CatalogProvider, DeregisterTableRequest, SchemaProvider, SchemaProviderRef}; +use crate::DeregisterTableRequest; pub struct InformationSchema { pub system: Arc, } -#[async_trait] -impl SchemaProvider for InformationSchema { - fn as_any(&self) -> &dyn Any { - self - } - - async fn table_names(&self) -> Result, Error> { - Ok(vec![SYSTEM_CATALOG_TABLE_NAME.to_string()]) - } - - async fn table(&self, name: &str) -> Result, Error> { - if name.eq_ignore_ascii_case(SYSTEM_CATALOG_TABLE_NAME) { - Ok(Some(self.system.clone())) - } else { - Ok(None) - } - } - - async fn table_exist(&self, name: &str) -> Result { - Ok(name.eq_ignore_ascii_case(SYSTEM_CATALOG_TABLE_NAME)) - } -} - pub struct SystemCatalog { pub information_schema: Arc, } @@ -125,30 +99,3 @@ impl SystemCatalog { .context(InsertCatalogRecordSnafu) } } - -#[async_trait::async_trait] -impl CatalogProvider for SystemCatalog { - fn as_any(&self) -> &dyn Any { - self - } - - async fn schema_names(&self) -> Result, Error> { - Ok(vec![INFORMATION_SCHEMA_NAME.to_string()]) - } - - async fn register_schema( - &self, - _name: String, - _schema: SchemaProviderRef, - ) -> Result, Error> { - panic!("System catalog does not support registering schema!") - } - - async fn schema(&self, name: &str) -> Result>, Error> { - if name.eq_ignore_ascii_case(INFORMATION_SCHEMA_NAME) { - Ok(Some(self.information_schema.clone())) - } else { - Ok(None) - } - } -} diff --git a/src/catalog/tests/remote_catalog_tests.rs b/src/catalog/tests/remote_catalog_tests.rs index 9d3f539f830e..983b18ebd7fd 100644 --- a/src/catalog/tests/remote_catalog_tests.rs +++ b/src/catalog/tests/remote_catalog_tests.rs @@ -24,11 +24,8 @@ mod tests { use catalog::helper::{CatalogKey, CatalogValue, SchemaKey, SchemaValue}; use catalog::remote::mock::{MockKvBackend, MockTableEngine}; use catalog::remote::region_alive_keeper::RegionAliveKeepers; - use catalog::remote::{ - CachedMetaKvBackend, KvBackend, KvBackendRef, RemoteCatalogManager, RemoteCatalogProvider, - RemoteSchemaProvider, - }; - use catalog::{CatalogManager, RegisterTableRequest}; + use catalog::remote::{CachedMetaKvBackend, KvBackend, KvBackendRef, RemoteCatalogManager}; + use catalog::{CatalogManager, RegisterSchemaRequest, RegisterTableRequest}; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, MITO_ENGINE}; use common_meta::ident::TableIdent; use datatypes::schema::RawSchema; @@ -40,6 +37,7 @@ mod tests { use tokio::time::Instant; struct TestingComponents { + #[allow(dead_code)] kv_backend: KvBackendRef, catalog_manager: Arc, table_engine_manager: TableEngineManagerRef, @@ -179,14 +177,12 @@ mod tests { catalog_manager.catalog_names().await.unwrap() ); - let default_catalog = catalog_manager - .catalog(DEFAULT_CATALOG_NAME) - .await - .unwrap() - .unwrap(); assert_eq!( vec![DEFAULT_SCHEMA_NAME.to_string()], - default_catalog.schema_names().await.unwrap() + catalog_manager + .schema_names(DEFAULT_CATALOG_NAME) + .await + .unwrap() ); } @@ -242,23 +238,15 @@ mod tests { async fn test_register_table() { let node_id = 42; let components = prepare_components(node_id).await; - let catalog_manager = &components.catalog_manager; - let default_catalog = catalog_manager - .catalog(DEFAULT_CATALOG_NAME) - .await - .unwrap() - .unwrap(); assert_eq!( vec![DEFAULT_SCHEMA_NAME.to_string()], - default_catalog.schema_names().await.unwrap() + components + .catalog_manager + .schema_names(DEFAULT_CATALOG_NAME) + .await + .unwrap() ); - let default_schema = default_catalog - .schema(DEFAULT_SCHEMA_NAME) - .await - .unwrap() - .unwrap(); - // register a new table with an nonexistent catalog let catalog_name = DEFAULT_CATALOG_NAME.to_string(); let schema_name = DEFAULT_SCHEMA_NAME.to_string(); @@ -293,10 +281,18 @@ mod tests { table_id, table, }; - assert!(catalog_manager.register_table(reg_req).await.unwrap()); + assert!(components + .catalog_manager + .register_table(reg_req) + .await + .unwrap()); assert_eq!( vec![table_name], - default_schema.table_names().await.unwrap() + components + .catalog_manager + .table_names(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME) + .await + .unwrap() ); } @@ -304,29 +300,28 @@ mod tests { async fn test_register_catalog_schema_table() { let node_id = 42; let components = prepare_components(node_id).await; - let backend = &components.kv_backend; - let catalog_manager = components.catalog_manager.clone(); - let engine_manager = components.table_engine_manager.clone(); let catalog_name = "test_catalog".to_string(); let schema_name = "nonexistent_schema".to_string(); - let catalog = Arc::new(RemoteCatalogProvider::new( - catalog_name.clone(), - backend.clone(), - engine_manager.clone(), - node_id, - components.region_alive_keepers.clone(), - )); // register catalog to catalog manager - CatalogManager::register_catalog(&*catalog_manager, catalog_name.clone(), catalog) + components + .catalog_manager + .register_catalog(catalog_name.clone()) .await .unwrap(); assert_eq!( HashSet::::from_iter( vec![DEFAULT_CATALOG_NAME.to_string(), catalog_name.clone()].into_iter() ), - HashSet::from_iter(catalog_manager.catalog_names().await.unwrap().into_iter()) + HashSet::from_iter( + components + .catalog_manager + .catalog_names() + .await + .unwrap() + .into_iter() + ) ); let table_to_register = components @@ -359,38 +354,34 @@ mod tests { }; // this register will fail since schema does not exist yet assert_matches!( - catalog_manager + components + .catalog_manager .register_table(reg_req.clone()) .await .unwrap_err(), catalog::error::Error::SchemaNotFound { .. } ); - let new_catalog = catalog_manager - .catalog(&catalog_name) + let register_schema_request = RegisterSchemaRequest { + catalog: catalog_name.to_string(), + schema: schema_name.to_string(), + }; + assert!(components + .catalog_manager + .register_schema(register_schema_request) .await - .unwrap() - .expect("catalog should exist since it's already registered"); - let schema = Arc::new(RemoteSchemaProvider::new( - catalog_name.clone(), - schema_name.clone(), - node_id, - engine_manager, - backend.clone(), - components.region_alive_keepers.clone(), - )); - - let prev = new_catalog - .register_schema(schema_name.clone(), schema.clone()) + .expect("Register schema should not fail")); + assert!(components + .catalog_manager + .register_table(reg_req) .await - .expect("Register schema should not fail"); - assert!(prev.is_none()); - assert!(catalog_manager.register_table(reg_req).await.unwrap()); + .unwrap()); assert_eq!( HashSet::from([schema_name.clone()]), - new_catalog - .schema_names() + components + .catalog_manager + .schema_names(&catalog_name) .await .unwrap() .into_iter() diff --git a/src/common/catalog/src/consts.rs b/src/common/catalog/src/consts.rs index 3130770c274b..555359f7b666 100644 --- a/src/common/catalog/src/consts.rs +++ b/src/common/catalog/src/consts.rs @@ -27,6 +27,8 @@ pub const MAX_SYS_TABLE_ID: u32 = MIN_USER_TABLE_ID - 1; pub const SYSTEM_CATALOG_TABLE_ID: u32 = 0; /// scripts table id pub const SCRIPTS_TABLE_ID: u32 = 1; +/// numbers table id +pub const NUMBERS_TABLE_ID: u32 = 2; pub const MITO_ENGINE: &str = "mito"; pub const IMMUTABLE_FILE_ENGINE: &str = "file"; diff --git a/src/datanode/src/heartbeat/handler/close_region.rs b/src/datanode/src/heartbeat/handler/close_region.rs index 69b9e3e66588..6ae4a2a6fd6a 100644 --- a/src/datanode/src/heartbeat/handler/close_region.rs +++ b/src/datanode/src/heartbeat/handler/close_region.rs @@ -189,15 +189,13 @@ impl CloseRegionHandler { })? { CloseTableResult::NotFound | CloseTableResult::Released(_) => { // Deregister table if The table released. - let deregistered = self.deregister_table(table_ref).await?; + self.deregister_table(table_ref).await?; - if deregistered { - self.region_alive_keepers - .deregister_table(table_ident) - .await; - } + self.region_alive_keepers + .deregister_table(table_ident) + .await; - Ok(deregistered) + Ok(true) } CloseTableResult::PartialClosed(regions) => { // Requires caller to update the region_numbers @@ -220,7 +218,7 @@ impl CloseRegionHandler { Ok(true) } - async fn deregister_table(&self, table_ref: &TableReference<'_>) -> Result { + async fn deregister_table(&self, table_ref: &TableReference<'_>) -> Result<()> { self.catalog_manager .deregister_table(DeregisterTableRequest { catalog: table_ref.catalog.to_string(), diff --git a/src/datanode/src/instance.rs b/src/datanode/src/instance.rs index 3cd1e57e6d2a..194cc047c7c5 100644 --- a/src/datanode/src/instance.rs +++ b/src/datanode/src/instance.rs @@ -238,6 +238,7 @@ impl Instance { } }; + catalog_manager.start().await.context(CatalogSnafu)?; let factory = QueryEngineFactory::new(catalog_manager.clone(), false); let query_engine = factory.query_engine(); @@ -315,15 +316,10 @@ impl Instance { } pub async fn flush_tables(&self) -> Result<()> { - info!("going to flush all schemas"); + info!("going to flush all schemas under {DEFAULT_CATALOG_NAME}"); let schema_list = self .catalog_manager - .catalog(DEFAULT_CATALOG_NAME) - .await - .map_err(BoxedError::new) - .context(ShutdownInstanceSnafu)? - .expect("Default schema not found") - .schema_names() + .schema_names(DEFAULT_CATALOG_NAME) .await .map_err(BoxedError::new) .context(ShutdownInstanceSnafu)?; diff --git a/src/datanode/src/instance/grpc.rs b/src/datanode/src/instance/grpc.rs index a727bf27c1b5..adc3dbc23b6d 100644 --- a/src/datanode/src/instance/grpc.rs +++ b/src/datanode/src/instance/grpc.rs @@ -42,9 +42,9 @@ use table::requests::CreateDatabaseRequest; use table::table::adapter::DfTableProviderAdapter; use crate::error::{ - self, CatalogNotFoundSnafu, CatalogSnafu, DecodeLogicalPlanSnafu, DeleteExprToRequestSnafu, - DeleteSnafu, ExecuteLogicalPlanSnafu, ExecuteSqlSnafu, InsertDataSnafu, InsertSnafu, - JoinTaskSnafu, PlanStatementSnafu, Result, SchemaNotFoundSnafu, TableNotFoundSnafu, + self, CatalogSnafu, DecodeLogicalPlanSnafu, DeleteExprToRequestSnafu, DeleteSnafu, + ExecuteLogicalPlanSnafu, ExecuteSqlSnafu, InsertDataSnafu, InsertSnafu, JoinTaskSnafu, + PlanStatementSnafu, Result, TableNotFoundSnafu, }; use crate::instance::Instance; @@ -231,19 +231,10 @@ impl DummySchemaProvider { schema_name: String, catalog_manager: CatalogManagerRef, ) -> Result { - let catalog = catalog_manager - .catalog(&catalog_name) + let table_names = catalog_manager + .table_names(&catalog_name, &schema_name) .await - .context(CatalogSnafu)? - .context(CatalogNotFoundSnafu { - name: &catalog_name, - })?; - let schema = catalog - .schema(&schema_name) - .await - .context(CatalogSnafu)? - .context(SchemaNotFoundSnafu { name: &schema_name })?; - let table_names = schema.table_names().await.context(CatalogSnafu)?; + .unwrap(); Ok(Self { catalog: catalog_name, schema: schema_name, diff --git a/src/datanode/src/sql/create.rs b/src/datanode/src/sql/create.rs index 682552f8eded..36b9a9aa429f 100644 --- a/src/datanode/src/sql/create.rs +++ b/src/datanode/src/sql/create.rs @@ -48,10 +48,9 @@ impl SqlHandler { let schema = req.db_name; if self .catalog_manager - .schema(&catalog, &schema) + .schema_exist(&catalog, &schema) .await .context(CatalogSnafu)? - .is_some() { return if req.create_if_not_exists { Ok(Output::AffectedRows(1)) diff --git a/src/datanode/src/sql/flush_table.rs b/src/datanode/src/sql/flush_table.rs index 71b7d632de36..3951caa944ab 100644 --- a/src/datanode/src/sql/flush_table.rs +++ b/src/datanode/src/sql/flush_table.rs @@ -12,33 +12,41 @@ // See the License for the specific language governing permissions and // limitations under the License. -use catalog::SchemaProviderRef; +use catalog::CatalogManagerRef; use common_query::Output; use snafu::{OptionExt, ResultExt}; use table::requests::FlushTableRequest; -use crate::error::{self, CatalogSnafu, DatabaseNotFoundSnafu, Result}; +use crate::error::{self, CatalogSnafu, Result}; use crate::sql::SqlHandler; impl SqlHandler { pub(crate) async fn flush_table(&self, req: FlushTableRequest) -> Result { - let schema = self - .catalog_manager - .schema(&req.catalog_name, &req.schema_name) - .await - .context(CatalogSnafu)? - .context(DatabaseNotFoundSnafu { - catalog: &req.catalog_name, - schema: &req.schema_name, - })?; - if let Some(table) = &req.table_name { - self.flush_table_inner(schema, table, req.region_number, req.wait) - .await?; + self.flush_table_inner( + &self.catalog_manager, + &req.catalog_name, + &req.schema_name, + table, + req.region_number, + req.wait, + ) + .await?; } else { - let all_table_names = schema.table_names().await.context(CatalogSnafu)?; + let all_table_names = self + .catalog_manager + .table_names(&req.catalog_name, &req.schema_name) + .await + .context(CatalogSnafu)?; futures::future::join_all(all_table_names.iter().map(|table| { - self.flush_table_inner(schema.clone(), table, req.region_number, req.wait) + self.flush_table_inner( + &self.catalog_manager, + &req.catalog_name, + &req.schema_name, + table, + req.region_number, + req.wait, + ) })) .await .into_iter() @@ -49,13 +57,15 @@ impl SqlHandler { async fn flush_table_inner( &self, - schema: SchemaProviderRef, + catalog_manager: &CatalogManagerRef, + catalog_name: &str, + schema_name: &str, table_name: &str, region: Option, wait: Option, ) -> Result<()> { - schema - .table(table_name) + catalog_manager + .table(catalog_name, schema_name, table_name) .await .context(error::FindTableSnafu { table_name })? .context(error::TableNotFoundSnafu { table_name })? diff --git a/src/datanode/src/tests/test_util.rs b/src/datanode/src/tests/test_util.rs index d59f9f5670ae..f82001432a1d 100644 --- a/src/datanode/src/tests/test_util.rs +++ b/src/datanode/src/tests/test_util.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use catalog::RegisterTableRequest; use common_catalog::consts::{ DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, MIN_USER_TABLE_ID, MITO_ENGINE, }; @@ -119,15 +120,13 @@ pub(crate) async fn create_test_table( .await .context(CreateTableSnafu { table_name })?; - let schema_provider = instance - .catalog_manager - .schema(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME) - .await - .unwrap() - .unwrap(); - schema_provider - .register_table(table_name.to_string(), table.clone()) - .await - .unwrap(); + let req = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: table_name.to_string(), + table_id: table.table_info().ident.table_id, + table: table.clone(), + }; + instance.catalog_manager.register_table(req).await.unwrap(); Ok(table) } diff --git a/src/frontend/src/catalog.rs b/src/frontend/src/catalog.rs index d91fd6eabb1f..05d6b2e8281a 100644 --- a/src/frontend/src/catalog.rs +++ b/src/frontend/src/catalog.rs @@ -17,7 +17,6 @@ use std::collections::HashSet; use std::sync::Arc; use api::v1::CreateTableExpr; -use async_trait::async_trait; use catalog::error::{ self as catalog_err, InternalSnafu, InvalidCatalogValueSnafu, InvalidSystemTableDefSnafu, Result as CatalogResult, UnimplementedSnafu, @@ -26,14 +25,14 @@ use catalog::helper::{ build_catalog_prefix, build_schema_prefix, build_table_global_prefix, CatalogKey, SchemaKey, TableGlobalKey, TableGlobalValue, }; +use catalog::information_schema::InformationSchemaProvider; use catalog::remote::{Kv, KvBackendRef, KvCacheInvalidatorRef}; use catalog::{ - CatalogManager, CatalogProvider, CatalogProviderRef, DeregisterTableRequest, - RegisterSchemaRequest, RegisterSystemTableRequest, RegisterTableRequest, RenameTableRequest, - SchemaProvider, SchemaProviderRef, + CatalogManager, DeregisterTableRequest, RegisterSchemaRequest, RegisterSystemTableRequest, + RegisterTableRequest, RenameTableRequest, }; use client::client_manager::DatanodeClients; -use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; +use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, INFORMATION_SCHEMA_NAME}; use common_error::prelude::BoxedError; use common_meta::table_name::TableName; use common_telemetry::warn; @@ -129,12 +128,8 @@ impl CatalogManager for FrontendCatalogManager { Ok(()) } - async fn register_catalog( - &self, - _name: String, - _catalog: CatalogProviderRef, - ) -> CatalogResult> { - unimplemented!("Frontend catalog list does not support register catalog") + async fn register_catalog(&self, _name: String) -> CatalogResult { + unimplemented!("FrontendCatalogManager does not support register catalog") } // TODO(LFC): Handle the table caching in (de)register_table. @@ -142,20 +137,20 @@ impl CatalogManager for FrontendCatalogManager { Ok(true) } - async fn deregister_table(&self, request: DeregisterTableRequest) -> CatalogResult { + async fn deregister_table(&self, request: DeregisterTableRequest) -> CatalogResult<()> { let table_name = TableName::new(request.catalog, request.schema, request.table_name); self.partition_manager .table_routes() .invalidate_table_route(&table_name) .await; - Ok(true) + Ok(()) } async fn register_schema( &self, _request: RegisterSchemaRequest, ) -> catalog::error::Result { - unimplemented!() + unimplemented!("FrontendCatalogManager does not support register schema") } async fn rename_table(&self, _request: RenameTableRequest) -> catalog_err::Result { @@ -270,67 +265,9 @@ impl CatalogManager for FrontendCatalogManager { Ok(res.into_iter().collect()) } - async fn catalog(&self, catalog: &str) -> CatalogResult> { - let key = CatalogKey { - catalog_name: catalog.to_string(), - } - .to_string(); - - Ok(self.backend.get(key.as_bytes()).await?.map(|_| { - Arc::new(FrontendCatalogProvider { - catalog_name: catalog.to_string(), - catalog_manager: Arc::new(self.clone()), - }) as Arc<_> - })) - } - - async fn schema( - &self, - catalog: &str, - schema: &str, - ) -> catalog::error::Result> { - self.catalog(catalog) - .await? - .context(catalog::error::CatalogNotFoundSnafu { - catalog_name: catalog, - })? - .schema(schema) - .await - } - - async fn table( - &self, - catalog: &str, - schema: &str, - table_name: &str, - ) -> catalog::error::Result> { - self.schema(catalog, schema) - .await? - .context(catalog::error::SchemaNotFoundSnafu { catalog, schema })? - .table(table_name) - .await - } - - fn as_any(&self) -> &dyn Any { - self - } -} - -pub struct FrontendCatalogProvider { - catalog_name: String, - catalog_manager: Arc, -} - -#[async_trait::async_trait] -impl CatalogProvider for FrontendCatalogProvider { - fn as_any(&self) -> &dyn Any { - self - } - - async fn schema_names(&self) -> catalog::error::Result> { - let key = build_schema_prefix(&self.catalog_name); - let backend = self.catalog_manager.backend(); - let mut iter = backend.range(key.as_bytes()); + async fn schema_names(&self, catalog: &str) -> CatalogResult> { + let key = build_schema_prefix(catalog); + let mut iter = self.backend.range(key.as_bytes()); let mut res = HashSet::new(); while let Some(r) = iter.next().await { let Kv(k, _) = r?; @@ -341,61 +278,13 @@ impl CatalogProvider for FrontendCatalogProvider { Ok(res.into_iter().collect()) } - async fn register_schema( - &self, - _name: String, - _schema: SchemaProviderRef, - ) -> catalog::error::Result> { - unimplemented!("Frontend catalog provider does not support register schema") - } - - async fn schema(&self, name: &str) -> catalog::error::Result> { - let catalog = &self.catalog_name; - - let schema_key = SchemaKey { - catalog_name: catalog.clone(), - schema_name: name.to_string(), - } - .to_string(); - - let val = self - .catalog_manager - .backend() - .get(schema_key.as_bytes()) - .await?; - - let provider = val.map(|_| { - Arc::new(FrontendSchemaProvider { - catalog_name: catalog.clone(), - schema_name: name.to_string(), - catalog_manager: self.catalog_manager.clone(), - }) as Arc - }); - - Ok(provider) - } -} - -pub struct FrontendSchemaProvider { - catalog_name: String, - schema_name: String, - catalog_manager: Arc, -} - -#[async_trait] -impl SchemaProvider for FrontendSchemaProvider { - fn as_any(&self) -> &dyn Any { - self - } - - async fn table_names(&self) -> catalog::error::Result> { + async fn table_names(&self, catalog: &str, schema: &str) -> CatalogResult> { let mut tables = vec![]; - if self.catalog_name == DEFAULT_CATALOG_NAME && self.schema_name == DEFAULT_SCHEMA_NAME { + if catalog == DEFAULT_CATALOG_NAME && schema == DEFAULT_SCHEMA_NAME { tables.push("numbers".to_string()); } - let key = build_table_global_prefix(&self.catalog_name, &self.schema_name); - let backend = self.catalog_manager.backend(); - let iter = backend.range(key.as_bytes()); + let key = build_table_global_prefix(catalog, schema); + let iter = self.backend.range(key.as_bytes()); let result = iter .map(|r| { let Kv(k, _) = r?; @@ -409,20 +298,71 @@ impl SchemaProvider for FrontendSchemaProvider { Ok(tables) } - async fn table(&self, name: &str) -> catalog::error::Result> { - if self.catalog_name == DEFAULT_CATALOG_NAME - && self.schema_name == DEFAULT_SCHEMA_NAME - && name == "numbers" + async fn catalog_exist(&self, catalog: &str) -> CatalogResult { + let key = CatalogKey { + catalog_name: catalog.to_string(), + } + .to_string(); + + Ok(self.backend.get(key.as_bytes()).await?.is_some()) + } + + async fn schema_exist(&self, catalog: &str, schema: &str) -> CatalogResult { + let schema_key = SchemaKey { + catalog_name: catalog.to_string(), + schema_name: schema.to_string(), + } + .to_string(); + + Ok(self.backend().get(schema_key.as_bytes()).await?.is_some()) + } + + async fn table_exist(&self, catalog: &str, schema: &str, table: &str) -> CatalogResult { + let table_global_key = TableGlobalKey { + catalog_name: catalog.to_string(), + schema_name: schema.to_string(), + table_name: table.to_string(), + }; + Ok(self + .backend() + .get(table_global_key.to_string().as_bytes()) + .await? + .is_some()) + } + + async fn table( + &self, + catalog: &str, + schema: &str, + table_name: &str, + ) -> CatalogResult> { + if catalog == DEFAULT_CATALOG_NAME + && schema == DEFAULT_SCHEMA_NAME + && table_name == "numbers" { return Ok(Some(Arc::new(NumbersTable::default()))); } + if schema == INFORMATION_SCHEMA_NAME { + // hack: use existing cyclin reference to get Arc. + // This can be remove by refactoring the struct into something like Arc + let manager = if let Some(instance) = self.dist_instance.as_ref() { + instance.catalog_manager() as _ + } else { + return Ok(None); + }; + + let provider = + InformationSchemaProvider::new(catalog.to_string(), Arc::downgrade(&manager)); + return provider.table(table_name); + } + let table_global_key = TableGlobalKey { - catalog_name: self.catalog_name.clone(), - schema_name: self.schema_name.clone(), - table_name: name.to_string(), + catalog_name: catalog.to_string(), + schema_name: schema.to_string(), + table_name: table_name.to_string(), }; - let Some(kv) = self.catalog_manager.backend().get(table_global_key.to_string().as_bytes()).await? else { + let Some(kv) = self.backend().get(table_global_key.to_string().as_bytes()).await? else { return Ok(None); }; let v = TableGlobalValue::from_bytes(kv.1).context(InvalidCatalogValueSnafu)?; @@ -432,14 +372,14 @@ impl SchemaProvider for FrontendSchemaProvider { .context(catalog_err::InvalidTableInfoInCatalogSnafu)?, ); let table = Arc::new(DistTable::new( - TableName::new(&self.catalog_name, &self.schema_name, name), + TableName::new(catalog, schema, table_name), table_info, - self.catalog_manager.clone(), + Arc::new(self.clone()), )); Ok(Some(table)) } - async fn table_exist(&self, name: &str) -> catalog::error::Result { - Ok(self.table_names().await?.contains(&name.to_string())) + fn as_any(&self) -> &dyn Any { + self } } diff --git a/src/frontend/src/instance.rs b/src/frontend/src/instance.rs index 0f116c50d21b..ed70a2467190 100644 --- a/src/frontend/src/instance.rs +++ b/src/frontend/src/instance.rs @@ -557,9 +557,8 @@ impl SqlQueryHandler for Instance { async fn is_valid_schema(&self, catalog: &str, schema: &str) -> Result { self.catalog_manager - .schema(catalog, schema) + .schema_exist(catalog, schema) .await - .map(|s| s.is_some()) .context(error::CatalogSnafu) } } diff --git a/src/frontend/src/instance/distributed.rs b/src/frontend/src/instance/distributed.rs index cbe26a264265..c5ff15ea3db1 100644 --- a/src/frontend/src/instance/distributed.rs +++ b/src/frontend/src/instance/distributed.rs @@ -261,15 +261,10 @@ impl DistInstance { schema: table_name.schema_name.clone(), table_name: table_name.table_name.clone(), }; - ensure!( - self.catalog_manager - .deregister_table(request) - .await - .context(CatalogSnafu)?, - error::TableNotFoundSnafu { - table_name: table_name.to_string() - } - ); + self.catalog_manager + .deregister_table(request) + .await + .context(CatalogSnafu)?; let expr = DropTableExpr { catalog_name: table_name.catalog_name.clone(), @@ -467,10 +462,9 @@ impl DistInstance { let catalog = query_ctx.current_catalog(); if self .catalog_manager - .schema(&catalog, &expr.database_name) + .schema_exist(&catalog, &expr.database_name) .await .context(CatalogSnafu)? - .is_some() { return if expr.create_if_not_exists { Ok(Output::AffectedRows(1)) diff --git a/src/frontend/src/statement.rs b/src/frontend/src/statement.rs index 4030452fc972..46b0d39f59ce 100644 --- a/src/frontend/src/statement.rs +++ b/src/frontend/src/statement.rs @@ -96,7 +96,7 @@ impl StatementExecutor { Statement::Use(db) => self.handle_use(db, query_ctx).await, - Statement::ShowDatabases(stmt) => self.show_databases(stmt).await, + Statement::ShowDatabases(stmt) => self.show_databases(stmt, query_ctx).await, Statement::ShowTables(stmt) => self.show_tables(stmt, query_ctx).await, @@ -147,10 +147,9 @@ impl StatementExecutor { let catalog = &query_ctx.current_catalog(); ensure!( self.catalog_manager - .schema(catalog, &db) + .schema_exist(catalog, &db) .await - .context(CatalogSnafu)? - .is_some(), + .context(CatalogSnafu)?, SchemaNotFoundSnafu { schema_info: &db } ); diff --git a/src/frontend/src/statement/backup.rs b/src/frontend/src/statement/backup.rs index bdd99f73c309..b0004ad5f2bb 100644 --- a/src/frontend/src/statement/backup.rs +++ b/src/frontend/src/statement/backup.rs @@ -15,13 +15,11 @@ use common_datasource::file_format::Format; use common_query::Output; use common_telemetry::info; -use snafu::{ensure, OptionExt, ResultExt}; +use snafu::{ensure, ResultExt}; use table::requests::{CopyDatabaseRequest, CopyDirection, CopyTableRequest}; use crate::error; -use crate::error::{ - CatalogNotFoundSnafu, CatalogSnafu, InvalidCopyParameterSnafu, SchemaNotFoundSnafu, -}; +use crate::error::{CatalogSnafu, InvalidCopyParameterSnafu}; use crate::statement::StatementExecutor; pub(crate) const COPY_DATABASE_TIME_START_KEY: &str = "start_time"; @@ -42,27 +40,16 @@ impl StatementExecutor { "Copy database {}.{}, dir: {},. time: {:?}", req.catalog_name, req.schema_name, req.location, req.time_range ); - let schema = self + let table_names = self .catalog_manager - .catalog(&req.catalog_name) + .table_names(&req.catalog_name, &req.schema_name) .await - .context(CatalogSnafu)? - .context(CatalogNotFoundSnafu { - catalog_name: &req.catalog_name, - })? - .schema(&req.schema_name) - .await - .context(CatalogSnafu)? - .context(SchemaNotFoundSnafu { - schema_info: &req.schema_name, - })?; + .context(CatalogSnafu)?; let suffix = Format::try_from(&req.with) .context(error::ParseFileFormatSnafu)? .suffix(); - let table_names = schema.table_names().await.context(CatalogSnafu)?; - let mut exported_rows = 0; for table_name in table_names { // TODO(hl): remove this hardcode once we've removed numbers table. diff --git a/src/frontend/src/statement/show.rs b/src/frontend/src/statement/show.rs index 0c00a0cdae92..37df294a2ae3 100644 --- a/src/frontend/src/statement/show.rs +++ b/src/frontend/src/statement/show.rs @@ -21,8 +21,12 @@ use crate::error::{ExecuteStatementSnafu, Result}; use crate::statement::StatementExecutor; impl StatementExecutor { - pub(super) async fn show_databases(&self, stmt: ShowDatabases) -> Result { - query::sql::show_databases(stmt, self.catalog_manager.clone()) + pub(super) async fn show_databases( + &self, + stmt: ShowDatabases, + query_ctx: QueryContextRef, + ) -> Result { + query::sql::show_databases(stmt, self.catalog_manager.clone(), query_ctx) .await .context(ExecuteStatementSnafu) } diff --git a/src/query/src/datafusion.rs b/src/query/src/datafusion.rs index 4931ac58ebf7..10066d411e46 100644 --- a/src/query/src/datafusion.rs +++ b/src/query/src/datafusion.rs @@ -46,9 +46,8 @@ use table::TableRef; use crate::dataframe::DataFrame; pub use crate::datafusion::planner::DfContextProviderAdapter; use crate::error::{ - CatalogNotFoundSnafu, CatalogSnafu, CreateRecordBatchSnafu, DataFusionSnafu, - MissingTimestampColumnSnafu, QueryExecutionSnafu, Result, SchemaNotFoundSnafu, - TableNotFoundSnafu, UnsupportedExprSnafu, + CatalogSnafu, CreateRecordBatchSnafu, DataFusionSnafu, MissingTimestampColumnSnafu, + QueryExecutionSnafu, Result, TableNotFoundSnafu, UnsupportedExprSnafu, }; use crate::executor::QueryExecutor; use crate::logical_optimizer::LogicalOptimizer; @@ -181,28 +180,12 @@ impl DatafusionQueryEngine { let schema_name = table_name.schema.as_ref(); let table_name = table_name.table.as_ref(); - let catalog = self - .state + self.state .catalog_manager() - .catalog(catalog_name) + .table(catalog_name, schema_name, table_name) .await .context(CatalogSnafu)? - .context(CatalogNotFoundSnafu { - catalog: catalog_name, - })?; - let schema = catalog - .schema(schema_name) - .await - .context(CatalogSnafu)? - .context(SchemaNotFoundSnafu { - schema: schema_name, - })?; - let table = schema - .table(table_name) - .await - .context(CatalogSnafu)? - .context(TableNotFoundSnafu { table: table_name })?; - Ok(table) + .with_context(|| TableNotFoundSnafu { table: table_name }) } } @@ -395,9 +378,8 @@ mod tests { use std::borrow::Cow::Borrowed; use std::sync::Arc; - use catalog::local::{MemoryCatalogProvider, MemorySchemaProvider}; - use catalog::{CatalogProvider, SchemaProvider}; - use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; + use catalog::{CatalogManager, RegisterTableRequest}; + use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, NUMBERS_TABLE_ID}; use common_query::Output; use common_recordbatch::util; use datafusion::prelude::{col, lit}; @@ -405,30 +387,24 @@ mod tests { use datatypes::schema::ColumnSchema; use datatypes::vectors::{Helper, UInt32Vector, UInt64Vector, VectorRef}; use session::context::QueryContext; - use table::table::numbers::NumbersTable; + use table::table::numbers::{NumbersTable, NUMBERS_TABLE_NAME}; use super::*; use crate::parser::QueryLanguageParser; use crate::query_engine::{QueryEngineFactory, QueryEngineRef}; async fn create_test_engine() -> QueryEngineRef { - let catalog_list = catalog::local::new_memory_catalog_list().unwrap(); - - let default_schema = Arc::new(MemorySchemaProvider::new()); - default_schema - .register_table("numbers".to_string(), Arc::new(NumbersTable::default())) - .await - .unwrap(); - let default_catalog = Arc::new(MemoryCatalogProvider::new()); - default_catalog - .register_schema(DEFAULT_SCHEMA_NAME.to_string(), default_schema) - .await - .unwrap(); - catalog_list - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), default_catalog) - .unwrap(); + let catalog_manager = catalog::local::new_memory_catalog_manager().unwrap(); + let req = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: NUMBERS_TABLE_NAME.to_string(), + table_id: NUMBERS_TABLE_ID, + table: Arc::new(NumbersTable::default()), + }; + catalog_manager.register_table(req).await.unwrap(); - QueryEngineFactory::new(catalog_list, false).query_engine() + QueryEngineFactory::new(catalog_manager, false).query_engine() } #[tokio::test] diff --git a/src/query/src/query_engine.rs b/src/query/src/query_engine.rs index dbab74cf2bb9..ed7f75e700a6 100644 --- a/src/query/src/query_engine.rs +++ b/src/query/src/query_engine.rs @@ -138,7 +138,7 @@ mod tests { #[test] fn test_query_engine_factory() { - let catalog_list = catalog::local::new_memory_catalog_list().unwrap(); + let catalog_list = catalog::local::new_memory_catalog_manager().unwrap(); let factory = QueryEngineFactory::new(catalog_list, false); let engine = factory.query_engine(); diff --git a/src/query/src/sql.rs b/src/query/src/sql.rs index 0757938952ac..fb39f3e65a1e 100644 --- a/src/query/src/sql.rs +++ b/src/query/src/sql.rs @@ -19,7 +19,7 @@ use std::sync::Arc; use catalog::CatalogManagerRef; use common_catalog::consts::{ - DEFAULT_CATALOG_NAME, SEMANTIC_TYPE_FIELD, SEMANTIC_TYPE_PRIMARY_KEY, SEMANTIC_TYPE_TIME_INDEX, + SEMANTIC_TYPE_FIELD, SEMANTIC_TYPE_PRIMARY_KEY, SEMANTIC_TYPE_TIME_INDEX, }; use common_datasource::file_format::{infer_schemas, FileFormat, Format}; use common_datasource::lister::{Lister, Source}; @@ -95,6 +95,7 @@ static SHOW_CREATE_TABLE_OUTPUT_SCHEMA: Lazy> = Lazy::new(|| { pub async fn show_databases( stmt: ShowDatabases, catalog_manager: CatalogManagerRef, + query_ctx: QueryContextRef, ) -> Result { ensure!( matches!(stmt.kind, ShowKind::All | ShowKind::Like(_)), @@ -103,14 +104,11 @@ pub async fn show_databases( } ); - let catalog = catalog_manager - .catalog(DEFAULT_CATALOG_NAME) + let mut databases = catalog_manager + .schema_names(&query_ctx.current_catalog()) .await - .context(error::CatalogSnafu)? - .context(error::CatalogNotFoundSnafu { - catalog: DEFAULT_CATALOG_NAME, - })?; - let mut databases = catalog.schema_names().await.context(error::CatalogSnafu)?; + .context(error::CatalogSnafu)?; + // TODO(dennis): Specify the order of the results in catalog manager API databases.sort(); @@ -148,12 +146,11 @@ pub async fn show_tables( query_ctx.current_schema() }; // TODO(sunng87): move this function into query_ctx - let schema = catalog_manager - .schema(&query_ctx.current_catalog(), &schema) + let mut tables = catalog_manager + .table_names(&query_ctx.current_catalog(), &schema) .await - .context(error::CatalogSnafu)? - .context(error::SchemaNotFoundSnafu { schema })?; - let mut tables = schema.table_names().await.context(error::CatalogSnafu)?; + .context(error::CatalogSnafu)?; + // TODO(dennis): Specify the order of the results in schema provider API tables.sort(); diff --git a/src/query/src/tests.rs b/src/query/src/tests.rs index d633ac9d4c8d..09d92e6d01b2 100644 --- a/src/query/src/tests.rs +++ b/src/query/src/tests.rs @@ -12,12 +12,16 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::sync::Arc; + +use catalog::local::MemoryCatalogManager; use common_query::Output; use common_recordbatch::{util, RecordBatch}; use session::context::QueryContext; +use table::test_util::MemTable; use crate::parser::QueryLanguageParser; -use crate::QueryEngineRef; +use crate::{QueryEngineFactory, QueryEngineRef}; mod argmax_test; mod argmin_test; @@ -46,3 +50,10 @@ async fn exec_selection(engine: QueryEngineRef, sql: &str) -> Vec { .unwrap() else { unreachable!() }; util::collect(stream).await.unwrap() } + +pub fn new_query_engine_with_table(table: MemTable) -> QueryEngineRef { + let table = Arc::new(table); + let catalog_manager = Arc::new(MemoryCatalogManager::new_with_table(table)); + + QueryEngineFactory::new(catalog_manager, false).query_engine() +} diff --git a/src/query/src/tests/function.rs b/src/query/src/tests/function.rs index 767fb5fbd40f..99b2a490bf37 100644 --- a/src/query/src/tests/function.rs +++ b/src/query/src/tests/function.rs @@ -14,8 +14,6 @@ use std::sync::Arc; -use catalog::local::{MemoryCatalogManager, MemoryCatalogProvider, MemorySchemaProvider}; -use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use common_recordbatch::RecordBatch; use datatypes::for_all_primitive_types; use datatypes::prelude::*; @@ -25,14 +23,10 @@ use datatypes::vectors::Helper; use rand::Rng; use table::test_util::MemTable; -use crate::tests::exec_selection; -use crate::{QueryEngine, QueryEngineFactory}; - -pub fn create_query_engine() -> Arc { - let schema_provider = Arc::new(MemorySchemaProvider::new()); - let catalog_provider = Arc::new(MemoryCatalogProvider::new()); - let catalog_list = Arc::new(MemoryCatalogManager::default()); +use crate::tests::{exec_selection, new_query_engine_with_table}; +use crate::{QueryEngine, QueryEngineRef}; +pub fn create_query_engine() -> QueryEngineRef { let mut column_schemas = vec![]; let mut columns = vec![]; macro_rules! create_number_table { @@ -54,19 +48,8 @@ pub fn create_query_engine() -> Arc { let schema = Arc::new(Schema::new(column_schemas.clone())); let recordbatch = RecordBatch::new(schema, columns).unwrap(); - let number_table = Arc::new(MemTable::new("numbers", recordbatch)); - schema_provider - .register_table_sync(number_table.table_name().to_string(), number_table) - .unwrap(); - - catalog_provider - .register_schema_sync(DEFAULT_SCHEMA_NAME.to_string(), schema_provider) - .unwrap(); - catalog_list - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), catalog_provider) - .unwrap(); - - QueryEngineFactory::new(catalog_list, false).query_engine() + let number_table = MemTable::new("numbers", recordbatch); + new_query_engine_with_table(number_table) } pub async fn get_numbers_from_table<'s, T>( diff --git a/src/query/src/tests/my_sum_udaf_example.rs b/src/query/src/tests/my_sum_udaf_example.rs index 2ae4c6cea6fc..6058bd0090c3 100644 --- a/src/query/src/tests/my_sum_udaf_example.rs +++ b/src/query/src/tests/my_sum_udaf_example.rs @@ -16,8 +16,6 @@ use std::fmt::Debug; use std::marker::PhantomData; use std::sync::Arc; -use catalog::local::{MemoryCatalogManager, MemoryCatalogProvider, MemorySchemaProvider}; -use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use common_function::scalars::aggregate::AggregateFunctionMeta; use common_function_macro::{as_aggr_func_creator, AggrFuncTypeStore}; use common_query::error::{CreateAccumulatorSnafu, Result as QueryResult}; @@ -33,8 +31,7 @@ use num_traits::AsPrimitive; use table::test_util::MemTable; use crate::error::Result; -use crate::tests::exec_selection; -use crate::QueryEngineFactory; +use crate::tests::{exec_selection, new_query_engine_with_table}; #[derive(Debug, Default)] struct MySumAccumulator { @@ -207,8 +204,7 @@ where let recordbatch = RecordBatch::new(schema, vec![column]).unwrap(); let testing_table = MemTable::new(&table_name, recordbatch); - let factory = new_query_engine_factory(testing_table); - let engine = factory.query_engine(); + let engine = new_query_engine_with_table(testing_table); engine.register_aggregate_function(Arc::new(AggregateFunctionMeta::new( "my_sum", @@ -224,24 +220,3 @@ where assert_eq!(expected, pretty_print); Ok(()) } - -fn new_query_engine_factory(table: MemTable) -> QueryEngineFactory { - let table_name = table.table_name().to_string(); - let table = Arc::new(table); - - let schema_provider = Arc::new(MemorySchemaProvider::new()); - let catalog_provider = Arc::new(MemoryCatalogProvider::new()); - let catalog_list = Arc::new(MemoryCatalogManager::default()); - - schema_provider - .register_table_sync(table_name, table) - .unwrap(); - catalog_provider - .register_schema_sync(DEFAULT_SCHEMA_NAME.to_string(), schema_provider) - .unwrap(); - catalog_list - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), catalog_provider) - .unwrap(); - - QueryEngineFactory::new(catalog_list, false) -} diff --git a/src/query/src/tests/percentile_test.rs b/src/query/src/tests/percentile_test.rs index 7fa9e6e903e5..ce278aa986f3 100644 --- a/src/query/src/tests/percentile_test.rs +++ b/src/query/src/tests/percentile_test.rs @@ -14,8 +14,6 @@ use std::sync::Arc; -use catalog::local::{MemoryCatalogManager, MemoryCatalogProvider, MemorySchemaProvider}; -use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use common_recordbatch::RecordBatch; use datatypes::for_all_primitive_types; use datatypes::prelude::*; @@ -25,9 +23,10 @@ use function::{create_query_engine, get_numbers_from_table}; use num_traits::AsPrimitive; use table::test_util::MemTable; +use super::new_query_engine_with_table; use crate::error::Result; use crate::tests::{exec_selection, function}; -use crate::{QueryEngine, QueryEngineFactory}; +use crate::QueryEngine; #[tokio::test] async fn test_percentile_aggregator() -> Result<()> { @@ -80,9 +79,6 @@ where fn create_correctness_engine() -> Arc { // create engine - let schema_provider = Arc::new(MemorySchemaProvider::new()); - let catalog_provider = Arc::new(MemoryCatalogProvider::new()); - let catalog_list = Arc::new(MemoryCatalogManager::default()); let mut column_schemas = vec![]; let mut columns = vec![]; @@ -96,20 +92,6 @@ fn create_correctness_engine() -> Arc { columns.push(column); let schema = Arc::new(Schema::new(column_schemas)); - let number_table = Arc::new(MemTable::new( - "corr_numbers", - RecordBatch::new(schema, columns).unwrap(), - )); - schema_provider - .register_table_sync(number_table.table_name().to_string(), number_table) - .unwrap(); - - catalog_provider - .register_schema_sync(DEFAULT_SCHEMA_NAME.to_string(), schema_provider) - .unwrap(); - catalog_list - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), catalog_provider) - .unwrap(); - - QueryEngineFactory::new(catalog_list, false).query_engine() + let number_table = MemTable::new("corr_numbers", RecordBatch::new(schema, columns).unwrap()); + new_query_engine_with_table(number_table) } diff --git a/src/query/src/tests/query_engine_test.rs b/src/query/src/tests/query_engine_test.rs index 9b23e2c80066..673c439f1bc0 100644 --- a/src/query/src/tests/query_engine_test.rs +++ b/src/query/src/tests/query_engine_test.rs @@ -14,9 +14,10 @@ use std::sync::Arc; -use catalog::local::{MemoryCatalogManager, MemoryCatalogProvider, MemorySchemaProvider}; +use catalog::local::MemoryCatalogManager; +use catalog::RegisterTableRequest; use common_base::Plugins; -use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; +use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, NUMBERS_TABLE_ID}; use common_error::prelude::BoxedError; use common_query::prelude::{create_udf, make_scalar_function, Volatility}; use common_query::Output; @@ -29,7 +30,7 @@ use datatypes::vectors::UInt32Vector; use session::context::QueryContext; use snafu::ResultExt; use table::table::adapter::DfTableProviderAdapter; -use table::table::numbers::NumbersTable; +use table::table::numbers::{NumbersTable, NUMBERS_TABLE_NAME}; use table::test_util::MemTable; use crate::error::{QueryExecutionSnafu, Result}; @@ -43,7 +44,7 @@ use crate::tests::pow::pow; #[tokio::test] async fn test_datafusion_query_engine() -> Result<()> { common_telemetry::init_default_ut_logging(); - let catalog_list = catalog::local::new_memory_catalog_list() + let catalog_list = catalog::local::new_memory_catalog_manager() .map_err(BoxedError::new) .context(QueryExecutionSnafu)?; let factory = QueryEngineFactory::new(catalog_list, false); @@ -102,29 +103,24 @@ async fn test_datafusion_query_engine() -> Result<()> { Ok(()) } -fn catalog_list() -> Result> { - let catalog_list = catalog::local::new_memory_catalog_list() - .map_err(BoxedError::new) - .context(QueryExecutionSnafu)?; +fn catalog_manager() -> Result> { + let catalog_manager = catalog::local::new_memory_catalog_manager().unwrap(); + let req = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: NUMBERS_TABLE_NAME.to_string(), + table_id: NUMBERS_TABLE_ID, + table: Arc::new(NumbersTable::default()), + }; + catalog_manager.register_table_sync(req).unwrap(); - let default_schema = Arc::new(MemorySchemaProvider::new()); - default_schema - .register_table_sync("numbers".to_string(), Arc::new(NumbersTable::default())) - .unwrap(); - let default_catalog = Arc::new(MemoryCatalogProvider::new()); - default_catalog - .register_schema_sync(DEFAULT_SCHEMA_NAME.to_string(), default_schema) - .unwrap(); - catalog_list - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), default_catalog) - .unwrap(); - Ok(catalog_list) + Ok(catalog_manager) } #[tokio::test] async fn test_query_validate() -> Result<()> { common_telemetry::init_default_ut_logging(); - let catalog_list = catalog_list()?; + let catalog_list = catalog_manager()?; // set plugins let plugins = Plugins::new(); @@ -155,7 +151,7 @@ async fn test_query_validate() -> Result<()> { #[tokio::test] async fn test_udf() -> Result<()> { common_telemetry::init_default_ut_logging(); - let catalog_list = catalog_list()?; + let catalog_list = catalog_manager()?; let factory = QueryEngineFactory::new(catalog_list, false); let engine = factory.query_engine(); diff --git a/src/query/src/tests/time_range_filter_test.rs b/src/query/src/tests/time_range_filter_test.rs index 73431ed2484b..73fe78d748f1 100644 --- a/src/query/src/tests/time_range_filter_test.rs +++ b/src/query/src/tests/time_range_filter_test.rs @@ -15,7 +15,9 @@ use std::any::Any; use std::sync::Arc; -use catalog::local::{new_memory_catalog_list, MemoryCatalogProvider, MemorySchemaProvider}; +use catalog::local::new_memory_catalog_manager; +use catalog::RegisterTableRequest; +use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use common_query::physical_plan::PhysicalPlanRef; use common_query::prelude::Expr; use common_recordbatch::{RecordBatch, SendableRecordBatchStream}; @@ -114,21 +116,17 @@ fn create_test_engine() -> TimeRangeTester { filter: Default::default(), }); - let catalog_list = new_memory_catalog_list().unwrap(); - - let default_schema = Arc::new(MemorySchemaProvider::new()); - MemorySchemaProvider::register_table_sync(&default_schema, "m".to_string(), table.clone()) - .unwrap(); - - let default_catalog = Arc::new(MemoryCatalogProvider::new()); - default_catalog - .register_schema_sync("public".to_string(), default_schema) - .unwrap(); - catalog_list - .register_catalog_sync("greptime".to_string(), default_catalog) - .unwrap(); - - let engine = QueryEngineFactory::new(catalog_list, false).query_engine(); + let catalog_manager = new_memory_catalog_manager().unwrap(); + let req = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: "m".to_string(), + table_id: table.table_info().ident.table_id, + table: table.clone(), + }; + catalog_manager.register_table_sync(req).unwrap(); + + let engine = QueryEngineFactory::new(catalog_manager, false).query_engine(); TimeRangeTester { engine, table } } diff --git a/src/script/Cargo.toml b/src/script/Cargo.toml index 87f4307f1585..aa0da80f8c30 100644 --- a/src/script/Cargo.toml +++ b/src/script/Cargo.toml @@ -69,6 +69,7 @@ table = { path = "../table" } tokio.workspace = true [dev-dependencies] +catalog = { path = "../catalog", features = ["testing"] } common-test-util = { path = "../common/test-util" } log-store = { path = "../log-store" } mito = { path = "../mito", features = ["test"] } diff --git a/src/script/benches/py_benchmark.rs b/src/script/benches/py_benchmark.rs index 0b5eede342fe..eb97e16f0cc7 100644 --- a/src/script/benches/py_benchmark.rs +++ b/src/script/benches/py_benchmark.rs @@ -15,8 +15,7 @@ use std::collections::HashMap; use std::sync::Arc; -use catalog::local::{MemoryCatalogProvider, MemorySchemaProvider}; -use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; +use catalog::local::MemoryCatalogManager; use common_query::Output; use criterion::{black_box, criterion_group, criterion_main, Criterion}; use futures::Future; @@ -50,22 +49,10 @@ where } pub(crate) fn sample_script_engine() -> PyEngine { - let catalog_list = catalog::local::new_memory_catalog_list().unwrap(); - - let default_schema = Arc::new(MemorySchemaProvider::new()); - default_schema - .register_table_sync("numbers".to_string(), Arc::new(NumbersTable::default())) - .unwrap(); - let default_catalog = Arc::new(MemoryCatalogProvider::new()); - default_catalog - .register_schema_sync(DEFAULT_SCHEMA_NAME.to_string(), default_schema) - .unwrap(); - catalog_list - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), default_catalog) - .unwrap(); - - let factory = QueryEngineFactory::new(catalog_list, false); - let query_engine = factory.query_engine(); + let catalog_manager = Arc::new(MemoryCatalogManager::new_with_table(Arc::new( + NumbersTable::default(), + ))); + let query_engine = QueryEngineFactory::new(catalog_manager, false).query_engine(); PyEngine::new(query_engine.clone()) } diff --git a/src/script/src/manager.rs b/src/script/src/manager.rs index 84e9ad87a2d1..6af2006e8c4d 100644 --- a/src/script/src/manager.rs +++ b/src/script/src/manager.rs @@ -170,6 +170,7 @@ mod tests { .await .unwrap(), ); + catalog_manager.start().await.unwrap(); let factory = QueryEngineFactory::new(catalog_manager.clone(), false); let query_engine = factory.query_engine(); diff --git a/src/script/src/python/engine.rs b/src/script/src/python/engine.rs index 10c3de5b9f80..5dec2c4f37da 100644 --- a/src/script/src/python/engine.rs +++ b/src/script/src/python/engine.rs @@ -357,8 +357,7 @@ pub(crate) use tests::sample_script_engine; #[cfg(test)] mod tests { - use catalog::local::{MemoryCatalogProvider, MemorySchemaProvider}; - use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; + use catalog::local::MemoryCatalogManager; use common_recordbatch::util; use datatypes::prelude::ScalarVector; use datatypes::value::Value; @@ -369,22 +368,10 @@ mod tests { use super::*; pub(crate) fn sample_script_engine() -> PyEngine { - let catalog_list = catalog::local::new_memory_catalog_list().unwrap(); - - let default_schema = Arc::new(MemorySchemaProvider::new()); - default_schema - .register_table_sync("numbers".to_string(), Arc::new(NumbersTable::default())) - .unwrap(); - let default_catalog = Arc::new(MemoryCatalogProvider::new()); - default_catalog - .register_schema_sync(DEFAULT_SCHEMA_NAME.to_string(), default_schema) - .unwrap(); - catalog_list - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), default_catalog) - .unwrap(); - - let factory = QueryEngineFactory::new(catalog_list, false); - let query_engine = factory.query_engine(); + let catalog_manager = Arc::new(MemoryCatalogManager::new_with_table(Arc::new( + NumbersTable::default(), + ))); + let query_engine = QueryEngineFactory::new(catalog_manager, false).query_engine(); PyEngine::new(query_engine.clone()) } diff --git a/src/servers/tests/mod.rs b/src/servers/tests/mod.rs index 910b2ca132b7..ea3f9aa4ccd4 100644 --- a/src/servers/tests/mod.rs +++ b/src/servers/tests/mod.rs @@ -18,7 +18,7 @@ use std::sync::{Arc, RwLock}; use api::v1::greptime_request::{Request as GreptimeRequest, Request}; use api::v1::query_request::Query; use async_trait::async_trait; -use catalog::local::{MemoryCatalogManager, MemoryCatalogProvider, MemorySchemaProvider}; +use catalog::local::MemoryCatalogManager; use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME}; use common_query::Output; use query::parser::{PromQuery, QueryLanguageParser, QueryStatement}; @@ -200,24 +200,9 @@ impl GrpcQueryHandler for DummyInstance { } fn create_testing_instance(table: MemTable) -> DummyInstance { - let table_name = table.table_name().to_string(); let table = Arc::new(table); - - let schema_provider = Arc::new(MemorySchemaProvider::new()); - let catalog_provider = Arc::new(MemoryCatalogProvider::new()); - let catalog_list = Arc::new(MemoryCatalogManager::default()); - schema_provider - .register_table_sync(table_name, table) - .unwrap(); - catalog_provider - .register_schema_sync(DEFAULT_SCHEMA_NAME.to_string(), schema_provider) - .unwrap(); - catalog_list - .register_catalog_sync(DEFAULT_CATALOG_NAME.to_string(), catalog_provider) - .unwrap(); - - let factory = QueryEngineFactory::new(catalog_list, false); - let query_engine = factory.query_engine(); + let catalog_manager = Arc::new(MemoryCatalogManager::new_with_table(table)); + let query_engine = QueryEngineFactory::new(catalog_manager, false).query_engine(); DummyInstance::new(query_engine) } diff --git a/src/table-procedure/src/alter.rs b/src/table-procedure/src/alter.rs index 9674dbd6be37..4f46cbb6e86e 100644 --- a/src/table-procedure/src/alter.rs +++ b/src/table-procedure/src/alter.rs @@ -29,8 +29,8 @@ use table::metadata::TableId; use table::requests::{AlterKind, AlterTableRequest}; use crate::error::{ - AccessCatalogSnafu, CatalogNotFoundSnafu, DeserializeProcedureSnafu, SchemaNotFoundSnafu, - SerializeProcedureSnafu, TableExistsSnafu, TableNotFoundSnafu, + AccessCatalogSnafu, DeserializeProcedureSnafu, SerializeProcedureSnafu, TableExistsSnafu, + TableNotFoundSnafu, }; /// Procedure to alter a table. @@ -134,26 +134,14 @@ impl AlterTableProcedure { } async fn on_prepare(&mut self) -> Result { - // Check whether catalog and schema exist. let request = &self.data.request; - let catalog = self + let table = self .catalog_manager - .catalog(&request.catalog_name) - .await - .context(AccessCatalogSnafu)? - .context(CatalogNotFoundSnafu { - name: &request.catalog_name, - })?; - let schema = catalog - .schema(&request.schema_name) - .await - .context(AccessCatalogSnafu)? - .context(SchemaNotFoundSnafu { - name: &request.schema_name, - })?; - - let table = schema - .table(&request.table_name) + .table( + &request.catalog_name, + &request.schema_name, + &request.table_name, + ) .await .context(AccessCatalogSnafu)? .with_context(|| TableNotFoundSnafu { @@ -162,12 +150,14 @@ impl AlterTableProcedure { request.catalog_name, request.schema_name, request.table_name ), })?; + if let AlterKind::RenameTable { new_table_name } = &self.data.request.alter_kind { ensure!( - !schema - .table_exist(new_table_name) + self.catalog_manager + .table(&request.catalog_name, &request.schema_name, new_table_name) .await - .context(AccessCatalogSnafu)?, + .context(AccessCatalogSnafu)? + .is_none(), TableExistsSnafu { name: format!( "{}.{}.{}", @@ -341,16 +331,17 @@ mod tests { let mut watcher = procedure_manager.submit(procedure_with_id).await.unwrap(); watcher.changed().await.unwrap(); - let catalog = catalog_manager - .catalog(DEFAULT_CATALOG_NAME) + let table = catalog_manager + .table(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, new_table_name) .await .unwrap() .unwrap(); - let schema = catalog.schema(DEFAULT_SCHEMA_NAME).await.unwrap().unwrap(); - let table = schema.table(new_table_name).await.unwrap().unwrap(); let table_info = table.table_info(); assert_eq!(new_table_name, table_info.name); - assert!(schema.table(table_name).await.unwrap().is_none()); + assert!(!catalog_manager + .table_exist(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) + .await + .unwrap()); } } diff --git a/src/table-procedure/src/create.rs b/src/table-procedure/src/create.rs index e6ab7ef21037..3d1f41a29e44 100644 --- a/src/table-procedure/src/create.rs +++ b/src/table-procedure/src/create.rs @@ -23,13 +23,12 @@ use common_procedure::{ }; use common_telemetry::logging; use serde::{Deserialize, Serialize}; -use snafu::{OptionExt, ResultExt}; +use snafu::ResultExt; use table::engine::{EngineContext, TableEngineProcedureRef, TableEngineRef, TableReference}; use table::requests::{CreateTableRequest, OpenTableRequest}; use crate::error::{ - AccessCatalogSnafu, CatalogNotFoundSnafu, DeserializeProcedureSnafu, SchemaNotFoundSnafu, - SerializeProcedureSnafu, + AccessCatalogSnafu, DeserializeProcedureSnafu, SchemaNotFoundSnafu, SerializeProcedureSnafu, }; /// Procedure to create a table. @@ -133,34 +132,24 @@ impl CreateTableProcedure { } async fn on_prepare(&mut self) -> Result { - // Check whether catalog and schema exist. - let catalog = self + if !self .catalog_manager - .catalog(&self.data.request.catalog_name) - .await - .context(AccessCatalogSnafu)? - .with_context(|| { - logging::error!( - "Failed to create table {}, catalog not found", - self.data.table_ref() - ); - CatalogNotFoundSnafu { - name: &self.data.request.catalog_name, - } - })?; - catalog - .schema(&self.data.request.schema_name) + .schema_exist( + &self.data.request.catalog_name, + &self.data.request.schema_name, + ) .await .context(AccessCatalogSnafu)? - .with_context(|| { - logging::error!( - "Failed to create table {}, schema not found", - self.data.table_ref(), - ); - SchemaNotFoundSnafu { - name: &self.data.request.schema_name, - } - })?; + { + logging::error!( + "Failed to create table {}, schema not found", + self.data.table_ref(), + ); + return SchemaNotFoundSnafu { + name: &self.data.request.schema_name, + } + .fail()?; + } self.data.state = CreateTableState::EngineCreateTable; // Assign procedure id to the subprocedure. @@ -224,28 +213,16 @@ impl CreateTableProcedure { } async fn on_register_catalog(&mut self) -> Result { - let catalog = self + if self .catalog_manager - .catalog(&self.data.request.catalog_name) - .await - .context(AccessCatalogSnafu)? - .context(CatalogNotFoundSnafu { - name: &self.data.request.catalog_name, - })?; - let schema = catalog - .schema(&self.data.request.schema_name) - .await - .context(AccessCatalogSnafu)? - .context(SchemaNotFoundSnafu { - name: &self.data.request.schema_name, - })?; - let table_exists = schema - .table(&self.data.request.table_name) + .table_exist( + &self.data.request.catalog_name, + &self.data.request.schema_name, + &self.data.request.table_name, + ) .await .map_err(Error::from_error_ext)? - .is_some(); - if table_exists { - // Table already exists. + { return Ok(Status::Done); } diff --git a/src/table-procedure/src/drop.rs b/src/table-procedure/src/drop.rs index cfd8c4fa9fef..a796bea1f0b3 100644 --- a/src/table-procedure/src/drop.rs +++ b/src/table-procedure/src/drop.rs @@ -28,8 +28,7 @@ use table::engine::{EngineContext, TableEngineProcedureRef, TableReference}; use table::requests::DropTableRequest; use crate::error::{ - AccessCatalogSnafu, DeregisterTableSnafu, DeserializeProcedureSnafu, SerializeProcedureSnafu, - TableNotFoundSnafu, + AccessCatalogSnafu, DeserializeProcedureSnafu, SerializeProcedureSnafu, TableNotFoundSnafu, }; /// Procedure to drop a table. @@ -159,17 +158,10 @@ impl DropTableProcedure { schema: self.data.request.schema_name.clone(), table_name: self.data.request.table_name.clone(), }; - if !self - .catalog_manager + self.catalog_manager .deregister_table(deregister_table_req) .await - .context(AccessCatalogSnafu)? - { - return DeregisterTableSnafu { - name: request.table_ref().to_string(), - } - .fail()?; - } + .context(AccessCatalogSnafu)?; } self.data.state = DropTableState::EngineDropTable; @@ -298,13 +290,11 @@ mod tests { let mut watcher = procedure_manager.submit(procedure_with_id).await.unwrap(); watcher.changed().await.unwrap(); - let catalog = catalog_manager - .catalog(DEFAULT_CATALOG_NAME) + assert!(!catalog_manager + .table_exist(DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, table_name) .await - .unwrap() - .unwrap(); - let schema = catalog.schema(DEFAULT_SCHEMA_NAME).await.unwrap().unwrap(); - assert!(schema.table(table_name).await.unwrap().is_none()); + .unwrap()); + let ctx = EngineContext::default(); assert!(!table_engine.table_exists(&ctx, table_id,)); } diff --git a/src/table/src/table/numbers.rs b/src/table/src/table/numbers.rs index 4ee32e40c827..c826c8f23cda 100644 --- a/src/table/src/table/numbers.rs +++ b/src/table/src/table/numbers.rs @@ -16,6 +16,7 @@ use std::any::Any; use std::pin::Pin; use std::sync::Arc; +use common_catalog::consts::{DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, NUMBERS_TABLE_ID}; use common_query::physical_plan::PhysicalPlanRef; use common_recordbatch::error::Result as RecordBatchResult; use common_recordbatch::{RecordBatch, RecordBatchStream, SendableRecordBatchStream}; @@ -38,6 +39,8 @@ use crate::table::{Expr, Table}; const NUMBER_COLUMN: &str = "number"; +pub const NUMBERS_TABLE_NAME: &str = "numbers"; + /// numbers table for test #[derive(Debug, Clone)] pub struct NumbersTable { @@ -49,7 +52,7 @@ pub struct NumbersTable { impl NumbersTable { pub fn new(table_id: TableId) -> Self { - NumbersTable::with_name(table_id, "numbers".to_string()) + NumbersTable::with_name(table_id, NUMBERS_TABLE_NAME.to_string()) } pub fn with_name(table_id: TableId, name: String) -> Self { @@ -74,7 +77,7 @@ impl NumbersTable { impl Default for NumbersTable { fn default() -> Self { - NumbersTable::new(1) + NumbersTable::new(NUMBERS_TABLE_ID) } } @@ -93,8 +96,8 @@ impl Table for NumbersTable { TableInfoBuilder::default() .table_id(self.table_id) .name(&self.name) - .catalog_name("greptime") - .schema_name("public") + .catalog_name(DEFAULT_CATALOG_NAME) + .schema_name(DEFAULT_SCHEMA_NAME) .table_version(0) .table_type(TableType::Base) .meta( diff --git a/tests-integration/src/cluster.rs b/tests-integration/src/cluster.rs index 691963dcdb2e..67c46d7f35c6 100644 --- a/tests-integration/src/cluster.rs +++ b/tests-integration/src/cluster.rs @@ -184,7 +184,7 @@ impl GreptimeDbClusterBuilder { instance.start().await.unwrap(); // create another catalog and schema for testing - let _ = instance + instance .catalog_manager() .as_any() .downcast_ref::() diff --git a/tests-integration/src/test_util.rs b/tests-integration/src/test_util.rs index 70c32b6d3e5c..975105fa4bc4 100644 --- a/tests-integration/src/test_util.rs +++ b/tests-integration/src/test_util.rs @@ -19,7 +19,7 @@ use std::sync::Arc; use std::time::Duration; use axum::Router; -use catalog::CatalogManagerRef; +use catalog::{CatalogManagerRef, RegisterTableRequest}; use common_catalog::consts::{ DEFAULT_CATALOG_NAME, DEFAULT_SCHEMA_NAME, MIN_USER_TABLE_ID, MITO_ENGINE, }; @@ -308,19 +308,14 @@ pub async fn create_test_table( .await .context(CreateTableSnafu { table_name })?; - let schema_provider = catalog_manager - .catalog(DEFAULT_CATALOG_NAME) - .await - .unwrap() - .unwrap() - .schema(DEFAULT_SCHEMA_NAME) - .await - .unwrap() - .unwrap(); - schema_provider - .register_table(table_name.to_string(), table) - .await - .unwrap(); + let req = RegisterTableRequest { + catalog: DEFAULT_CATALOG_NAME.to_string(), + schema: DEFAULT_SCHEMA_NAME.to_string(), + table_name: table_name.to_string(), + table_id: table.table_info().ident.table_id, + table, + }; + catalog_manager.register_table(req).await.unwrap(); Ok(()) } diff --git a/tests-integration/src/tests.rs b/tests-integration/src/tests.rs index 06442b4d0a06..24b0843559e2 100644 --- a/tests-integration/src/tests.rs +++ b/tests-integration/src/tests.rs @@ -19,7 +19,7 @@ mod test_util; use std::collections::HashMap; use std::sync::Arc; -use catalog::local::{MemoryCatalogProvider, MemorySchemaProvider}; +use catalog::RegisterSchemaRequest; use common_test_util::temp_dir::TempDir; use datanode::instance::Instance as DatanodeInstance; use frontend::instance::Instance; @@ -71,17 +71,18 @@ pub(crate) async fn create_standalone_instance(test_name: &str) -> MockStandalon .await .unwrap(); - // create another catalog and schema for testing - let another_catalog = Arc::new(MemoryCatalogProvider::new()); - let _ = another_catalog - .register_schema_sync( - "another_schema".to_string(), - Arc::new(MemorySchemaProvider::new()), - ) + dn_instance + .catalog_manager() + .register_catalog("another_catalog".to_string()) + .await .unwrap(); - let _ = dn_instance + let req = RegisterSchemaRequest { + catalog: "another_catalog".to_string(), + schema: "another_schema".to_string(), + }; + dn_instance .catalog_manager() - .register_catalog("another_catalog".to_string(), another_catalog) + .register_schema(req) .await .unwrap(); diff --git a/tests-integration/src/tests/instance_test.rs b/tests-integration/src/tests/instance_test.rs index 27c6df82b92f..8f975229b92c 100644 --- a/tests-integration/src/tests/instance_test.rs +++ b/tests-integration/src/tests/instance_test.rs @@ -1325,7 +1325,7 @@ async fn test_information_schema_dot_tables(instance: Arc) { +---------------+--------------+------------+------------+----------+-------------+ | table_catalog | table_schema | table_name | table_type | table_id | engine | +---------------+--------------+------------+------------+----------+-------------+ -| greptime | public | numbers | BASE TABLE | 1 | test_engine | +| greptime | public | numbers | BASE TABLE | 2 | test_engine | | greptime | public | scripts | BASE TABLE | 1024 | mito | +---------------+--------------+------------+------------+----------+-------------+" } @@ -1334,7 +1334,7 @@ async fn test_information_schema_dot_tables(instance: Arc) { +---------------+--------------+------------+------------+----------+-------------+ | table_catalog | table_schema | table_name | table_type | table_id | engine | +---------------+--------------+------------+------------+----------+-------------+ -| greptime | public | numbers | BASE TABLE | 1 | test_engine | +| greptime | public | numbers | BASE TABLE | 2 | test_engine | | greptime | public | scripts | BASE TABLE | 1 | mito | +---------------+--------------+------------+------------+----------+-------------+" }