You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "paleolimbot (via GitHub)" <gi...@apache.org> on 2026/10/01 19:28:00 UTC

[PR] feat(rust/sedona-catalog): Simplify catalog extension and integrate with SQL planning [sedona-db]

paleolimbot opened a new pull request, #1382:
URL: https://github.com/apache/sedona-db/pull/1382

   (no comment)


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(rust/sedona-catalog): Simplify catalog extension and integrate with SQL planning [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #1382:
URL: https://github.com/apache/sedona-db/pull/1382#discussion_r4162535197


##########
rust/sedona/src/catalog.rs:
##########
@@ -26,38 +26,90 @@ use datafusion::datasource::TableProvider;
 use datafusion::error::Result;
 use datafusion::execution::context::SessionState;
 use parking_lot::RwLock;
-
-/// Wraps another catalog, automatically register require object stores for the file locations
+use sedona_catalog::{CatalogObjectType, SedonaCatalogList};
+
+/// Owns the catalog composition for a Sedona session.
+///
+/// Foreign Sedona catalogs are searched newest-first and take precedence over
+/// DataFusion's built-in catalogs during Sedona SQL planning. Foreign catalogs
+/// are never exposed through the synchronous DataFusion API. Registration continues
+/// to target the built-in list. When enabled, built-in schemas are wrapped to
+/// register object stores dynamically for file locations.
 #[derive(Debug)]
-pub struct DynamicObjectStoreCatalog {
-    inner: Arc<dyn CatalogProviderList>,
+pub struct SedonaCatalogRegistry {
+    foreign: RwLock<Vec<Arc<dyn SedonaCatalogList>>>,
+    fallback: Arc<dyn CatalogProviderList>,
     state: Weak<RwLock<SessionState>>,
+    dynamic_object_store: bool,
 }
 
-impl DynamicObjectStoreCatalog {
-    pub fn new(inner: Arc<dyn CatalogProviderList>, state: Weak<RwLock<SessionState>>) -> Self {
-        Self { inner, state }
+impl SedonaCatalogRegistry {
+    /// Create a registry that layers foreign catalogs over `fallback`.
+    pub fn new(
+        fallback: Arc<dyn CatalogProviderList>,
+        state: Weak<RwLock<SessionState>>,
+        dynamic_object_store: bool,
+    ) -> Self {
+        Self {
+            foreign: RwLock::new(Vec::new()),
+            fallback,
+            state,
+            dynamic_object_store,
+        }
+    }
+
+    /// Add a foreign catalog list. Later registrations take precedence.
+    pub fn register_foreign(&self, catalogs: Arc<dyn SedonaCatalogList>) {
+        self.foreign.write().push(catalogs);
+    }
+
+    /// Resolve catalog ownership asynchronously, newest registration first.
+    pub async fn foreign_catalog(&self, name: &str) -> Result<Option<Arc<dyn SedonaCatalogList>>> {
+        let foreign = self.foreign.read().clone();
+        for catalogs in foreign.iter().rev() {
+            if catalogs
+                .list_identifiers(&[name], Some(0))
+                .await?

Review Comment:
   Hopefully the catalog implementations do a reasonable job caching that...I may not be understanding the whole issue but I think that if the catalog list can't be resolved it can't be ruled out that a foreign catalog doesn't need to be resolved.



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(rust/sedona-catalog): Simplify catalog extension and integrate with SQL planning [sedona-db]

Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #1382:
URL: https://github.com/apache/sedona-db/pull/1382#discussion_r4162106835


##########
rust/sedona/src/context.rs:
##########
@@ -655,7 +668,11 @@ impl SedonaContext {
         let mut results = Vec::with_capacity(statements.len());
         for statement in statements {
             let plan = create_plan_from_sql(self, statement.clone()).await?;
-            let df = self.ctx.execute_logical_plan(plan).await?;
+            let df = if let Some(df) = execute_sedona_catalog_ddl(self, &plan, &statement).await? {

Review Comment:
   execute_sedona_catalog_ddl is kind of a misnomer here right? which means that we plan all the queries before any of them run, which can result in erroneous resolution failure.
   
   I think it would be correct to actually execute DDLs here, which I think is what non-foreign catalogs do.



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(rust/sedona-catalog): Simplify catalog extension and integrate with SQL planning [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #1382:
URL: https://github.com/apache/sedona-db/pull/1382


-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(rust/sedona-catalog): Simplify catalog extension and integrate with SQL planning [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #1382:
URL: https://github.com/apache/sedona-db/pull/1382#discussion_r4162312802


##########
rust/sedona/src/context.rs:
##########
@@ -655,7 +668,11 @@ impl SedonaContext {
         let mut results = Vec::with_capacity(statements.len());
         for statement in statements {
             let plan = create_plan_from_sql(self, statement.clone()).await?;
-            let df = self.ctx.execute_logical_plan(plan).await?;
+            let df = if let Some(df) = execute_sedona_catalog_ddl(self, &plan, &statement).await? {

Review Comment:
   Good catch! I renamed this one to `resolve_sedona_catalog_ddl()` and executed the plan like DataFusion does.



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(rust/sedona-catalog): Simplify catalog extension and integrate with SQL planning [sedona-db]

Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #1382:
URL: https://github.com/apache/sedona-db/pull/1382#discussion_r4162124693


##########
rust/sedona/src/catalog.rs:
##########
@@ -26,38 +26,90 @@ use datafusion::datasource::TableProvider;
 use datafusion::error::Result;
 use datafusion::execution::context::SessionState;
 use parking_lot::RwLock;
-
-/// Wraps another catalog, automatically register require object stores for the file locations
+use sedona_catalog::{CatalogObjectType, SedonaCatalogList};
+
+/// Owns the catalog composition for a Sedona session.
+///
+/// Foreign Sedona catalogs are searched newest-first and take precedence over
+/// DataFusion's built-in catalogs during Sedona SQL planning. Foreign catalogs
+/// are never exposed through the synchronous DataFusion API. Registration continues
+/// to target the built-in list. When enabled, built-in schemas are wrapped to
+/// register object stores dynamically for file locations.
 #[derive(Debug)]
-pub struct DynamicObjectStoreCatalog {
-    inner: Arc<dyn CatalogProviderList>,
+pub struct SedonaCatalogRegistry {
+    foreign: RwLock<Vec<Arc<dyn SedonaCatalogList>>>,
+    fallback: Arc<dyn CatalogProviderList>,
     state: Weak<RwLock<SessionState>>,
+    dynamic_object_store: bool,
 }
 
-impl DynamicObjectStoreCatalog {
-    pub fn new(inner: Arc<dyn CatalogProviderList>, state: Weak<RwLock<SessionState>>) -> Self {
-        Self { inner, state }
+impl SedonaCatalogRegistry {
+    /// Create a registry that layers foreign catalogs over `fallback`.
+    pub fn new(
+        fallback: Arc<dyn CatalogProviderList>,
+        state: Weak<RwLock<SessionState>>,
+        dynamic_object_store: bool,
+    ) -> Self {
+        Self {
+            foreign: RwLock::new(Vec::new()),
+            fallback,
+            state,
+            dynamic_object_store,
+        }
+    }
+
+    /// Add a foreign catalog list. Later registrations take precedence.
+    pub fn register_foreign(&self, catalogs: Arc<dyn SedonaCatalogList>) {
+        self.foreign.write().push(catalogs);
+    }
+
+    /// Resolve catalog ownership asynchronously, newest registration first.
+    pub async fn foreign_catalog(&self, name: &str) -> Result<Option<Arc<dyn SedonaCatalogList>>> {
+        let foreign = self.foreign.read().clone();
+        for catalogs in foreign.iter().rev() {
+            if catalogs
+                .list_identifiers(&[name], Some(0))
+                .await?

Review Comment:
   should we handle when this throws so that a catalog list failing doesn't fail queries that don't need to resolve from it? 



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(rust/sedona-catalog): Simplify catalog extension and integrate with SQL planning [sedona-db]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #1382:
URL: https://github.com/apache/sedona-db/pull/1382#discussion_r4161380977


##########
rust/sedona/src/catalog_planner.rs:
##########
@@ -0,0 +1,238 @@
+// 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.
+//! Resolve foreign table sources asynchronously before invoking the SQL planner.
+use crate::context::SedonaContext;
+use arrow_schema::DataType;
+use datafusion::{
+    datasource::provider_as_source,
+    execution::session_state::{SessionState, SessionStateBuilder},
+    optimizer::simplify_expressions::ExprSimplifier,
+    sql::{parser::Statement, planner::SqlToRel},
+};
+use datafusion_common::{
+    config::ConfigOptions, plan_datafusion_err, DFSchema, ResolvedTableReference, Result,
+    TableReference,
+};
+use datafusion_expr::{
+    planner::{ContextProvider, ExprPlanner, RelationPlanner, TypePlanner},
+    simplify::SimplifyContext,
+    AggregateUDF, Expr, HigherOrderUDF, LogicalPlan, ScalarUDF, TableSource, WindowUDF,
+};
+use std::{collections::HashMap, sync::Arc};
+
+pub(crate) async fn statement_to_plan(
+    ctx: &SedonaContext,
+    statement: Statement,
+) -> Result<LogicalPlan> {
+    let state = ctx.ctx.state();
+    if ctx.catalog_registry().latest_foreign().is_none() {
+        return state.statement_to_plan(statement).await;
+    }
+    let mut builder = SessionStateBuilder::from(state.clone());
+    let mut provider = CatalogContextProvider {
+        state: &state,
+        tables: HashMap::new(),
+        type_planner: builder.type_planner().clone(),
+    };
+    for reference in state.resolve_table_references(&statement)? {

Review Comment:
   The execution of a statement is scanned for table references which are resolved using the list of registered SedonaCatalogList (last registered resolves first)



##########
rust/sedona-catalog/src/lib.rs:
##########
@@ -0,0 +1,149 @@
+// 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.
+
+//! Asynchronous catalog operations used by SedonaDB extensions.
+//!
+//! Identifiers are paths of literal components, never dot-separated SQL names.
+//! No catalog or schema objects, or synchronous DataFusion adapters, are needed.
+
+use async_trait::async_trait;
+use datafusion_catalog::{Session, TableProvider};
+use datafusion_common::Result;
+use datafusion_physical_plan::ExecutionPlan;
+use serde::{Deserialize, Serialize};
+use std::{fmt::Debug, sync::Arc};
+
+/// Behavior when creating an object that already exists.
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum CreateMode {
+    /// Fail if the object exists.
+    #[default]
+    Create,
+    /// Leave an existing object unchanged.
+    CreateOrIgnore,
+    /// Replace the existing object.
+    Replace,
+}
+
+/// Kind of object in the catalog hierarchy.
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum CatalogObjectType {
+    /// A catalog (database).
+    Catalog,
+    /// A schema (namespace).
+    Schema,
+    /// A physical table.
+    #[default]
+    Table,
+    /// A non-materialized view.
+    View,
+    /// An index belonging to a table.
+    Index,
+}
+
+/// An entry returned by [`SedonaCatalogList::list_identifiers`].
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct CatalogObject {
+    /// Full path from the root, including all parent components.
+    pub identifier: Vec<String>,
+    /// Kind of this object, independent of its path length.
+    pub object_type: CatalogObjectType,
+}
+
+/// Options for creating any catalog object.
+#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(default)]
+pub struct CreateObjectOptions {
+    /// Kind of object to create.
+    pub object_type: CatalogObjectType,
+    /// Conflict behavior, applied when the returned plan executes.
+    pub mode: CreateMode,
+    /// Whether the object is temporary.
+    pub temporary: bool,
+    /// Whether this is an external table.
+    pub external: bool,
+    /// Optional SQL definition supplied by the planner for views and external
+    /// tables. This is passed through as supplied and may omit DDL clauses.
+    pub definition: Option<String>,
+}
+
+/// Options for dropping any catalog object.
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(default)]
+pub struct DropObjectOptions {
+    /// Expected kind. Implementations must not drop an object of another kind.
+    pub object_type: CatalogObjectType,
+    /// Ignore a missing object at execution time.
+    pub if_exists: bool,
+    /// Also drop contained/dependent objects.
+    pub cascade: bool,
+    /// Delete referenced data as well as metadata, where applicable.
+    pub purge: bool,
+}
+
+/// A catalog extension with asynchronous lookup and deferred mutations.
+///
+/// SQL uses `[catalog]`, `[catalog, schema]`, and `[catalog, schema, table]`.
+/// Implementations may expose deeper hierarchies through listing. All errors,
+/// including listing errors, are propagated to the caller.
+#[async_trait]
+pub trait SedonaCatalogList: Debug + Send + Sync {
+    /// Return the stable name of this catalog implementation, such as `iceberg`.
+    /// This identifies the implementation for provider selection (for example,
+    /// a SQL `USING` clause), independently of the catalog names in its hierarchy.
+    /// The name must be available without catalog I/O.
+    fn name(&self) -> &str;
+
+    /// List objects beneath an exact, literal prefix, including the prefix itself
+    /// if it identifies an object. `depth` limits the number of additional path
+    /// components: zero is an exact lookup, one includes immediate children,
+    /// and `None` includes all descendants. An empty prefix addresses the root.
+    ///
+    /// Return full identifiers, without duplicates; ordering is unspecified.
+    /// Missing prefixes return an empty list.
+    async fn list_identifiers(
+        &self,
+        prefix: &[&str],
+        depth: Option<usize>,
+    ) -> Result<Vec<CatalogObject>>;
+
+    /// Look up a table or view by its full identifier; missing objects return None.
+    async fn table(&self, identifier: &[&str]) -> Result<Option<Arc<dyn TableProvider>>>;
+
+    /// Build a create plan without changing catalog state. `input` provides the
+    /// managed table/view schema and data when applicable; it is absent for
+    /// external tables, namespaces, and indexes. `session` belongs to
+    /// this call and must not be retained by the catalog. The returned plan applies
+    /// conflict behavior and validates the parent namespace when executed.
+    async fn create_object(
+        &self,
+        session: &dyn Session,
+        identifier: &[&str],
+        options: &CreateObjectOptions,
+        input: Option<Arc<dyn ExecutionPlan>>,
+    ) -> Result<Arc<dyn ExecutionPlan>>;
+
+    /// Build a drop plan without changing catalog state. Missing-object and
+    /// object-kind checks belong to execution, including `if_exists` handling.
+    async fn drop_object(
+        &self,
+        identifier: &[&str],
+        options: &DropObjectOptions,
+    ) -> Result<Arc<dyn ExecutionPlan>>;
+}

Review Comment:
   This is the extension point: an object can be plugged in that resolves an identifier into a table asynchronously. I also added modifiers even though there's nothing that implements them today...the SQL routing for CREATE TABLE foreign.xx.xx AS SELECT ... or DROP TABLE routes here.



##########
rust/sedona/src/context.rs:
##########
@@ -508,6 +508,19 @@ impl SedonaContext {
         }
     }
 
+    /// Register a foreign catalog list for this session.
+    ///
+    /// Later registrations take precedence over earlier foreign catalogs and
+    /// DataFusion's built-in catalogs when names overlap in Sedona SQL. Foreign
+    /// objects are not exposed through the synchronous DataFusion catalog API.
+    pub fn register_catalog_list(&self, catalogs: Arc<dyn SedonaCatalogList>) {
+        self.catalogs.register_foreign(catalogs);

Review Comment:
   They can be registered in the context



##########
rust/sedona/src/exec.rs:
##########
@@ -14,50 +14,770 @@
 // KIND, either express or implied.  See the License for the
 // specific language governing permissions and limitations
 // under the License.
-use datafusion_expr::{DdlStatement, LogicalPlan};
-
 use crate::{context::SedonaContext, object_storage::register_object_store_and_config_extensions};
-
-use datafusion::{
-    error::{DataFusionError, Result},
-    sql::parser::Statement,
-};
-
-/// A Sedona-specific hook for creating a logical plan from a SQL Statement
-///
-/// Currently this provides support for CREATE EXTERNAL TABLE and
+use async_trait::async_trait;
+use datafusion::catalog::{Session, TableProvider};
+use datafusion::prelude::DataFrame;
+use datafusion::{error::Result, sql::parser::Statement};
+use datafusion_common::{exec_err, SchemaReference, TableReference};
+use datafusion_expr::{DdlStatement, LogicalPlan, TableType};
+use datafusion_physical_plan::ExecutionPlan;
+use sedona_catalog::{CatalogObjectType, CreateMode, CreateObjectOptions, DropObjectOptions};
+use std::{fmt::Debug, sync::Arc};
+
+/// Resolve tables asynchronously and configure stores required by SQL I/O.
 pub(crate) async fn create_plan_from_sql(
     ctx: &SedonaContext,
     statement: Statement,
-) -> Result<LogicalPlan, DataFusionError> {
-    let mut plan = ctx.ctx.state().statement_to_plan(statement).await?;
-
-    // Note that cmd is a mutable reference so that create_external_table function can remove all
-    // datafusion-cli specific options before passing through to datafusion. Otherwise, datafusion
-    // will raise Configuration errors.
+) -> Result<LogicalPlan> {
+    let mut plan = crate::catalog_planner::statement_to_plan(ctx, statement).await?;
     if let LogicalPlan::Ddl(DdlStatement::CreateExternalTable(cmd)) = &plan {
-        register_object_store_and_config_extensions(ctx, &cmd.location, &cmd.options).await?;
+        let state = ctx.ctx.state();
+        let defaults = &state.config_options().catalog;
+        let name = cmd
+            .name
+            .clone()
+            .resolve(&defaults.default_catalog, &defaults.default_schema);
+        if ctx
+            .catalog_registry()
+            .foreign_catalog(&name.catalog)
+            .await?
+            .is_none()
+        {
+            register_object_store_and_config_extensions(ctx, &cmd.location, &cmd.options).await?;
+        }
     }
-
     if let LogicalPlan::Copy(copy_to) = &mut plan {
         register_object_store_and_config_extensions(ctx, &copy_to.output_url, &copy_to.options)
             .await?;
     }
-
     Ok(plan)
 }
 
+/// Route catalog DDL to one async extension. Built-in targets retain DataFusion behavior.
+/// No existence checks or mutations happen here: the returned plan owns them.
+pub(crate) async fn execute_sedona_catalog_ddl(

Review Comment:
   This is the bit that SQL execution goes through to handle DDL statements and route them through the catalog if needed



##########
rust/sedona/src/catalog_planner.rs:
##########
@@ -0,0 +1,238 @@
+// 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.
+//! Resolve foreign table sources asynchronously before invoking the SQL planner.
+use crate::context::SedonaContext;
+use arrow_schema::DataType;
+use datafusion::{
+    datasource::provider_as_source,
+    execution::session_state::{SessionState, SessionStateBuilder},
+    optimizer::simplify_expressions::ExprSimplifier,
+    sql::{parser::Statement, planner::SqlToRel},
+};
+use datafusion_common::{
+    config::ConfigOptions, plan_datafusion_err, DFSchema, ResolvedTableReference, Result,
+    TableReference,
+};
+use datafusion_expr::{
+    planner::{ContextProvider, ExprPlanner, RelationPlanner, TypePlanner},
+    simplify::SimplifyContext,
+    AggregateUDF, Expr, HigherOrderUDF, LogicalPlan, ScalarUDF, TableSource, WindowUDF,
+};
+use std::{collections::HashMap, sync::Arc};
+
+pub(crate) async fn statement_to_plan(
+    ctx: &SedonaContext,
+    statement: Statement,
+) -> Result<LogicalPlan> {
+    let state = ctx.ctx.state();
+    if ctx.catalog_registry().latest_foreign().is_none() {
+        return state.statement_to_plan(statement).await;
+    }
+    let mut builder = SessionStateBuilder::from(state.clone());
+    let mut provider = CatalogContextProvider {
+        state: &state,
+        tables: HashMap::new(),
+        type_planner: builder.type_planner().clone(),
+    };
+    for reference in state.resolve_table_references(&statement)? {
+        let resolved = reference.resolve(
+            &state.config_options().catalog.default_catalog,
+            &state.config_options().catalog.default_schema,
+        );
+        if provider.tables.contains_key(&resolved) {
+            continue;
+        }
+        let table = if let Some(catalog) = ctx
+            .catalog_registry()
+            .foreign_catalog(&resolved.catalog)
+            .await?
+        {
+            catalog
+                .table(&[&resolved.catalog, &resolved.schema, &resolved.table])
+                .await?
+        } else if let Ok(schema) = state.schema_for_ref(resolved.clone()) {
+            schema.table(&resolved.table).await?
+        } else {
+            None
+        };
+        if let Some(table) = table {
+            provider.tables.insert(resolved, provider_as_source(table));
+        }
+    }
+    SqlToRel::new_with_options(&provider, (&state.config_options().sql_parser).into())
+        .statement_to_plan(statement)
+}
+
+struct CatalogContextProvider<'a> {
+    state: &'a SessionState,
+    tables: HashMap<ResolvedTableReference, Arc<dyn TableSource>>,
+    type_planner: Option<Arc<dyn TypePlanner>>,
+}
+
+impl ContextProvider for CatalogContextProvider<'_> {
+    fn get_expr_planners(&self) -> &[Arc<dyn ExprPlanner>] {
+        self.state.expr_planners()
+    }

Review Comment:
   This piece is duplicated from a private internal implementation. The reason we do this is because the other approach involves copying all of the tables from the catalog (full recursive heirarchy)...this is probably the cleanest way to lazily resolve tables until the exact time they need to be resolved. We can revisit this later if it causes issues.



##########
c/sedona-extension/src/sedona_extension.h:
##########
@@ -497,116 +497,48 @@ struct SedonaCTableProvider {
   void* private_data;
 };
 
-/// \brief ABI-stable schema provider interface
+/// \brief Catalog operations over full identifier paths.
 ///
-struct SedonaCSchemaProvider {
-  /// \brief Get the data type of a property
-  int (*get_property_schema)(const struct SedonaCSchemaProvider* self,
-                             const char* property, struct ArrowSchema* out,
-                             struct SedonaCError* err);
-
-  /// \brief Extract a JSON-encoded property from this schema
-  ///
-  /// Supported properties are `owner_name`, `table_names`, and `table_exist`.
-  /// The `table_exist` property accepts `{ "name": "..." }` in args.
-  int (*get_property)(const struct SedonaCSchemaProvider* self, const char* property,
-                      const char* args, struct ArrowArray* out, struct SedonaCError* err);
-
-  /// \brief Look up a table by name
-  int (*table)(const struct SedonaCSchemaProvider* self, const char* name,
-               struct SedonaCTableProvider* out, struct SedonaCError* err);
-
-  /// \brief Create a table from `plan`, taking ownership of the input plan
-  ///
-  /// The returned execution plan performs the create operation when executed.
-  int (*create_table)(const struct SedonaCSchemaProvider* self, const char* name,
-                      struct SedonaCExecutionPlan* plan, struct SedonaCExecutionPlan* out,
-                      struct SedonaCError* err);
-
-  /// \brief Deregister a table by name
-  int (*deregister_table)(const struct SedonaCSchemaProvider* self, const char* name,
-                          struct SedonaCTableProvider* out, struct SedonaCError* err);
-
-  /// \brief Reserved for future use. Must be NULL.
-  void* reserved;
-
-  /// \brief Release this instance
-  ///
-  /// Implementations of this callback must set self->release to NULL.
-  void (*release)(struct SedonaCSchemaProvider* self);
-
-  /// \brief Opaque implementation-specific data
-  void* private_data;
-};
-
-/// \brief ABI-stable catalog provider interface
-struct SedonaCCatalogProvider {
-  /// \brief Get the data type of a property
-  int (*get_property_schema)(const struct SedonaCCatalogProvider* self,
-                             const char* property, struct ArrowSchema* out,
-                             struct SedonaCError* err);
-
-  /// \brief Extract a JSON-encoded property from this catalog
-  ///
-  /// The supported property is `schema_names`.
-  int (*get_property)(const struct SedonaCCatalogProvider* self, const char* property,
-                      const char* args, struct ArrowArray* out, struct SedonaCError* err);
-
-  /// \brief Look up a schema by name
-  int (*schema)(const struct SedonaCCatalogProvider* self, const char* name,
-                struct SedonaCSchemaProvider* out, struct SedonaCError* err);
-
-  /// \brief Create a schema and return it
-  int (*create_schema)(const struct SedonaCCatalogProvider* self, const char* name,
-                       struct SedonaCSchemaProvider* out, struct SedonaCError* err);
-
-  /// \brief Deregister a schema by name
-  int (*deregister_schema)(const struct SedonaCCatalogProvider* self, const char* name,
-                           bool cascade, struct SedonaCSchemaProvider* out,
-                           struct SedonaCError* err);
-
-  /// \brief Reserved for future use. Must be NULL.
-  void* reserved;
-
-  /// \brief Release this instance
-  ///
-  /// Implementations of this callback must set self->release to NULL.
-  void (*release)(struct SedonaCCatalogProvider* self);
-
-  /// \brief Opaque implementation-specific data
-  void* private_data;
-};
-
-/// \brief ABI-stable catalog provider list interface
+/// Callbacks may block and must support concurrent calls. Property names are
+/// UTF-8; identifiers and arguments are UTF-8 JSON. Strings are borrowed for the
+/// duration of the call. All successful outputs transfer ownership to the caller.
 struct SedonaCCatalogProviderList {
-  /// \brief Get the data type of a property
+  /// Get a property's schema. name and list_identifiers are non-null Utf8.
   int (*get_property_schema)(const struct SedonaCCatalogProviderList* self,
                              const char* property, struct ArrowSchema* out,
                              struct SedonaCError* err);
-
-  /// \brief Extract a JSON-encoded property from this catalog list
-  ///
-  /// The supported property is `catalog_names`.
+  /// Get a JSON-encoded property. name returns the stable implementation name
+  /// (e.g., "iceberg") as a JSON string and takes no arguments. It must be
+  /// available without catalog I/O and is cached on import.
+  /// list_identifiers accepts
+  /// args: {"prefix": [...], "depth": null|integer}.
+  /// Prefix matches exact components. Depth counts components after
+  /// prefix, including the prefix itself at depth zero; null is unlimited.
+  /// out: one non-null Utf8 Arrow value containing a JSON array of
+  /// {"identifier": [...], "object_type": "catalog"|"schema"|"table"|"view"|"index"}.
   int (*get_property)(const struct SedonaCCatalogProviderList* self, const char* property,
                       const char* args, struct ArrowArray* out, struct SedonaCError* err);
-
-  /// \brief Look up a catalog by name
-  int (*catalog)(const struct SedonaCCatalogProviderList* self, const char* name,
-                 struct SedonaCCatalogProvider* out, struct SedonaCError* err);
-
-  /// \brief Create a catalog and return it
-  int (*create_catalog)(const struct SedonaCCatalogProviderList* self, const char* name,
-                        struct SedonaCCatalogProvider* out, struct SedonaCError* err);
-
-  /// \brief Reserved for future use. Must be NULL.
+  /// identifier: JSON array of literal components. Missing table: NULL release.
+  int (*table)(const struct SedonaCCatalogProviderList* self, const char* identifier,
+               struct SedonaCTableProvider* out, struct SedonaCError* err);
+  /// args: {"identifier": [...], "options": {"object_type": ..., "mode":
+  /// "create"|"create_or_ignore"|"replace", "temporary": bool, "external": bool,
+  /// "definition": null|string}}. Missing options fields use their Rust defaults.
+  /// A NULL input means no plan; otherwise move it and clear its release callback
+  /// when taking ownership. out is required on success and defers the mutation
+  /// (including existence/conflict checks) until execution.
+  int (*create_object)(const struct SedonaCCatalogProviderList* self, const char* args,
+                       struct SedonaCExecutionPlan* input,
+                       struct SedonaCExecutionPlan* out, struct SedonaCError* err);
+  /// args: {"identifier": [...], "options": {"object_type": ..., "if_exists": bool,
+  /// "cascade": bool, "purge": bool}}. Missing fields use their Rust defaults.
+  /// out is required on success, including for DROP IF EXISTS.
+  int (*drop_object)(const struct SedonaCCatalogProviderList* self, const char* args,
+                     struct SedonaCExecutionPlan* out, struct SedonaCError* err);

Review Comment:
   The FFI for this is much easier than trying to emulate the full catalog list/catalog/schema setup.



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(rust/sedona-catalog): Simplify catalog extension and integrate with SQL planning [sedona-db]

Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #1382:
URL: https://github.com/apache/sedona-db/pull/1382#discussion_r4162106835


##########
rust/sedona/src/context.rs:
##########
@@ -655,7 +668,11 @@ impl SedonaContext {
         let mut results = Vec::with_capacity(statements.len());
         for statement in statements {
             let plan = create_plan_from_sql(self, statement.clone()).await?;
-            let df = self.ctx.execute_logical_plan(plan).await?;
+            let df = if let Some(df) = execute_sedona_catalog_ddl(self, &plan, &statement).await? {

Review Comment:
   execute_sedona_catalog_ddl is kind of a misnomer here right? which means that we plan all the queries before any of them run, which can result in erroneous resolution failure.
   
   I think it would be correct to actually execute DDLs here



-- 
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: issues-unsubscribe@sedona.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org