You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "pepijnve (via GitHub)" <gi...@apache.org> on 2026/09/29 22:15:39 UTC
[PR] Information schema extension [datafusion]
pepijnve opened a new pull request, #25887:
URL: https://github.com/apache/datafusion/pull/25887
## Which issue does this PR close?
- Closes #15960.
- Supersedes PR #24200.
## Rationale for this change
Catalog-qualified information schema queries currently return metadata from every registered catalog.
For example, `my_catalog.information_schema.tables` should describe only `my_catalog`, consistent with PostgreSQL's information schema representing the current database.
## What changes are included in this PR?
- The catalog list provided to `InformationSchemaProvider` is now constructed dynamically based on the catalog being queries. An optional 'system' catalog provides the existing global catalog behaviour. All other catalogs are restricted to just their own content.
- Unqualified information_schema queries now use the configured default catalog instead of returning metadata from all catalogs.
## 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.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6351223195
> Would you prefer to split that part off? The information_schema schema itself is included in the information_schema views since they do exist and can describe themselves.
If it is easy I recommend we do, if not it is ok to keep here
> The view definition is a bit tricky to actually write though since these are implemented as virtual tables
Yeah I agree we shouldn't make up some SQL for a view that is dynamically created
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6384029647
I had missed the fact that there was a compile error when the `sql` feature is not enabled. Should be fixed now.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6350546837
> @alamb the most recent two commits change things so that the only remaining incompatible change is in the CLI where we'll need to decide what the default behaviour is. Should I revert that back to the current behaviour as well?
yes I think we should revert the CLI behavior back
I also tested again and found a few issues:
## EXPLAIN SHOW TABLES stopped working:
On main:
```sql
andrewlamb@Andrews-MacBook-Pro-3 ~ % datafusion-cli
DataFusion CLI v55.1.0
> explain show tables;
+---------------+-------------------------------+
| plan_type | plan |
+---------------+-------------------------------+
| physical_plan | ┌───────────────────────────┐ |
| | │ StreamingTableExec │ |
| | │ -------------------- │ |
| | │ limit: None │ |
| | └───────────────────────────┘ |
| | |
+---------------+-------------------------------+
1 row(s) fetched.
Elapsed 0.004 seconds.
```
This PR:
```sql
> EXPLAIN SHOW TABLES;
Error during planning: SHOW TABLES is not supported unless information_schema is enabled
> show datafusion.catalog.information_schema;
+---------------------------------------+-------+
| name | value |
+---------------------------------------+-------+
| datafusion.catalog.information_schema | true |
+---------------------------------------+-------+
1 row(s) fetched.
Elapsed 0.012 seconds.
```
## There are a bunch of new `NULL` views in information schema
```sql
> -- set up two catalogs with a table each
create catalog my_catalog;
create schema my_catalog.my_schema;
create table my_catalog.my_schema.t1 as values (1);
create view v1 as select 1; -- a real view in datafusion.public, so a non-NULL definition shows up
-- scoped to the default catalog (datafusion)
select table_catalog, table_schema, table_name, definition from information_schema.views;
```
Main:
```
+---------------+--------------+------------+----------------------------+
| table_catalog | table_schema | table_name | definition |
+---------------+--------------+------------+----------------------------+
| my_catalog | my_schema | t1 | NULL |
| datafusion | public | v1 | CREATE VIEW v1 AS SELECT 1 |
+---------------+--------------+------------+----------------------------+
```
This PR
```sql
+---------------+--------------------+-------------+----------------------------+
| table_catalog | table_schema | table_name | definition |
+---------------+--------------------+-------------+----------------------------+
| datafusion | public | v1 | CREATE VIEW v1 AS SELECT 1 |
| datafusion | information_schema | tables | NULL |
| datafusion | information_schema | views | NULL |
| datafusion | information_schema | columns | NULL |
| datafusion | information_schema | df_settings | NULL |
| datafusion | information_schema | schemata | NULL |
| datafusion | information_schema | routines | NULL |
| datafusion | information_schema | parameters | NULL |
+---------------+--------------------+-------------+----------------------------+
8 row(s) fetched.
Elapsed 0.005 seconds.
```
## Change (in a good way)
Also, I think on main this sql query runs (weirdly as `no_such_catalog` does not exist)
however, on this PR it errors (which seems better)
```sql
-- Querying the information_schema of a catalog that does not exist
-- main: returns the global listing (catalog name is ignored)
-- PR #25887: Error during planning: table 'no_such_catalog.information_schema.tables' not found
create catalog my_catalog;
create schema my_catalog.my_schema;
create table my_catalog.my_schema.t1 as values (1);
select table_catalog, table_schema, table_name
from no_such_catalog.information_schema.tables
where table_schema <> 'information_schema';
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4353496058
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -672,19 +673,8 @@ impl SessionState {
statement: Statement,
) -> datafusion_common::Result<LogicalPlan> {
let mut references = self.resolve_table_references(&statement)?;
- let require_info_schema = match &statement {
- Statement::Statement(s) => matches!(
- s.as_ref(),
- SQLStatement::ShowCreate { .. }
- | SQLStatement::ShowTables { .. }
- | SQLStatement::ShowColumns { .. }
- | SQLStatement::ShowVariable { .. }
- | SQLStatement::ShowFunctions { .. }
- ),
- _ => false,
- };
- if require_info_schema {
+ if Self::requires_information_schema(&statement) {
Review Comment:
👍
##########
datafusion/sql/src/resolve.rs:
##########
@@ -25,28 +25,6 @@ use crate::planner::object_name_to_table_reference;
use datafusion_common::TableReference;
use sqlparser::ast::*;
-// following constants are used in `resolve_table_references`
Review Comment:
it is really nice to remove this stuff out of the sql module 💯 as it shouldn't really be in the sql parser
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-5969899845
@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.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4300283083
##########
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:
I was curious about how this was theoretically supposed to work per the SQL specification. In ISO 9075-11:2023 at least, the information schema views are indeed segregated per catalog. There's a theoretical notion of a 'definition schema' containing base tables which are allowed to contain everything, and then the information schema view are defined in terms of those.
First we have
```
CREATE VIEW INFORMATION_SCHEMA_CATALOG_NAME AS
SELECT CATALOG_NAME
FROM DEFINITION_SCHEMA.CATALOG_NAME
WHERE CATALOG_NAME = 'CN';
```
where `CN` is the name of the catalog in which this Information Schema resides.
The tables view is defined as (shortened for legibility)
```
CREATE VIEW TABLES AS
SELECT TABLE_CATALOG, TABLE_SCHEMA, TABLE_NAME, TABLE_TYPE, ...
FROM DEFINITION_SCHEMA.TABLES
WHERE ...
AND
TABLE_CATALOG
= ( SELECT CATALOG_NAME
FROM INFORMATION_SCHEMA_CATALOG_NAME );
```
The last predicate is what restricts the set of tables to only those that exist in the same catalog as the information schema itself. I did not check all of them exhaustively, but the same seems to hold for most of the other object views as well.
This is of course just the standards view on things. Logging this here as FYI.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6249513105
Sorry -- this PR is on my list to review / respond to mroe carefully. I will try and do so soon
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-5969795389
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25887?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `78.26087%` with `40 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 82.63%. Comparing base ([`b9c8b3d`](https://app.codecov.io/gh/apache/datafusion/commit/b9c8b3dce8038372c24a48525d2f1fc0e81c5e83?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`1e5b40e`](https://app.codecov.io/gh/apache/datafusion/commit/1e5b40e3c05cd1e6ab277e01a4d0b5a3f61a8473?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 59 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/25887?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/catalog/src/information\_schema.rs](https://app.codecov.io/gh/apache/datafusion/pull/25887?src=pr&el=tree&filepath=datafusion%2Fcatalog%2Fsrc%2Finformation_schema.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9jYXRhbG9nL3NyYy9pbmZvcm1hdGlvbl9zY2hlbWEucnM=) | 77.90% | [13 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/core/src/execution/session\_state.rs](https://app.codecov.io/gh/apache/datafusion/pull/25887?src=pr&el=tree&filepath=datafusion%2Fcore%2Fsrc%2Fexecution%2Fsession_state.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9jb3JlL3NyYy9leGVjdXRpb24vc2Vzc2lvbl9zdGF0ZS5ycw==) | 79.68% | [12 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/sql/src/statement.rs](https://app.codecov.io/gh/apache/datafusion/pull/25887?src=pr&el=tree&filepath=datafusion%2Fsql%2Fsrc%2Fstatement.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9zcWwvc3JjL3N0YXRlbWVudC5ycw==) | 65.21% | [6 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25887?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ 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
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25887?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6010760875
I think you're referring to PostgreSQL's [schema search path](https://www.postgresql.org/docs/current/ddl-schemas.html#DDL-SCHEMAS-PATH). I'm not familiar with that, but will look into 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6351567863
From the spec
> 2) Case:
a) If the character representation of the user-specified <query expression> contained in the corresponding view descriptor can be represented without truncation, then the value of VIEW_DEFINITION is that character representation.
b) Otherwise, the value of VIEW_DEFINITION is the null value.
Still compliant if we claim that the definition is so long that it would need to be truncated 😄
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4367232833
##########
datafusion/sql/src/statement.rs:
##########
@@ -1675,14 +1676,16 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
/// Generate a logical plan from a "SHOW TABLES" query
fn show_tables_to_plan(&self) -> Result<LogicalPlan> {
- if self.has_table("information_schema", "tables") {
- let query = "SELECT * FROM information_schema.tables;";
- let mut rewrite = DFParser::parse_sql(query)?;
- assert_eq!(rewrite.len(), 1);
- self.statement_to_plan(rewrite.pop_front().unwrap()) // length of rewrite is 1
- } else {
- plan_err!("SHOW TABLES is not supported unless information_schema is enabled")
- }
+ let Some(tables_table_ref) = self.resolve_info_table("tables") else {
+ return plan_err!(
+ "SHOW TABLES is not supported unless information_schema is enabled"
+ );
+ };
+
+ let query = format!("SELECT * FROM {tables_table_ref};");
Review Comment:
Sure!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6291636449
@alamb the most recent two commits change things so that the only remaining incompatible change is in the CLI where we'll need to decide what the default behaviour is. Should I revert that back to the current behaviour as well?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4361837568
##########
datafusion/sql/src/resolve.rs:
##########
@@ -25,28 +25,6 @@ use crate::planner::object_name_to_table_reference;
use datafusion_common::TableReference;
use sqlparser::ast::*;
-// following constants are used in `resolve_table_references`
Review Comment:
You're going to like my next PR 😄
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6403163037
Thanks @pepijnve and @martin-g -- let's do it!
Iw ould like to avoid publishing 56 with a new API that is immediately removed.
Can you post what PRs you think we need to merge before we ship 56 on this ticket?
- https://github.com/apache/datafusion/issues/24461
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4367052601
##########
datafusion/sql/src/statement.rs:
##########
@@ -3135,11 +3137,12 @@ FROM (
&self,
sql_table_name: ObjectName,
) -> Result<LogicalPlan> {
- if !self.has_table("information_schema", "tables") {
+ let Some(tables_table_ref) = self.resolve_info_table("views") else {
Review Comment:
nit:
```suggestion
let Some(views_table_ref) = self.resolve_info_table("views") else {
```
to be clearer what it is about
##########
datafusion/catalog/src/information_schema.rs:
##########
@@ -81,10 +82,55 @@ pub struct InformationSchemaProvider {
}
impl InformationSchemaProvider {
+ /// Creates a new [`InformationSchemaProvider`] for the provided `catalog_list`
Review Comment:
```suggestion
/// Creates a new [`InformationSchemaProvider`] for the provided `catalog`
```
##########
datafusion/catalog/src/information_schema.rs:
##########
@@ -150,6 +202,17 @@ impl InformationSchemaConfig {
}
}
+ if let Some(system_catalog_name) = &self.system_catalog_name {
+ for table_name in INFORMATION_SCHEMA_TABLES {
+ builder.add_table(
Review Comment:
according to https://github.com/apache/datafusion/pull/25887/changes#diff-8303a17e3c522b51c73c7fb0474b775746730873fc953e0c52c3fb7e67e84d5bR319 the system catalog could be a normal catalog this will add the same tables second time, no ?
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -87,13 +89,16 @@ use datafusion_sql::{
use async_trait::async_trait;
use chrono::{DateTime, Utc};
+use datafusion_sql::parser::ExplainStatement;
Review Comment:
line 86 imports `Statement` conditionally: `#[cfg(feature = "sql")]`
does this import need the same ?
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -645,6 +711,24 @@ impl SessionState {
query.statement_to_plan(statement)
}
+ #[cfg(feature = "sql")]
+ fn requires_information_schema(statement: &Statement) -> bool {
+ match &statement {
+ Statement::Explain(ExplainStatement { statement, .. }) => {
+ Self::requires_information_schema(statement)
+ }
+ Statement::Statement(s) => matches!(
+ s.as_ref(),
+ SQLStatement::ShowCreate { .. }
+ | SQLStatement::ShowTables { .. }
+ | SQLStatement::ShowColumns { .. }
+ | SQLStatement::ShowVariable { .. }
+ | SQLStatement::ShowFunctions { .. }
Review Comment:
ShowStatus, ShowVariables and ShowCollation are no more matched. Intentional ?
##########
datafusion/sql/src/statement.rs:
##########
@@ -1675,14 +1676,16 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
/// Generate a logical plan from a "SHOW TABLES" query
fn show_tables_to_plan(&self) -> Result<LogicalPlan> {
- if self.has_table("information_schema", "tables") {
- let query = "SELECT * FROM information_schema.tables;";
- let mut rewrite = DFParser::parse_sql(query)?;
- assert_eq!(rewrite.len(), 1);
- self.statement_to_plan(rewrite.pop_front().unwrap()) // length of rewrite is 1
- } else {
- plan_err!("SHOW TABLES is not supported unless information_schema is enabled")
- }
+ let Some(tables_table_ref) = self.resolve_info_table("tables") else {
+ return plan_err!(
+ "SHOW TABLES is not supported unless information_schema is enabled"
+ );
+ };
+
+ let query = format!("SELECT * FROM {tables_table_ref};");
Review Comment:
```suggestion
let query = format!("SELECT * FROM {};", tables_table_ref.to_quoted_string());
```
quote that table name to support system catalog names like `System`
##########
datafusion/sql/src/statement.rs:
##########
Review Comment:
resolve_info_table("routines") ?!
##########
datafusion/sql/src/statement.rs:
##########
Review Comment:
shouldn't this use `resolve_info_table("parameters")` to resolve the correct catalog+schema+table ?
##########
datafusion/sql/src/statement.rs:
##########
Review Comment:
same 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4192408374
##########
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:
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. That makes this resolution method disappear entirely.
##########
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:
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.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6149045403
@alamb sorry to bother you, but if you have a moment could you let me know which direction seems the most promising to you? I'm happy to continue refining this PR, but not entirely sure what an acceptable end goal looks like.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6384139476
Is there a simple way to run the equivalent of `cargo check datafusion features` locally? I want to make sure I verify this proactively for the next PR.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #25887:
URL: https://github.com/apache/datafusion/pull/25887
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4192312974
##########
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:
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.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6351855578
> > Would you prefer to split that part off? The information_schema schema itself is included in the information_schema views since they do exist and can describe themselves.
>
> If it is easy I recommend we do, if not it is ok to keep here
Just removing ~10 lines of code. All done.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4367131061
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -87,13 +89,16 @@ use datafusion_sql::{
use async_trait::async_trait;
use chrono::{DateTime, Utc};
+use datafusion_sql::parser::ExplainStatement;
Review Comment:
This one has been resolved already. Still working on figuring out what the appropriate behaviour is for one more block that needs a config directive.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6395929392
@alamb while working through @martin-g's review comments I came to the realisation that in the followup #26057 PR we can drop the 'system catalog' concept entirely. That was only really necessary to support a partitioned information schema at the table level, but still allow the `SHOW` statements to report global results. In the referenced PR, the `SHOW` statements are no longer implemented as SQL queries so the need disappears.
I've gone ahead and removed the concept in #26057. This PR is then an intermediate step. I'm not sure what the best approach is here. On the one hand I think it would be easiest from a review point-of-view to merge this work already, and then make sure that the other PR is merged before the next release ships. On the other hand, that does put some timing pressure on the review. If we want to avoid that it might be better to consolidate the two PRs and merge the work in one go. Wdyt?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4370309254
##########
datafusion/sql/src/statement.rs:
##########
@@ -1675,14 +1676,16 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
/// Generate a logical plan from a "SHOW TABLES" query
fn show_tables_to_plan(&self) -> Result<LogicalPlan> {
- if self.has_table("information_schema", "tables") {
- let query = "SELECT * FROM information_schema.tables;";
- let mut rewrite = DFParser::parse_sql(query)?;
- assert_eq!(rewrite.len(), 1);
- self.statement_to_plan(rewrite.pop_front().unwrap()) // length of rewrite is 1
- } else {
- plan_err!("SHOW TABLES is not supported unless information_schema is enabled")
- }
+ let Some(tables_table_ref) = self.resolve_info_table("tables") else {
+ return plan_err!(
+ "SHOW TABLES is not supported unless information_schema is enabled"
+ );
+ };
+
+ let query = format!("SELECT * FROM {tables_table_ref};");
Review Comment:
I've polished #26057 and I think it's now in good enough shape for review. Leaving it marked as draft for now since it builds on the work done here and GitHub can't do cross fork stacked PRs yet.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4192387307
##########
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:
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`.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-5900341776
<!-- semver-check-comment -->
Thank you for opening this pull request!
Reviewer note: [cargo-semver-checks](https://github.com/obi1kenobi/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>
<summary>Details</summary>
```
Cloning apache/main
Building datafusion v55.1.0 (current)
Built [ 63.650s] (current)
Parsing datafusion v55.1.0 (current)
Parsed [ 0.039s] (current)
Building datafusion v55.1.0 (baseline)
Built [ 61.669s] (baseline)
Parsing datafusion v55.1.0 (baseline)
Parsed [ 0.040s] (baseline)
Checking datafusion v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 0.978s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 128.633s] datafusion
Building datafusion-catalog v55.1.0 (current)
Built [ 42.565s] (current)
Parsing datafusion-catalog v55.1.0 (current)
Parsed [ 0.027s] (current)
Building datafusion-catalog v55.1.0 (baseline)
Built [ 42.304s] (baseline)
Parsing datafusion-catalog v55.1.0 (baseline)
Parsed [ 0.028s] (baseline)
Checking datafusion-catalog v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 0.163s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 86.353s] datafusion-catalog
Building datafusion-cli v55.1.0 (current)
Built [ 98.760s] (current)
Parsing datafusion-cli v55.1.0 (current)
Parsed [ 0.037s] (current)
Building datafusion-cli v55.1.0 (baseline)
Built [ 101.075s] (baseline)
Parsing datafusion-cli v55.1.0 (baseline)
Parsed [ 0.039s] (baseline)
Checking datafusion-cli v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 0.173s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 203.324s] datafusion-cli
Building datafusion-common v55.1.0 (current)
Built [ 35.426s] (current)
Parsing datafusion-common v55.1.0 (current)
Parsed [ 0.070s] (current)
Building datafusion-common v55.1.0 (baseline)
Built [ 35.569s] (baseline)
Parsing datafusion-common v55.1.0 (baseline)
Parsed [ 0.072s] (baseline)
Checking datafusion-common v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 1.139s] 223 checks: 222 pass, 1 fail, 0 warn, 31 skip
--- failure constructible_struct_adds_field: struct exhaustively constructible through public API adds field ---
Description:
A pub struct that could be exhaustively constructed with a literal using only public API has a new pub field, breaking existing exhaustive literals.
ref: https://doc.rust-lang.org/reference/expressions/struct-expr.html
impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.50.0/src/lints/constructible_struct_adds_field.ron
Failed in:
field CatalogOptions.system_catalog in /home/runner/work/datafusion/datafusion/datafusion/common/src/config.rs:224
Summary semver requires new major version: 1 major and 0 minor checks failed
Finished [ 73.702s] datafusion-common
Building datafusion-execution v55.1.0 (current)
Built [ 31.746s] (current)
Parsing datafusion-execution v55.1.0 (current)
Parsed [ 0.029s] (current)
Building datafusion-execution v55.1.0 (baseline)
Built [ 31.889s] (baseline)
Parsing datafusion-execution v55.1.0 (baseline)
Parsed [ 0.031s] (baseline)
Checking datafusion-execution v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 0.370s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 65.562s] datafusion-execution
Building datafusion-sql v55.1.0 (current)
Built [ 45.791s] (current)
Parsing datafusion-sql v55.1.0 (current)
Parsed [ 0.037s] (current)
Building datafusion-sql v55.1.0 (baseline)
Built [ 46.249s] (baseline)
Parsing datafusion-sql v55.1.0 (baseline)
Parsed [ 0.039s] (baseline)
Checking datafusion-sql v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 0.336s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 94.082s] datafusion-sql
Building datafusion-sqllogictest v55.1.0 (current)
Built [ 107.398s] (current)
Parsing datafusion-sqllogictest v55.1.0 (current)
Parsed [ 0.025s] (current)
Building datafusion-sqllogictest v55.1.0 (baseline)
Built [ 104.913s] (baseline)
Parsing datafusion-sqllogictest v55.1.0 (baseline)
Parsed [ 0.025s] (baseline)
Checking datafusion-sqllogictest v55.1.0 -> v55.1.0 (no change; assume patch)
Checked [ 0.135s] 223 checks: 223 pass, 31 skip
Summary no semver update required
Finished [ 215.386s] datafusion-sqllogictest
```
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6277046161
> A hybrid solution where we special case the unqualified resolution of information_schema is also an option.
Perthaps you mean here some way to preserve the existing behavior
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6290696429
> > A hybrid solution where we special case the unqualified resolution of information_schema is also an option.
>
> Perthaps you mean here some way to preserve the existing behavior
Indeed. What I had in mind was that we would use the 'system catalog' configuration option for this. If it's undefined, then we stick to the current behaviour of every catalog containing being an identical, global one. If it is defined, then we make the per catalog instances so that they only describe objects in the same catalog and the global one provides the cross catalog variant. This way the change becomes opt-in.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4367140957
##########
datafusion/core/src/execution/session_state.rs:
##########
@@ -645,6 +711,24 @@ impl SessionState {
query.statement_to_plan(statement)
}
+ #[cfg(feature = "sql")]
+ fn requires_information_schema(statement: &Statement) -> bool {
+ match &statement {
+ Statement::Explain(ExplainStatement { statement, .. }) => {
+ Self::requires_information_schema(statement)
+ }
+ Statement::Statement(s) => matches!(
+ s.as_ref(),
+ SQLStatement::ShowCreate { .. }
+ | SQLStatement::ShowTables { .. }
+ | SQLStatement::ShowColumns { .. }
+ | SQLStatement::ShowVariable { .. }
+ | SQLStatement::ShowFunctions { .. }
Review Comment:
Yes, these don't actually have a backing implementation. Next PR will clean all this up properly.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4368363868
##########
datafusion/catalog/src/information_schema.rs:
##########
@@ -150,6 +202,17 @@ impl InformationSchemaConfig {
}
}
+ if let Some(system_catalog_name) = &self.system_catalog_name {
+ for table_name in INFORMATION_SCHEMA_TABLES {
+ builder.add_table(
Review Comment:
I've added an extra guard to avoid duplicates. I think I might reconsider this 'system catalog' idea in the followup where all the SQL stuff disappears. I can get away with a simpler global/partitioned toggle there.
##########
datafusion/sql/src/statement.rs:
##########
Review Comment:
Fixed
##########
datafusion/sql/src/statement.rs:
##########
Review Comment:
Fixed
##########
datafusion/sql/src/statement.rs:
##########
Review Comment:
Fixed
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6351305389
FYI, checking what 55.2.0 does reminded me of why I had fixed this. Resolves the inconsistency where querying `tables` lists a number of views, but querying `views` returns an empty set.
```
pepijn@Flandrien T % datafusion-cli
DataFusion CLI v55.2.0
> select * from information_schema.tables;
+---------------+--------------------+-------------+------------+
| table_catalog | table_schema | table_name | table_type |
+---------------+--------------------+-------------+------------+
| datafusion | information_schema | tables | VIEW |
| datafusion | information_schema | views | VIEW |
| datafusion | information_schema | columns | VIEW |
| datafusion | information_schema | df_settings | VIEW |
| datafusion | information_schema | schemata | VIEW |
| datafusion | information_schema | routines | VIEW |
| datafusion | information_schema | parameters | VIEW |
+---------------+--------------------+-------------+------------+
7 row(s) fetched.
Elapsed 0.002 seconds.
> select * from information_schema.views;
+---------------+--------------+------------+------------+
| table_catalog | table_schema | table_name | definition |
+---------------+--------------+------------+------------+
+---------------+--------------+------------+------------+
0 row(s) fetched.
Elapsed 0.003 seconds.
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6015448009
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:
1. Should `information_schema` be global or per catalog?
2. If global, which catalog is it a child of?
3. If per catalog, what does an unqualified usage of `information_schema` resolve to?
Here's what I got from Claude wrt other some other systems out there
| System | Global? | Where it lives? | Unqualified? |
|---|---|---|---|
| DuckDB | Yes, one view covers all attached databases | Once, in the `system` catalog | `system.information_schema` |
| SQL Server | No; query `db.INFORMATION_SCHEMA.TABLES` per database, or use `sys.databases` plus dynamic SQL | Each database | Current database (`USE`) |
| Snowflake | No; `SNOWFLAKE.ACCOUNT_USAGE` views cover the account (with latency) | Each database | Current database; errors if none is set |
| Trino | No; `system.jdbc.*` and `system.metadata.*` cover all catalogs | Each catalog (generated per connector) | Session catalog; errors if none is set |
| Databricks (Unity Catalog) | Opt-in via `system.information_schema` | Each catalog, plus `system.information_schema` | Current catalog (`USE CATALOG`) |
| BigQuery | Region-qualified, e.g. `` `region-us`.INFORMATION_SCHEMA.TABLES `` | Per dataset or region, as a qualifier | Generally errors without a qualifier |
| MySQL / MariaDB | Yes, but "databases" are really schemas; `table_catalog` is always `def` | Once, global | The global schema |
| SQLite | No; query `aux.sqlite_schema` per attachment | None; each attached DB has `sqlite_schema` | `main.sqlite_schema` |
| StarRocks | No; query `catalog.information_schema.tables` per catalog (external catalogs supported from v3.2) | Each catalog (`default_catalog` and each external catalog) | Current catalog (`SET CATALOG`), `default_catalog` by default |
The current state of DataFusion is somewhere in between all the choices listed above: `information_schema` lives in each catalog which suggests it's partitioned per catalog, but each 'instance' is identical and is actually global/cross-catalog. This becomes particularly annoying when you're trying to work with remote catalogs. It doesn't seem desirable for a query that looks local to start accessing remote resources (e.g., I try to query the memory catalog via the information schema, and it starts accessing my Iceberg REST catalog over the internet).
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 `system` that holds the global information schema. Because DataFusion only has a single default catalog value at the moment rather than a PostgreSQL style search path, you need to address this explicitly.
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 `system` to the end of the search path by default. This would still introduce a behaviour change though for info schema queries in qualified form (i.e. `select * from datafusion.information_schema.tables` would not work).
A hybrid where there are special resolution rules for `information_schema` specifically doesn't seem like a good idea to me.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Information schema extension [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4192332314
##########
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:
Thanks, that gives me the feeling I'm heading in the right direction.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6350881243
> yes I think we should revert the CLI behavior back
Will do
> EXPLAIN SHOW TABLES stopped working
I'll look into that. I'll start by adding SLTs to cover the behaviour.
> There are a bunch of new NULL views in information schema
Would you prefer to split that part off? The information_schema schema itself is included in the information_schema views since they do exist and can describe themselves. The view definition is a bit tricky to actually write though since these are implemented as virtual tables. We can fill in a pseudo definition if that's preferred over the null value, but it won't be SQL you can actually execute. We would have to describe what the underlying definition schema is then.
Example from postgresql
```
table_catalog | table_schema | table_name | view_definition
---------------+--------------------+---------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
pepijn | information_schema | schemata | SELECT (current_database())::information_schema.sql_identifier AS catalog_name, +
| | | (n.nspname)::information_schema.sql_identifier AS schema_name, +
| | | (u.rolname)::information_schema.sql_identifier AS schema_owner, +
| | | (NULL::name)::information_schema.sql_identifier AS default_character_set_catalog, +
| | | (NULL::name)::information_schema.sql_identifier AS default_character_set_schema, +
| | | (NULL::name)::information_schema.sql_identifier AS default_character_set_name, +
| | | (NULL::character varying)::information_schema.character_data AS sql_path +
| | | FROM pg_namespace n, +
| | | pg_authid u +
| | | WHERE ((n.nspowner = u.oid) AND (pg_has_role(n.nspowner, 'USAGE'::text) OR has_schema_privilege(n.oid, 'CREATE, USAGE'::text)));
```
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Partition information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6267861335
@neilconway since you were reviewing the original version of this PR, I was wondering if you had any opinions on the global vs segregated information schema question above.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6384243705
Still some compilation issues when `sql` is disabled. I'll figure out how to test this locally, and tackle @martin-g's review comments while I'm at 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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4367123770
##########
datafusion/sql/src/statement.rs:
##########
@@ -1675,14 +1676,16 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
/// Generate a logical plan from a "SHOW TABLES" query
fn show_tables_to_plan(&self) -> Result<LogicalPlan> {
- if self.has_table("information_schema", "tables") {
- let query = "SELECT * FROM information_schema.tables;";
- let mut rewrite = DFParser::parse_sql(query)?;
- assert_eq!(rewrite.len(), 1);
- self.statement_to_plan(rewrite.pop_front().unwrap()) // length of rewrite is 1
- } else {
- plan_err!("SHOW TABLES is not supported unless information_schema is enabled")
- }
+ let Some(tables_table_ref) = self.resolve_info_table("tables") else {
+ return plan_err!(
+ "SHOW TABLES is not supported unless information_schema is enabled"
+ );
+ };
+
+ let query = format!("SELECT * FROM {tables_table_ref};");
Review Comment:
Is it ok if I handle this via the followup PR that removes the SQL composition altogether?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on code in PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#discussion_r4368358940
##########
datafusion/catalog/src/information_schema.rs:
##########
@@ -81,10 +82,55 @@ pub struct InformationSchemaProvider {
}
impl InformationSchemaProvider {
+ /// Creates a new [`InformationSchemaProvider`] for the provided `catalog_list`
Review Comment:
Fixed
##########
datafusion/sql/src/statement.rs:
##########
@@ -3135,11 +3137,12 @@ FROM (
&self,
sql_table_name: ObjectName,
) -> Result<LogicalPlan> {
- if !self.has_table("information_schema", "tables") {
+ let Some(tables_table_ref) = self.resolve_info_table("views") else {
Review Comment:
Fixed
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Support partitioning of information schema content per catalog [datafusion]
Posted by "pepijnve (via GitHub)" <gi...@apache.org>.
pepijnve commented on PR #25887:
URL: https://github.com/apache/datafusion/pull/25887#issuecomment-6389057149
@alamb I think we're good to go now. I ran `cargo check --no-default-features -p datafusion` as local test and that passed.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org