Repository navigation
Conversation
…alog # Conflicts: # datafusion/core/src/execution/session_state.rs
# Conflicts: # datafusion/execution/src/config.rs
github-actions
Bot
added
sql
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
github-actions
Bot
added
auto detected api change
b5c8c7a to
1b1ed04
Compare
1ae5b90 to
1e5b40e
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #25887 +/- ##
==========================================
+ Coverage 82.57% 82.63% +0.06%
==========================================
Files 1142 1147 +5
Lines 441215 445567 +4352
Branches 441215 445567 +4352
==========================================
+ Hits 364332 368215 +3883
- Misses 54841 54993 +152
- Partials 22042 22359 +317 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@neilconway this branch is now clean. There's still quite a lot that can be improved in the information schema, but I've restricted it to just the scope change here. |
alamb
left a comment
alamb
left a comment
There was a problem hiding this comment.
Thanks @pepijnve -- I am worried about the behavior change in this PR
I thought postgres had some notion of "default search path" or something that would scope its name resolution rules (and presumably also information_schema visibility). Did you consider something like that for DataFusion?
| statement: Statement, | ||
| ) -> datafusion_common::Result<LogicalPlan> { | ||
| let references = self.resolve_table_references(&statement)?; | ||
| let mut references = self.resolve_table_references(&statement)?; |
There was a problem hiding this comment.
this is a nice change to move it here into the session state
There was a problem hiding this comment.
Thanks, that gives me the feeling I'm heading in the right direction.
| schema: schema.into(), | ||
| table: table.into(), | ||
| fn resolve_info_table(&self, table: &str) -> Option<TableReference> { | ||
| let table_reference = if let Some(system_catalog) = |
There was a problem hiding this 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
There was a problem hiding this comment.
See other comment. Might be best to peek ahead at PR #26057 where this goes away entirely. In a nutshell, what I've done there is to change the planning of show ... statements so that they're not dependent on information_schema at all.
| { | ||
| TableReference::Full { | ||
| catalog: system_catalog.deref().into(), | ||
| schema: "information_schema".into(), |
There was a problem hiding this 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
There was a problem hiding this comment.
In this PR and more importantly in main, the references to information_schema, the various tables and their columns all exist here. As an example, see the show_tables_to_plan function on main.
I agree with you that the hardcoding of the information_schema table schema in the sql crate feels wrong. Getting rid of that is something I'm trying in followup PR #26057. This one is smaller in scope and only tries to land the work done in PR #24200 which seems to have stalled.
| WHERE table_schema <> 'information_schema'; | ||
| ---- | ||
| my_other_catalog my_other_schema t3 | ||
|
|
||
| query TTTT rowsort | ||
| SELECT * from information_schema.tables; |
There was a problem hiding this comment.
why don't all the other tables appear now? That seems ilke people would treat it as a regression 🤔
There was a problem hiding this comment.
What I've tried to implement in this PR is what's described at https://docs.databricks.com/aws/en/sql/language-manual/sql-ref-information-schema
In the SYSTEM catalog, the INFORMATION_SCHEMA is a SQL standard schema that provides metadata about objects across all catalogs in the metastore. It does not contain metadata about hive_metastore objects.
Separately, each catalog created in Unity Catalog also automatically includes an information_schema that describes metadata about objects in that catalog only.
The behaviour you're seeing here is that the default catalog was set to my_other_catalog. information_schema.tables then resolves to my_other_catalog.information_schema.tables which only describes the objects in my_other_catalog.
|
I think you're referring to PostgreSQL's schema search path. I'm not familiar with that, but will look into it. |
|
I spent some time surveying what's out there with AI to get a feel for our options. We'll need to make a decision on a couple of things:
Here's what I got from Claude wrt other some other systems out there
The current state of DataFusion is somewhere in between all the choices listed above: Given that a cross-catalog information schema currently exists, I think we at least need to retain that capability. The question then is how we expose that to the outside world. What I did in this PR is to introduce a new magic catalog named An alternative solution could be to add a search path capability for unqualified identifier resolution. By default, we would not expose the per-catalog information schema and add A hybrid solution where we special case the unqualified resolution of |
4 participants
Which issue does this PR close?
Rationale for this change
Catalog-qualified information schema queries currently return metadata from every registered catalog.
For example,
my_catalog.information_schema.tablesshould describe onlymy_catalog, consistent with PostgreSQL's information schema representing the current database.What changes are included in this PR?
InformationSchemaProvideris now constructed dynamically based on the catalog being queried. An optional 'system' catalog provides the existing global catalog behaviour. All other catalogs are restricted to just their own content.What is the testing strategy for this PR?
Added multi-catalog SQL logic coverage for qualified and unqualified information schema queries.
Are there any user-facing changes?
Yes. Information schema results are now scoped to the resolved catalog.