-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat: add 'system' subcommand to display system tables #25912
Conversation
Once I've got some feedback with respect to the |
Hey @waynr we should keep the CLI commands consistent with the Verb -> Noun verbiage we have so I think we could do something like
but what would be better for naming is up for debate, I just think we should keep things consistent so we don't end up like git which has both verb -> noun and noun -> verb when using it |
@mgattozzi I can see where you're coming from with the desire to retain a
The use of "show system" gives us the desired |
Hmm, we could even make this more generic with |
3597db8
to
c24ebee
Compare
Okay I ended up going with this subcommand structure as of the latest set of commits:
This preserves the I've also added tests so this is ready for review @mgattozzi @pauldix |
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 where you ended up with the influxdb 3 show ...
semantics.
4e1f6e8
to
5987a2e
Compare
test: add 'show system' subcommand tests
Okay I've squashed all my fixes/changes after adding a couple more unit tests and fixing a bug in the |
Addresses #25844
This PR is more or less ready for review, but I suspect there may be one or two changes left to make after getting feedback from the rest of the team. I'll discuss those possible changes below in the demo for each subcommand.
influxdb3 system get
I consider this subcommand more or less good to go. It has the following options:
--limit
- limit the number of entries show (demoed above)--order-by
- a comma-separated list of fields passed to aORDER BY
clause--select
- the set of fields selected in the query (demoed above)--format
- the format of the query result outputinfluxdb3 system summary
The system summary first queries for a list of system tables then iterates over each selecting all fields before displaying them in the chosen output format (
--format
). It also respects the (--limit
flag similar to theget
sub command, but applied individually to each system table).The thing I'm not certain about here is whether we should try to avoid printing results for empty tables.
influxdb3 system list
This subcommand currently displays each system table name along with the available column names for each table indented with two spaces. It works by first querying for all table names then for each table name querying its column names (this could reduced to a single query with a join if we want to keep this output formatting).
I am considering rewriting the implementation of this to use the following single query so that we can stick with the same output formatting options that come with the
summary
andlist
subcommands:The output of this query looks as follows:
I'm pretty sure I prefer this approach since it lets us use the same
--format
options as the other subcommands, but I thought I would get the team's input also.