[pepijnve]

Which issue does this PR close?

• None logged yet

This PR builds on #25887 so that should be merged first.

Rationale for this change

LogicalPlan has a representation for the DESCRIBE TABLE statement, but not for the various SHOW ... statements. This makes it necessary to lower SHOW statements to equivalent SELECT statements against the information_schema tables during SQL statement to logical plan conversion. The code in question is located in the sql module's SqlToRel::sql_statement_to_plan function.

As an example, show_tables_to_plan is implemented as

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")
    }
}

The implementation is simple enough, but has the downside that it hardcodes references to tables and their columns using magic strings in a module that should be independent of specific schemas, tables, and views. The exact set of columns (and hence the output format) of the show statements is being determined in the sql module which doesn't seem to be where it belongs. This is a logical-to-physical translation aspect, not a sql-to-logical one.

To move the translation to a more appropriate location, this MR extends the logical plan data model to support show statements as well. Rather than introducing a new plan type, the existing describe table plan type is extended to support all statements belonging to the family of introspection statements.

The logical-to-physical translation of show statements is move over to the physical_planner module next to the existing implementation for describe table.

What changes are included in this PR?

LogicalPlan::DescribeTable has been made been renamed to LogicalPlan::Show and extended to support all show statements.

Handling of show statements has been moved from sql::statement to core::physical_planner.

The implementations themselves have been decoupled from the information_schema tables. This allows SHOW statements to work even if the information schema provider is not enabled.

A new common::information_schema module has been added to avoid string based duplication of the information_schema schema. In other words, duplicate magic strings have been removed.

Are these changes tested?

Covered by SLTs

Are there any user-facing changes?

Yes

• LogicalPlan::DescribeTable has been replaced with LogicalPlan::Show
• Availability of SHOW statements is no longer tied to the information_schema schema being accessible in the catalog
• information_schema.views no longer contains base tables

[github-actions]

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

     Cloning 985bbbf60e14644d8945dce9342f6dd20a428f1a
    Building datafusion v55.1.0 (current)
       Built [  52.691s] (current)
     Parsing datafusion v55.1.0 (current)
      Parsed [   0.031s] (current)
    Building datafusion v55.1.0 (baseline)
       Built [  16.067s] (baseline)
     Parsing datafusion v55.1.0 (baseline)
      Parsed [   0.032s] (baseline)
    Checking datafusion v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.710s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  71.496s] datafusion
    Building datafusion-catalog v55.1.0 (current)
       Built [  16.940s] (current)
     Parsing datafusion-catalog v55.1.0 (current)
      Parsed [   0.018s] (current)
    Building datafusion-catalog v55.1.0 (baseline)
       Built [  13.347s] (baseline)
     Parsing datafusion-catalog v55.1.0 (baseline)
      Parsed [   0.021s] (baseline)
    Checking datafusion-catalog v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.134s] 229 checks: 226 pass, 3 fail, 0 warn, 31 skip

--- failure function_missing: pub fn removed or renamed ---

Description:
A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/function_missing.ron

Failed in:
  function datafusion_catalog::information_schema::schemata_schema, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/catalog/src/information_schema.rs:1136

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/inherent_method_missing.ron

Failed in:
  InformationSchemaProvider::new_from_catalog, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/catalog/src/information_schema.rs:86
  InformationSchemaProvider::with_system_catalog, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/catalog/src/information_schema.rs:153

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/struct_missing.ron

Failed in:
  struct datafusion_catalog::information_schema::InformationSchemataBuilder, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/catalog/src/information_schema.rs:1179

     Summary semver requires new major version: 3 major and 0 minor checks failed
    Finished [  31.412s] datafusion-catalog
    Building datafusion-cli v55.1.0 (current)
       Built [  54.436s] (current)
     Parsing datafusion-cli v55.1.0 (current)
      Parsed [   0.028s] (current)
    Building datafusion-cli v55.1.0 (baseline)
       Built [  18.518s] (baseline)
     Parsing datafusion-cli v55.1.0 (baseline)
      Parsed [   0.030s] (baseline)
    Checking datafusion-cli v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.149s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  75.306s] datafusion-cli
    Building datafusion-common v55.1.0 (current)
       Built [  15.196s] (current)
     Parsing datafusion-common v55.1.0 (current)
      Parsed [   0.067s] (current)
    Building datafusion-common v55.1.0 (baseline)
       Built [   1.132s] (baseline)
     Parsing datafusion-common v55.1.0 (baseline)
      Parsed [   0.068s] (baseline)
    Checking datafusion-common v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.956s] 229 checks: 227 pass, 2 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.51.0/src/lints/constructible_struct_adds_field.ron

Failed in:
  field CatalogOptions.partition_information_schema in /home/runner/work/datafusion/datafusion/datafusion/common/src/config.rs:225

--- failure struct_pub_field_missing: pub struct's pub field removed or renamed ---

Description:
A publicly-visible struct has at least one public field that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/struct_pub_field_missing.ron

Failed in:
  field system_catalog of struct CatalogOptions, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/common/src/config.rs:225

     Summary semver requires new major version: 2 major and 0 minor checks failed
    Finished [  18.350s] datafusion-common
    Building datafusion-execution v55.1.0 (current)
       Built [   8.970s] (current)
     Parsing datafusion-execution v55.1.0 (current)
      Parsed [   0.026s] (current)
    Building datafusion-execution v55.1.0 (baseline)
       Built [   5.185s] (baseline)
     Parsing datafusion-execution v55.1.0 (baseline)
      Parsed [   0.027s] (baseline)
    Checking datafusion-execution v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.317s] 229 checks: 228 pass, 1 fail, 0 warn, 31 skip

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/inherent_method_missing.ron

Failed in:
  SessionConfig::system_catalog, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/execution/src/config.rs:233
  SessionConfig::with_system_catalog, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/execution/src/config.rs:325

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  15.545s] datafusion-execution
    Building datafusion-expr v55.1.0 (current)
       Built [  13.562s] (current)
     Parsing datafusion-expr v55.1.0 (current)
      Parsed [   0.096s] (current)
    Building datafusion-expr v55.1.0 (baseline)
       Built [   6.784s] (baseline)
     Parsing datafusion-expr v55.1.0 (baseline)
      Parsed [   0.096s] (baseline)
    Checking datafusion-expr v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   1.735s] 229 checks: 226 pass, 3 fail, 0 warn, 31 skip

--- failure enum_variant_added: enum variant added on exhaustive enum ---

Description:
A publicly-visible enum without #[non_exhaustive] has a new variant.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#enum-variant-new
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/enum_variant_added.ron

Failed in:
  variant LogicalPlan:Show in /home/runner/work/datafusion/datafusion/datafusion/expr/src/logical_plan/plan.rs:361
  variant LogicalPlan:Show in /home/runner/work/datafusion/datafusion/datafusion/expr/src/logical_plan/plan.rs:361

--- failure enum_variant_missing: pub enum variant removed or renamed ---

Description:
A publicly-visible enum has at least one variant that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/enum_variant_missing.ron

Failed in:
  variant LogicalPlan::DescribeTable, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/expr/src/logical_plan/plan.rs:359
  variant LogicalPlan::DescribeTable, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/expr/src/logical_plan/plan.rs:359

--- failure struct_missing: pub struct removed or renamed ---

Description:
A publicly-visible struct cannot be imported by its prior path. A `pub use` may have been removed, or the struct itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.51.0/src/lints/struct_missing.ron

Failed in:
  struct datafusion_expr::logical_plan::DescribeTable, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/expr/src/logical_plan/plan.rs:3809
  struct datafusion_expr::DescribeTable, previously in file /home/runner/work/datafusion/datafusion/target/semver-checks-shared/semver-checks/git-985bbbf60e14644d8945dce9342f6dd20a428f1a/9dfa4e6aaecd8732e7585b085bf0c80199c8ad3e/datafusion/expr/src/logical_plan/plan.rs:3809

     Summary semver requires new major version: 3 major and 0 minor checks failed
    Finished [  23.560s] datafusion-expr
    Building datafusion-optimizer v55.1.0 (current)
       Built [  15.528s] (current)
     Parsing datafusion-optimizer v55.1.0 (current)
      Parsed [   0.031s] (current)
    Building datafusion-optimizer v55.1.0 (baseline)
       Built [   6.973s] (baseline)
     Parsing datafusion-optimizer v55.1.0 (baseline)
      Parsed [   0.032s] (baseline)
    Checking datafusion-optimizer v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.191s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  23.458s] datafusion-optimizer
    Building datafusion-proto v55.1.0 (current)
       Built [  24.029s] (current)
     Parsing datafusion-proto v55.1.0 (current)
      Parsed [   0.011s] (current)
    Building datafusion-proto v55.1.0 (baseline)
       Built [  20.167s] (baseline)
     Parsing datafusion-proto v55.1.0 (baseline)
      Parsed [   0.012s] (baseline)
    Checking datafusion-proto v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.133s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  45.730s] datafusion-proto
    Building datafusion-sql v55.1.0 (current)
       Built [  11.525s] (current)
     Parsing datafusion-sql v55.1.0 (current)
      Parsed [   0.030s] (current)
    Building datafusion-sql v55.1.0 (baseline)
       Built [  11.593s] (baseline)
     Parsing datafusion-sql v55.1.0 (baseline)
      Parsed [   0.031s] (baseline)
    Checking datafusion-sql v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.297s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [  24.393s] datafusion-sql
    Building datafusion-substrait v55.1.0 (current)
       Built [ 254.110s] (current)
     Parsing datafusion-substrait v55.1.0 (current)
      Parsed [   0.010s] (current)
    Building datafusion-substrait v55.1.0 (baseline)
       Built [  16.938s] (baseline)
     Parsing datafusion-substrait v55.1.0 (baseline)
      Parsed [   0.011s] (baseline)
    Checking datafusion-substrait v55.1.0 -> v55.1.0 (no change; assume patch)
     Checked [   0.300s] 229 checks: 229 pass, 31 skip
     Summary no semver update required
    Finished [ 272.871s] datafusion-substrait

[alamb]

Thank you @pepijnve

I really like this PR and I think it moves the code into a better shape (and it has restored my enjoyment of reviewing PRs for today)

Also, thank you for the very nice explanation / description on this PR. It has been a while since I read a description of that length that wasn't AI vomit and was a pleasure to read rather than making my brain hurt.

I also removed some redundant rust tests to slim down the size of the PR

While reviewing this PR one thing I was wondering is why we need to special case the InformationTable views.

I wonder what you think about following the DynamicFileCatalog approach and adding InformationSchema support as a wrapper.

datafusion/datafusion/catalog/src/dynamic_file/catalog.rs

Line 27 in 02b7149

pub struct DynamicFileCatalog {

So basically it would be automatically added to any catalog registered, like here

datafusion/datafusion/core/src/execution/context/mod.rs

Lines 415 to 432 in 02b7149

	pub fn enable_url_table(self) -> Self {
	let current_catalog_list = Arc::clone(self.state.read().catalog_list());
	let factory = Arc::new(DynamicListTableFactory::new(SessionStore::new()));
	let catalog_list = Arc::new(DynamicFileCatalog::new(
	current_catalog_list,
	Arc::clone(&factory) as Arc<dyn UrlTableFactory>,
	));
	let session_id = self.session_id.clone();
	let ctx: SessionContext = self
	.into_state_builder()
	.with_session_id(session_id)
	.with_catalog_list(catalog_list)
	.build()
	.into();
	// register new state with the factory
	factory.session_store().with_state(ctx.state_weak_ref());
	ctx

SHOW TABLES would still have to remain a rewrite into

SELECT * FROM information_schema.tables

But after that it would be planned like any other table

And then we could just remove LogicalPlan::DescribeTables (rather than renaming it)

[alamb]

I like how much simpler this code is

[alamb]

it took me a while to understand why this stuff was moved to common, and then I realized it is because the LogicalPlan now depends on the information schema to report its schema 🤔

[pepijnve]

Indeed. I wanted to avoid duplicating the schema in multiple places. By keeping the schema itself and builders for the arrays together it's easier to modify these going forward.

Looking at this again, I think we could move it from common to expr. Everything else that needs this is dependent on that crate already as well.

[pepijnve]

Thanks for challenging the common location. I've done the move to expr; that makes more sense. A similar pattern could be applied for explain as well.

[pepijnve]

I wonder what you think about following the DynamicFileCatalog approach and adding InformationSchema support as a wrapper.

I'll need to give that some thought. In this PR I was just continuing on what was already there and didn't consider this alternative.

SHOW ... would still have to remain a rewrite into SELECT

I think that would reintroduce the need for the global information schema and move the translation back into sql/statement.rs using strings. SHOW TABLES is simple enough, but SHOW FUNCTIONS for instance is something that we can implement much simpler with a little bit of code.

By implementing the show statements as dedicated execution plans we gain the freedom to adjust those and the informations schema independently. Additionally, you get to still use those statements without needing to make the information schema visible in each catalog (i.e. the dependency on the config setting goes away).