query with "--default"-flag fails when discovering the SQL mode

[Chrico]

Bug Report

• Yes, I reviewed the contribution guidelines.
• Yes, more specifically, I reviewed the guidelines on how to write clear bug reports.

Describe the current, buggy behavior

When I run following command:

wp db query "select 1;" --defaults

the query fails with following message:

Error: Failed to get current SQL modes. Reason: '"mysql --no-defaults --no-auto-rehash --batch --skip-column-names --execute="SELECT' is not recognized as an internal or external command,
operable program or batch file.

When executing wp db query, than 2 SQL queries are executed internally:

1. discover SQL mode
2. execute actual query

The actual query for 2. is build first and calls DB_Command::get_defaults_flag_string() here. This method will remove the $assoc_args['defaults'] here. Afterwards when DB_Command:.get_sql_mode_query() is called here, than the defaults is not set anymore in $array_assoc. This results in adding --no-defaults to the query 1.

Describe how other contributors can replicate this bug

• Install WP-CLI 2.8.1
• Run wp db query "SELECT 1;" --defaults
• Interally /usr/bin/env mysql --no-defaults /* snip */ --execute='SELECT @@SESSION.sql_mode' is executed first before /usr/bin/env mysql --execute='SELECT 1;' is called.

Describe what you would expect as the correct outcome

The "discover SQL mode" should also not add --no-defaults when wp db query {sql query} --defaults is executed.

Let us know what environment you are running this on

OS:     Windows NT 10.0 build 22621 (Windows 10) AMD64
PHP version:    8.0.19
WP-CLI version: 2.8.1

[danielbachhuber]

Thanks for the report, @Chrico !

Feel free to submit a pull request, if you'd like. Here is some guidance on our pull request best practices.

[lkraav]

Yep since migrating to SkySQL that requires --ssl-verify-server-cert, this is now biting us in the butt as well. db cli is possible to configure with wp-cli.yml, but db query fails with configuration or cmdline because of this issue.

Exploring how to put Copilot on it and get at least a patch candidate out.

[mrsdizzie]

Somewhat related is wp-cli/wp-cli#6073

In general, sticking --no-defaults on everything feels like a very bad default (ha!) and likely causes more problems than it ever fixed.

Removing that feels like the correct solution to many of these.

The way the code handles these is pretty confusing as well, with a function named get_defaults_flag_string which actually modifies $assoc_args in place

db-command/src/DB_Command.php

Lines 2071 to 2076 in c821b92

	* Writes out the `--no-defaults` flag for MySQL commands unless the --defaults flag is specified for the WP_CLI command.
	*
	* @param array $assoc_args Associative args array.
	* @return string Either the '--no-defaults' flag for use in the command or an empty string.
	*/
	protected function get_defaults_flag_string( &$assoc_args ) {

Which doesn't really work in many settings because you can only do this once. @swissspidy maybe you have a thought on if it is reasonable to change this now.

[kraftner]

related: #278

[swissspidy]

Which doesn't really work in many settings because you can only do this once. @swissspidy maybe you have a thought on if it is reasonable to change this now.

Late to the party but agree that it's confusing. If we wanna change it, now (with WP-CLI 3.0.0) around the corner would be a good time to do such a breaking change.

[swissspidy]

Moving this to #311