-
Couldn't load subscription status.
- Fork 1.7k
Support SHOW ALL VERBOSE to show settings description
#7735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @comphead -- I really like that the descriptions are now included.
I think the change to make it possible to select all the settings directly from df_settings is great 💯
select * from information_schema.df_settings
As you mention, I am not so sure about including the description in SHOW <name> and SHOW ALL as it makes the output much more verbose.
I think it would be a nicer experience if SHOW ALL kept the current behavior and we extended SHOW ALL VERBOSE and SHOW <value> VERBOSE that added the description too.
I checked and this appears to be supported by the parser so we could probably make this work without waiting on an upstream change
❯ show all verbose;
0 rows in set. Query took 0.002 seconds.
What do you think?
|
@alamb thanks. My concern with the first sentence as the developer who introducing the new param is not aware about concise first sentence requirements and this will still eventually lead to the wide output. Having another verbose pair makes much more sense for me. |
I agree |
# Conflicts: # datafusion/sqllogictest/test_files/information_schema.slt
Added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really nice to me -- thank you @comphead
| datafusion.sql_parser.parse_float_as_decimal false | ||
|
|
||
| # show all variables with verbose | ||
| query TTT rowsort |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥇
|
|
||
| +-------------------------------------------------+---------+ | ||
| | name | setting | | ||
| | name | value | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change in column name 👍
|
cc @waitingkuo |
SHOW ALL VERBOSE to show settings description
|
Thanks again @comphead -- this is a really nice contribution |
Which issue does this PR close?
Closes #7736 .
Rationale for this change
Expand SHOW ALL stmt to show settings description. Currently it requires going through the code to find the description for the specific datafusion setting, moreover code covered by macros which makes harder to understand the full setting name.
What changes are included in this PR?
The PR is to add settings description to the
SHOW ALLoutputAre these changes tested?
Yes
Are there any user-facing changes?
Yes, the
SHOW ALLoutput have extradescriptioncolumn and confusingsettingscolumn name was renamed tovalue