alamb commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4190193225
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -622,7 +673,34 @@ impl SessionState {
&self,
statement: Statement,
) -> datafusion_common::Result<LogicalPlan> {
- let references = self.resolve_table_references(&statement)?;
+ let mut references = self.resolve_table_references(&statement)?;
Review Comment:
this is a nice change to move it here into the session state
##########
datafusion/sql/src/statement.rs:
##########
@@ -3171,14 +3172,31 @@ FROM (
}
/// Return true if there is a table provider available for "schema.table"
- fn has_table(&self, schema: &str, table: &str) -> bool {
- let tables_reference = TableReference::Partial {
- schema: schema.into(),
- table: table.into(),
+ fn resolve_info_table(&self, table: &str) -> Option<TableReference> {
+ let table_reference = if let Some(system_catalog) =
+ &self.context_provider.options().catalog.system_catalog
+ {
+ TableReference::Full {
+ catalog: system_catalog.deref().into(),
+ schema: "information_schema".into(),
Review Comment:
This information_schema reference feels wrong to me -- I thought the design
was that the core of Datafusion / the sql parser didn't have any special case
for the information_schema and information schema was handled with the same API
as the other catalog providers
##########
datafusion/sqllogictest/test_files/information_schema_multiple_catalogs.slt:
##########
@@ -76,25 +76,24 @@ set datafusion.catalog.default_schema = my_other_schema;
statement ok
create table t3 as values(1);
+query TTT rowsort
+SELECT table_catalog, table_schema, table_name
+FROM my_catalog.information_schema.tables
+WHERE table_schema <> 'information_schema';
+----
+my_catalog my_schema t1
+my_catalog my_schema t2
+
+query TTT rowsort
+SELECT table_catalog, table_schema, table_name
+FROM my_other_catalog.information_schema.tables
+WHERE table_schema <> 'information_schema';
+----
+my_other_catalog my_other_schema t3
+
query TTTT rowsort
SELECT * from information_schema.tables;
Review Comment:
why don't all the other tables appear now? That seems ilke people would
treat it as a regression 🤔
##########
datafusion/sql/src/statement.rs:
##########
@@ -3171,14 +3172,31 @@ FROM (
}
/// Return true if there is a table provider available for "schema.table"
- fn has_table(&self, schema: &str, table: &str) -> bool {
- let tables_reference = TableReference::Partial {
- schema: schema.into(),
- table: table.into(),
+ fn resolve_info_table(&self, table: &str) -> Option<TableReference> {
+ let table_reference = if let Some(system_catalog) =
Review Comment:
maybe we can adjust the comments here to explain the scoping rules:
Looks first in the system catalog, if defined, and otherwise treats it as
unqualified reference
--
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]
