Skip to content

Comments

Added "db get-rows" command #153

Closed
petenelson wants to merge 7497 commits intowp-cli:masterfrom
petenelson:feature/db-output-single-column
Closed

Added "db get-rows" command #153
petenelson wants to merge 7497 commits intowp-cli:masterfrom
petenelson:feature/db-output-single-column

Conversation

@petenelson
Copy link
Contributor

This code addresses #145 by adding a get-rows command to allow custom DB queries to get results formatted in a list, CSV, JSON, or a single column that can be piped to other commands.

miya0001 and others added 30 commits August 25, 2017 22:53
…t-stdout"

This reverts commit 17e2438, reversing
changes made to 5159191.
Revert wp-cli#37 to remove `--stdout` for `wp db export`
Cater for reserved word column/table names in db search.
Change default regex delimiter to `chr(1)`
db search: fix match in context in non-regex case.
Search slash example no longer needs escaping.
Update scaffolded README and tests with latest
@petenelson petenelson requested a review from a team as a code owner November 4, 2019 21:29
@schlessera
Copy link
Member

At first I thought this PR was good for merging, but then I realized that it deviates considerably from #145 .

I think this should be implemented as a [--format=<format>] flag to the existing wp db query:

  • stick with using mysql directly, not loading WP for this.
  • strip --format=<format> from the arguments that are sent to mysql
  • only allow select queries when --format=<format> was specified
  • collect results from mysql and then pass them to the requested formatter

Are you up for another stab at this?

@schlessera schlessera removed this from the 2.0.5 milestone Nov 12, 2019
@petenelson
Copy link
Contributor Author

This was the part I'm not quite clear on how to implement collect results from mysql and then pass them to the requested formatter which is why I went with this get-rows route. Would the ob_start/ob_get_clean pickup the terminal output from the run_mysql_command() function?

@petenelson
Copy link
Contributor Author

Aha, think I found what's needed, passing descriptors and reading from pipes. Making some progress on this.

@petenelson
Copy link
Contributor Author

Something like this, but it will take an update to the run_mysql_command() in Utils in order to pass an arg to be able to capture the output.

$descriptors = [
	[ 'pipe', 'r' ],
	[ 'pipe', 'w' ],
	[ 'pipe', 'w' ],
];

$run_args = [ 'return_results' => true ];

$results = self::run( '/usr/bin/env mysql --no-defaults --no-auto-rehash --skip-column-names', $assoc_args, $descriptors, $run_args );

$results = array_filter( explode( PHP_EOL, $results ) );

I'll need to get the main wp-cli project setup locally for development in order to submit a PR for the run_mysql_command() update.

@schlessera
Copy link
Member

@petenelson Looks like you've got ti figured out. Let me know if you need assistance with building something like the above.

@schlessera
Copy link
Member

This will be greatly simplified by wp-cli/wp-cli#5384.

@schlessera
Copy link
Member

@petenelson Are you still interested in working on this?

It should be more straight-forward now, I've adapted Utils\ run_mysql_command(), so that if you pass false as the fourth argument, it will return an array with [ $stdout, $sterr, $exit_code ].

@schlessera
Copy link
Member

Hey @petenelson,

Gentle ping to see if you're still up for giving this another try...

No worries at all if you can't find the time or energy, I would just appreciate knowing about that to decide what to do with the PR.

@petenelson
Copy link
Contributor Author

Probably not right now. I'd need to get all of the development environment up and running again.

@danielbachhuber
Copy link
Member

Proceeding with wp-cli/wp-cli#5594 for this repository. I've captured this PR to https://gist.github.com/danielbachhuber/6924f9d9aa78fa3b2092943943bf45b7 in case this PR is auto-closed or broken in some way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.