alamb opened a new issue, #11529:
URL: https://github.com/apache/datafusion/issues/11529
### Describe the bug
If you execute `SHOW NONSENSE` via FlightSQL (or any other value after
`SHOW`) you get an empty result:
### To Reproduce
```sql
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
```sql
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
```sql
> 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
```rust
/// `{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)
```rust
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)
```rust
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",
--
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-unsubscr...@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
