Skip to content

SHOW NONSENSE does not error #11529

@alamb

Description

@alamb

Describe the bug

If you execute SHOW NONSENSE via FlightSQL (or any other value after SHOW) you get an empty result:

To Reproduce

DataFusion CLI v40.0.0
> show nonsense;
+------+-------+
| name | value |
+------+-------+
+------+-------+
0 row(s) fetched.
Elapsed 0.054 seconds.

Expected behavior

We expect the query to error/fail

However, other values like SHOW ALL should continue to work, for example

DataFusion CLI v40.0.0
> show all;
+-------------------------------------------------------------------------+---------------------------+
| name                                                                    | value                     |
+-------------------------------------------------------------------------+---------------------------+
| datafusion.catalog.create_default_catalog_and_schema                    | true                      |
| datafusion.catalog.default_catalog                                      | datafusion                |
| datafusion.catalog.default_schema                                       | public                    |
| datafusion.catalog.format                                               |                           |
| datafusion.catalog.has_header                                           | false                     |
| datafusion.catalog.information_schema                                   | true                      |
| datafusion.catalog.location                                             |                           |
| datafusion.execution.aggregate.scalar_update_factor                     | 10                        |
| datafusion.execution.batch_size                                         | 8192                      |
| datafusion.execution.coalesce_batches                                   | true                      |
| datafusion.execution.collect_statistics                                 | false                     |
| datafusion.execution.enable_recursive_ctes                              | true                      |

And

> show datafusion.catalog.format ;
+---------------------------+-------+
| name                      | value |
+---------------------------+-------+
| datafusion.catalog.format |       |
+---------------------------+-------+
1 row(s) fetched.
Elapsed 0.006 seconds.

Additional context

This is based on a code / report filed by @crepererum and @itsjunetime

    /// `{variable}` is not a valid config value and not a special-cased value, like `all`. If this
    /// statement does contain such a statement, return an error.
    fn check_stmt_for_invalid_show(state: &SessionState, stmt: &Statement) -> Result<()> {
      // these are the special cases which, if added after a `SHOW` command
        static SHOW_SPECIAL_CASES: [&str; 3] = ["all", "timezone", "time.zone"];

        match stmt {
            Statement::Statement(query) => match **query {
                SQLStatement::ShowVariable { variable: ref vars } => {
                    // if they specified `verbose` as the last argument, that's also allowed and
                    // special-cased. Once again, pulled from `show_variable_to_plan()`.
                    let vars = if vars.last().is_some_and(|id| id.value == "verbose") {
                        &vars[..vars.len() - 1]
                    } else {
                        vars.as_slice()
                    };

                    // if someone inputs something like datafusion.configuration.something, `vars`
                    // is `["datafusion", "configuration", "something"]`. For some reason, it's
                    // split out (it seems that you could also enter it like `SHOW datafusion
                    // contiruation something` and that would be valid as well, based on
                    // `show_variable_to_plan()`) and we need to join it back together to get the
                    // whole variable that we want to check against the valid options
                    let idents_str = vars
                        .iter()
                        .map(|id| id.to_string())
                        .collect::<Vec<_>>()
                        .join(".");

                    // try to find the first variable that doesn't exist and isn't special-cased
                    // for convenience
                    let is_invalid = !state
                        // config_options is used to create the information_schema table, so that's
                        // why we're using it here to check against the requested variables.
                        .config_options()
                        .entries()
                        .iter()
                        .map(|opt| opt.key.as_str())
                        .chain(SHOW_SPECIAL_CASES)
                        .any(|var| var == idents_str);

                    if is_invalid {
                        Err(plan_datafusion_err!("Cannot show variable '{idents_str}' as it is not a valid configuration value"))
                    } else {
                        Ok(())
                    }
                }
                _ => Ok(()),
            },
            _ => Ok(()),
        }
    }

Here are some error cases (should be in slt in datafusion tests)

        assert_eq!(
            planner.sql("SHOW NONSENSE", StatementParams::default()).await.unwrap_err().strip_backtrace(),
            "Error during planning: Cannot show variable 'NONSENSE' as it is not a valid configuration value"
        );

        assert_eq!(
            planner.sql("SHOW whatever", StatementParams::default()).await.unwrap_err().strip_backtrace(),
            "Error during planning: Cannot show variable 'whatever' as it is not a valid configuration value"
        );

        assert_eq!(
            planner.sql("SHOW still invalid verbose", StatementParams::default()).await.unwrap_err().strip_backtrace(),
            "Error during planning: Cannot show variable 'still.invalid' as it is not a valid configuration value"
        );
  // message is kind of confusing. Should we special-case this error case too?
        assert_eq!(
            planner.sql("SHOW verbose", StatementParams::default()).await.unwrap_err().strip_backtrace(),
            "Error during planning: Cannot show variable '' as it is not a valid configuration value"
        );
    }

Here are some postitive cases (may already be covered by slt)

       assert!(planner
            .sql("SHOW all", StatementParams::default())
            .await
            .is_ok());
        assert!(planner
            .sql("SHOW all verbose", StatementParams::default())
            .await
            .is_ok());
        assert!(planner
            .sql(
                "SHOW datafusion.catalog.location",

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions