timsaucer commented on code in PR #18022:
URL: https://github.com/apache/datafusion/pull/18022#discussion_r2442500181
##########
datafusion/catalog/src/schema.rs:
##########
@@ -88,4 +88,45 @@ pub trait SchemaProvider: Debug + Sync + Send {
/// Returns true if table exist in the schema provider, false otherwise.
fn table_exist(&self, name: &str) -> bool;
+
+ /// Retrieves the list of available table function names in this schema.
+ fn udtf_names(&self) -> Vec<String> {
+ vec![]
+ }
+
+ /// Retrieves a specific table function from the schema by name, if it
exists,
+ /// otherwise returns `None`.
+ fn udtf(&self, name: &str) -> Result<Option<Arc<TableFunction>>> {
+ let _ = name; // suppress unused warning
+ Ok(None)
+ }
+
+ /// If supported by the implementation, adds a new table function named
`name` to
+ /// this schema.
+ ///
+ /// If a table function of the same name was already registered, returns
"Table
+ /// function already exists" error.
+ #[allow(unused_variables)]
Review Comment:
nit: Instead of this I'd prefer to use `_name` and `_function`
##########
datafusion/catalog/src/schema.rs:
##########
@@ -88,4 +88,45 @@ pub trait SchemaProvider: Debug + Sync + Send {
/// Returns true if table exist in the schema provider, false otherwise.
fn table_exist(&self, name: &str) -> bool;
+
+ /// Retrieves the list of available table function names in this schema.
+ fn udtf_names(&self) -> Vec<String> {
+ vec![]
+ }
+
+ /// Retrieves a specific table function from the schema by name, if it
exists,
+ /// otherwise returns `None`.
+ fn udtf(&self, name: &str) -> Result<Option<Arc<TableFunction>>> {
+ let _ = name; // suppress unused warning
+ Ok(None)
+ }
+
+ /// If supported by the implementation, adds a new table function named
`name` to
+ /// this schema.
+ ///
+ /// If a table function of the same name was already registered, returns
"Table
+ /// function already exists" error.
+ #[allow(unused_variables)]
+ fn register_udtf(
+ &self,
+ name: String,
+ function: Arc<TableFunction>,
+ ) -> Result<Option<Arc<TableFunction>>> {
+ exec_err!("schema provider does not support registering table
functions")
+ }
+
+ /// If supported by the implementation, removes the `name` table function
from this
+ /// schema and returns the previously registered [`TableFunction`], if any.
+ ///
+ /// If no `name` table function exists, returns Ok(None).
+ #[allow(unused_variables)]
Review Comment:
Same comment re: unused_variables
##########
datafusion/sqllogictest/test_files/table_functions.slt:
##########
@@ -494,3 +494,32 @@ SELECT c, f.* FROM json_table, LATERAL
generate_series(1,2) f;
1 2
2 1
2 2
+
+#
+# Test schema-qualified table function names (Phase 3)
+# Global table functions are not accessible via qualified names
+#
+
+# Test: unqualified table function name resolves from global registry
(backward compatibility)
+query I
+SELECT * FROM generate_series(1, 3)
+----
+1
+2
+3
+
+# Test: qualified name for global table function should fail
+# (global table functions are not in schemas, they're in the global registry)
+statement error DataFusion error: Error during planning: Table function
'public.generate_series' not found in schema
+SELECT * FROM public.generate_series(1, 3)
Review Comment:
If I register a table without qualification, then I believe I can still
access that table via `datafusion.public.my_table`. Right? If so I think we
would want the same behavior for these functions as we do for bare tables.
--
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]
