QuakeWang commented on code in PR #62: URL: https://github.com/apache/paimon-rust/pull/62#discussion_r1741686346
########## crates/paimon/src/catalog/mod.rs: ########## @@ -0,0 +1,254 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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::collections::HashMap; +use std::fmt; +use std::hash::Hash; + +use async_trait::async_trait; +use chrono::Duration; + +use crate::error::Result; +use crate::io::FileIO; +use crate::spec::{RowType, SchemaChange, TableSchema}; + +/// This interface is responsible for reading and writing metadata such as database/table from a paimon catalog. +/// +/// Impl References: <https://github.com/apache/paimon/blob/release-0.8.2/paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java#L42> +#[async_trait] +pub trait Catalog: Send + Sync { + const DEFAULT_DATABASE: &'static str = "default"; + const SYSTEM_TABLE_SPLITTER: &'static str = "$"; + const SYSTEM_DATABASE_NAME: &'static str = "sys"; + + /// Returns the warehouse root path containing all database directories in this catalog. + fn warehouse(&self) -> &str; + + /// Returns the catalog options. + fn options(&self) -> &HashMap<String, String>; + + /// Returns the FileIO instance. + fn file_io(&self) -> &FileIO; Review Comment: > The catalog is integrated with file I/O, which is somewhat surprising to me. The [FileIO fileIO()](https://github.com/apache/paimon/blob/release-0.8.2/paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java#L55) function can be found at here. Do you have some good suggestions to deal this? ########## crates/paimon/src/catalog/mod.rs: ########## @@ -0,0 +1,254 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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::collections::HashMap; +use std::fmt; +use std::hash::Hash; + +use async_trait::async_trait; +use chrono::Duration; + +use crate::error::Result; +use crate::io::FileIO; +use crate::spec::{RowType, SchemaChange, TableSchema}; + +/// This interface is responsible for reading and writing metadata such as database/table from a paimon catalog. +/// +/// Impl References: <https://github.com/apache/paimon/blob/release-0.8.2/paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java#L42> +#[async_trait] +pub trait Catalog: Send + Sync { + const DEFAULT_DATABASE: &'static str = "default"; + const SYSTEM_TABLE_SPLITTER: &'static str = "$"; + const SYSTEM_DATABASE_NAME: &'static str = "sys"; + + /// Returns the warehouse root path containing all database directories in this catalog. + fn warehouse(&self) -> &str; + + /// Returns the catalog options. + fn options(&self) -> &HashMap<String, String>; + + /// Returns the FileIO instance. + fn file_io(&self) -> &FileIO; + + /// Lists all databases in this catalog. + async fn list_databases(&self) -> Result<Vec<String>>; + + /// Checks if a database exists in this catalog. + async fn database_exists(&self, database_name: &str) -> Result<bool>; + + /// Creates a new database. + async fn create_database( + &self, + name: &str, + ignore_if_exists: bool, + properties: Option<HashMap<String, String>>, + ) -> Result<()>; + + /// Loads database properties. + async fn load_database_properties(&self, name: &str) -> Result<HashMap<String, String>>; + + /// Drops a database. + async fn drop_database( + &self, + name: &str, + ignore_if_not_exists: bool, + cascade: bool, + ) -> Result<()>; + + /// Returns a Table instance for the specified identifier. + async fn get_table(&self, identifier: &Identifier) -> Result<impl Table>; Review Comment: > Implementing `Table` makes this trait non-object safe. Perhaps we could return a `Table` struct instead. Ok, I will create a Table struct instead of trait. ########## crates/paimon/src/catalog/mod.rs: ########## @@ -0,0 +1,254 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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::collections::HashMap; +use std::fmt; +use std::hash::Hash; + +use async_trait::async_trait; +use chrono::Duration; + +use crate::error::Result; +use crate::io::FileIO; +use crate::spec::{RowType, SchemaChange, TableSchema}; + +/// This interface is responsible for reading and writing metadata such as database/table from a paimon catalog. +/// +/// Impl References: <https://github.com/apache/paimon/blob/release-0.8.2/paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java#L42> +#[async_trait] +pub trait Catalog: Send + Sync { + const DEFAULT_DATABASE: &'static str = "default"; + const SYSTEM_TABLE_SPLITTER: &'static str = "$"; + const SYSTEM_DATABASE_NAME: &'static str = "sys"; + + /// Returns the warehouse root path containing all database directories in this catalog. + fn warehouse(&self) -> &str; + + /// Returns the catalog options. + fn options(&self) -> &HashMap<String, String>; + + /// Returns the FileIO instance. + fn file_io(&self) -> &FileIO; + + /// Lists all databases in this catalog. + async fn list_databases(&self) -> Result<Vec<String>>; + + /// Checks if a database exists in this catalog. + async fn database_exists(&self, database_name: &str) -> Result<bool>; + + /// Creates a new database. + async fn create_database( + &self, + name: &str, + ignore_if_exists: bool, + properties: Option<HashMap<String, String>>, + ) -> Result<()>; + + /// Loads database properties. + async fn load_database_properties(&self, name: &str) -> Result<HashMap<String, String>>; + + /// Drops a database. + async fn drop_database( + &self, + name: &str, + ignore_if_not_exists: bool, + cascade: bool, + ) -> Result<()>; + + /// Returns a Table instance for the specified identifier. + async fn get_table(&self, identifier: &Identifier) -> Result<impl Table>; + + /// Lists all tables in the specified database. + async fn list_tables(&self, database_name: &str) -> Result<Vec<String>>; + + /// Checks if a table exists. + async fn table_exists(&self, identifier: &Identifier) -> Result<bool> { Review Comment: > It's a bit surprising that we use `Identifier` in some places but not in others. Shouldn't we be consistent? In the Java version, `Identifier` is usually used for table-related methods. ########## crates/paimon/src/catalog/mod.rs: ########## @@ -0,0 +1,254 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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::collections::HashMap; +use std::fmt; +use std::hash::Hash; + +use async_trait::async_trait; +use chrono::Duration; + +use crate::error::Result; +use crate::io::FileIO; +use crate::spec::{RowType, SchemaChange, TableSchema}; + +/// This interface is responsible for reading and writing metadata such as database/table from a paimon catalog. +/// +/// Impl References: <https://github.com/apache/paimon/blob/release-0.8.2/paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java#L42> +#[async_trait] +pub trait Catalog: Send + Sync { + const DEFAULT_DATABASE: &'static str = "default"; + const SYSTEM_TABLE_SPLITTER: &'static str = "$"; + const SYSTEM_DATABASE_NAME: &'static str = "sys"; + + /// Returns the warehouse root path containing all database directories in this catalog. + fn warehouse(&self) -> &str; + Review Comment: > Do APIs like `lockFactory` need to be added? I think this PR will add the struct and definition first, maybe we can add some APIs like `lockFactory` in the further PRs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
