alamb commented on code in PR #23703: URL: https://github.com/apache/datafusion/pull/23703#discussion_r3634452151
########## datafusion/session/src/catalog.rs: ########## @@ -0,0 +1,246 @@ +// 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::any::Any; +use std::fmt::Debug; +use std::sync::Arc; + +pub use crate::schema::SchemaProvider; +use datafusion_common::Result; +use datafusion_common::not_impl_err; + +/// A catalog list that contains no catalogs. +/// +/// [`Session`](crate::Session) implementations that do not provide catalog Review Comment: this is something new I think ########## datafusion/catalog/src/lib.rs: ########## @@ -57,8 +57,3 @@ pub use memory::{ }; pub use schema::*; pub use table::*; - -// For backwards compatibility, Review Comment: lol ########## datafusion/catalog/src/catalog.rs: ########## @@ -15,195 +15,4 @@ // specific language governing permissions and limitations // under the License. -use std::any::Any; -use std::fmt::Debug; -use std::sync::Arc; - -pub use crate::schema::SchemaProvider; -use datafusion_common::Result; -use datafusion_common::not_impl_err; - -/// Represents a catalog, comprising a number of named schemas. -/// -/// # Catalog Overview -/// -/// To plan and execute queries, DataFusion needs a "Catalog" that provides -/// metadata such as which schemas and tables exist, their columns and data -/// types, and how to access the data. -/// -/// The Catalog API consists: -/// * [`CatalogProviderList`]: a collection of `CatalogProvider`s -/// * [`CatalogProvider`]: a collection of `SchemaProvider`s (sometimes called a "database" in other systems) -/// * [`SchemaProvider`]: a collection of `TableProvider`s (often called a "schema" in other systems) -/// * [`TableProvider`]: individual tables -/// -/// # Implementing Catalogs -/// -/// To implement a catalog, you implement at least one of the [`CatalogProviderList`], -/// [`CatalogProvider`] and [`SchemaProvider`] traits and register them -/// appropriately in the `SessionContext`. -/// -/// DataFusion comes with a simple in-memory catalog implementation, -/// `MemoryCatalogProvider`, that is used by default and has no persistence. -/// DataFusion does not include more complex Catalog implementations because -/// catalog management is a key design choice for most data systems, and thus -/// it is unlikely that any general-purpose catalog implementation will work -/// well across many use cases. -/// -/// # Implementing "Remote" catalogs -/// -/// See [`remote_catalog`] for an end to end example of how to implement a -/// remote catalog. -/// -/// Sometimes catalog information is stored remotely and requires a network call -/// to retrieve. For example, the [Delta Lake] table format stores table -/// metadata in files on S3 that must be first downloaded to discover what -/// schemas and tables exist. -/// -/// [Delta Lake]: https://delta.io/ -/// [`remote_catalog`]: https://github.com/apache/datafusion/blob/main/datafusion-examples/examples/data_io/remote_catalog.rs -/// -/// The [`CatalogProvider`] can support this use case, but it takes some care. -/// The planning APIs in DataFusion are not `async` and thus network IO can not -/// be performed "lazily" / "on demand" during query planning. The rationale for -/// this design is that using remote procedure calls for all catalog accesses -/// required for query planning would likely result in multiple network calls -/// per plan, resulting in very poor planning performance. -/// -/// To implement [`CatalogProvider`] and [`SchemaProvider`] for remote catalogs, -/// you need to provide an in memory snapshot of the required metadata. Most -/// systems typically either already have this information cached locally or can -/// batch access to the remote catalog to retrieve multiple schemas and tables -/// in a single network call. -/// -/// Note that [`SchemaProvider::table`] **is** an `async` function in order to -/// simplify implementing simple [`SchemaProvider`]s. For many table formats it -/// is easy to list all available tables but there is additional non trivial -/// access required to read table details (e.g. statistics). -/// -/// The pattern that DataFusion itself uses to plan SQL queries is to walk over -/// the query to find all table references, performing required remote catalog -/// lookups in parallel, storing the results in a cached snapshot, and then plans -/// the query using that snapshot. -/// -/// # Example Catalog Implementations -/// -/// Here are some examples of how to implement custom catalogs: -/// -/// * [`datafusion-cli`]: [`DynamicFileCatalogProvider`] catalog provider -/// that treats files and directories on a filesystem as tables. -/// -/// * The [`catalog.rs`]: a simple directory based catalog. -/// -/// * [delta-rs]: [`UnityCatalogProvider`] implementation that can -/// read from Delta Lake tables -/// -/// [`datafusion-cli`]: https://datafusion.apache.org/user-guide/cli/index.html -/// [`DynamicFileCatalogProvider`]: https://github.com/apache/datafusion/blob/31b9b48b08592b7d293f46e75707aad7dadd7cbc/datafusion-cli/src/catalog.rs#L75 -/// [`catalog.rs`]: https://github.com/apache/datafusion/blob/main/datafusion-examples/examples/data_io/catalog.rs -/// [delta-rs]: https://github.com/delta-io/delta-rs -/// [`UnityCatalogProvider`]: https://github.com/delta-io/delta-rs/blob/951436ecec476ce65b5ed3b58b50fb0846ca7b91/crates/deltalake-core/src/data_catalog/unity/datafusion.rs#L111-L123 -/// -/// [`TableProvider`]: crate::TableProvider -pub trait CatalogProvider: Any + Debug + Sync + Send { - /// Retrieves the list of available schema names in this catalog. - fn schema_names(&self) -> Vec<String>; - - /// Retrieves a specific schema from the catalog by name, provided it exists. - fn schema(&self, name: &str) -> Option<Arc<dyn SchemaProvider>>; - - /// Adds a new schema to this catalog. - /// - /// If a schema of the same name existed before, it is replaced in - /// the catalog and returned. - /// - /// By default returns a "Not Implemented" error - fn register_schema( - &self, - name: &str, - schema: Arc<dyn SchemaProvider>, - ) -> Result<Option<Arc<dyn SchemaProvider>>> { - // use variables to avoid unused variable warnings - let _ = name; - let _ = schema; - not_impl_err!("Registering new schemas is not supported") - } - - /// Removes a schema from this catalog. Implementations of this method should return - /// errors if the schema exists but cannot be dropped. For example, in DataFusion's - /// default in-memory catalog, `MemoryCatalogProvider`, a non-empty schema - /// will only be successfully dropped when `cascade` is true. - /// This is equivalent to how DROP SCHEMA works in PostgreSQL. - /// - /// Implementations of this method should return None if schema with `name` - /// does not exist. - /// - /// By default returns a "Not Implemented" error - fn deregister_schema( - &self, - _name: &str, - _cascade: bool, - ) -> Result<Option<Arc<dyn SchemaProvider>>> { - not_impl_err!("Deregistering new schemas is not supported") - } -} - -impl dyn CatalogProvider { - /// Returns `true` if the catalog provider is of type `T`. - /// - /// Prefer this over `downcast_ref::<T>().is_some()`. Works correctly when - /// called on `Arc<dyn CatalogProvider>` via auto-deref. - pub fn is<T: CatalogProvider>(&self) -> bool { - (self as &dyn Any).is::<T>() - } - - /// Attempts to downcast this catalog provider to a concrete type `T`, - /// returning `None` if the provider is not of that type. - /// - /// Works correctly when called on `Arc<dyn CatalogProvider>` via auto-deref, - /// unlike `(&arc as &dyn Any).downcast_ref::<T>()` which would attempt to - /// downcast the `Arc` itself. - pub fn downcast_ref<T: CatalogProvider>(&self) -> Option<&T> { - (self as &dyn Any).downcast_ref() - } -} - -/// Represent a list of named [`CatalogProvider`]s. -/// -/// Please see the documentation on [`CatalogProvider`] for details of -/// implementing a custom catalog. -pub trait CatalogProviderList: Any + Debug + Sync + Send { - /// Adds a new catalog to this catalog list - /// If a catalog of the same name existed before, it is replaced in the list and returned. - fn register_catalog( - &self, - name: String, - catalog: Arc<dyn CatalogProvider>, - ) -> Option<Arc<dyn CatalogProvider>>; - - /// Retrieves the list of available catalog names - fn catalog_names(&self) -> Vec<String>; - - /// Retrieves a specific catalog by name, provided it exists. - fn catalog(&self, name: &str) -> Option<Arc<dyn CatalogProvider>>; -} - -impl dyn CatalogProviderList { - /// Returns `true` if the catalog provider list is of type `T`. - /// - /// Prefer this over `downcast_ref::<T>().is_some()`. Works correctly when - /// called on `Arc<dyn CatalogProviderList>` via auto-deref. - pub fn is<T: CatalogProviderList>(&self) -> bool { - (self as &dyn Any).is::<T>() - } - - /// Attempts to downcast this catalog provider list to a concrete type `T`, - /// returning `None` if the provider list is not of that type. - /// - /// Works correctly when called on `Arc<dyn CatalogProviderList>` via - /// auto-deref, unlike `(&arc as &dyn Any).downcast_ref::<T>()` which would - /// attempt to downcast the `Arc` itself. - pub fn downcast_ref<T: CatalogProviderList>(&self) -> Option<&T> { - (self as &dyn Any).downcast_ref() - } -} +pub use datafusion_session::{CatalogProvider, CatalogProviderList}; Review Comment: maybe would be nice to metntion this is for backwards copatibility here in a comment ########## datafusion/session/src/catalog.rs: ########## @@ -0,0 +1,246 @@ +// Licensed to the Apache Software Foundation (ASF) under one Review Comment: So the idea is now that session is defining the APIs and the datafusion-catalog has the implementations? ########## datafusion/session/src/catalog.rs: ########## @@ -0,0 +1,246 @@ +// 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::any::Any; +use std::fmt::Debug; +use std::sync::Arc; + +pub use crate::schema::SchemaProvider; +use datafusion_common::Result; +use datafusion_common::not_impl_err; + +/// A catalog list that contains no catalogs. +/// +/// [`Session`](crate::Session) implementations that do not provide catalog Review Comment: Also a nit is that this is not accessable from datafusion::catalog — datafusion/catalog/src/catalog.rs:18 -- so users would have to add a dependency on datafusion_session I recommend adding a `pub use` in datafusion_catalot too -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
