Skip to content
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

Handle multi-column keys in search and replace #2531

Conversation

@markkimsal
Copy link
Contributor

commented Mar 7, 2016

The where clause was constructed without regard to handling multiple
column keys nor quoting character values.

The example table that the previous technique doesn't work with is
wp_wp_super_edit_options which has a primary key on id and name.

Adding single quotes around integer values has no negative effect.

CREATE TABLE "wp_wp_super_edit_options" (
"id" bigint(20) NOT NULL AUTO_INCREMENT,
"name" varchar(60) NOT NULL,
"value" text NOT NULL,
PRIMARY KEY ("id","name"),
UNIQUE KEY "name" ("name")
) ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8

Handle multi-column keys in search and replace
The where clause was constructed without regard to handling multiple
keys nor quoting values properly.

The example table that the previous technique doesn't work with is
wp_wp_super_edit_options which has a primary key on id and name.
@@ -288,7 +288,8 @@ private function php_handle_col( $col, $primary_keys, $table, $old, $new ) {
foreach ( $rows as $keys ) {
$where_sql = '';
foreach( (array) $keys as $k => $v ) {
$where_sql .= "{$k}={$v}";
if (strlen($where_sql)) $where_sql .= ' AND ';

This comment has been minimized.

Copy link
@danielbachhuber

danielbachhuber Mar 7, 2016

Member

Please wrap this conditional in braces, and break the statement with newlines.

@markkimsal

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2016

when i tried to search and replace my production URL with development URL using the wp_*_options syntax it snagged a table created by a plugin and gave the following error.

WordPress database error Unknown column '1name' in 'where clause' for query SELECT value FROM wp_wp_super_edit_options WHERE id=1name=tinymce_scan made by include('wp-cli/php/wp-cli.php'), WP_CLI\Runner->start, WP_CLI\Runner->_run_command, WP_CLI\Runner->run_command, WP_CLI\Dispatcher\Subcommand->invoke, call_user_func, WP_CLI\Dispatcher\CommandFactory::WP_CLI\Dispatcher\{closure}, call_user_func, Search_Replace_Command->__invoke, Search_Replace_Command->php_handle_col, W3_Db->query, W3_DbCache->query, W3_DbCallUnderlying->query, W3_Db->query, W3_DbProcessor->query, W3_Db->default_query

As you can see, the WHERE clause was "id=1name=tinymce" instead of "id='1' AND name='tinymce'"

@danielbachhuber

This comment has been minimized.

Copy link
Member

commented Mar 7, 2016

@markkimsal Can you add functional tests for this PR, please?

@markkimsal

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2016

I would have to refactor a lot of php_handle_col in the Search_Replace_Command class. Would that amount of rework be accepted in a PR?

@danielbachhuber

This comment has been minimized.

Copy link
Member

commented Mar 7, 2016

I would have to refactor a lot of php_handle_col in the Search_Replace_Command class.

Sorry, I don't follow. Why would you need to refactor php_handle_col() to add a Behat functional test?

@markkimsal

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2016

I didn't see the behat stuff, was only looking at tests/* and thinking about having to mock $wpdb or refactor the $where_sql generation parts into another private function

@danielbachhuber danielbachhuber added this to the 0.23.0 milestone Mar 7, 2016

@danielbachhuber

This comment has been minimized.

Copy link
Member

commented Mar 7, 2016

Thanks for the tests, @markkimsal

danielbachhuber added a commit that referenced this pull request Mar 7, 2016
Merge pull request #2531 from markkimsal/markkimsal/multi_key_search_…
…replace_bug

Handle multi-column keys in search and replace

@danielbachhuber danielbachhuber merged commit d397705 into wp-cli:master Mar 7, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
danielbachhuber added a commit that referenced this pull request Mar 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.