Discussion:
WPDB does not seem to support RENAME TABLE query
Paul Menard
2013-09-21 23:02:09 UTC
Permalink
Maybe I'm missing something. Or maybe this is just an oversight on the wpdb class.

When calling the SQL RENAME TABLE `table_source` TO `table_dest`;

I get a PHP Warning: errno:2 mysql_fetch_object(): supplied argument is not a valid MySQL result resource /usr/local/www/htdocs/projects/Incsub/local.inc352-snapshot.com/wp-includes/wp-db.php on line 1225

In looking at the wp-db.php code I see where it checks if the query is create, alter, truncate, drop (line 1212). This of course falls through.

I know I have some options

1. Do my own direct mysql to handle the rename - But I want to try and keep this within WP. Plus it would mean a new DB connection.

2. Change the query from RENAME TABLE query to something like INSERT INTO `table_dest` SELECT * FROM `table_source`; DROP TABLE `table_source`; This also mean I need to add in the CREATE TABLE query. So seems like multiple steps instead of the clean RENAME TABLE.

Anyone see what I'm missing? Is there are way to use RENAME TABLE within $wpdb ?

P-
J.D. Grimes
2013-09-22 13:07:17 UTC
Permalink
Are you sure the user has the correct permissions? A search online for "wpdb rename table" (https://www.google.com/search?q=wpdb+rename+table) brings up code in several plugins that seem to have done this. I don't see anything that should stop it.

And what version of WordPress are you using? Those line number don't seem to match trunk or any of the recent versions (maybe trunk has changed since you posted this).
Post by Paul Menard
Maybe I'm missing something. Or maybe this is just an oversight on the wpdb class.
When calling the SQL RENAME TABLE `table_source` TO `table_dest`;
I get a PHP Warning: errno:2 mysql_fetch_object(): supplied argument is not a valid MySQL result resource /usr/local/www/htdocs/projects/Incsub/local.inc352-snapshot.com/wp-includes/wp-db.php on line 1225
In looking at the wp-db.php code I see where it checks if the query is create, alter, truncate, drop (line 1212). This of course falls through.
I know I have some options
1. Do my own direct mysql to handle the rename - But I want to try and keep this within WP. Plus it would mean a new DB connection.
2. Change the query from RENAME TABLE query to something like INSERT INTO `table_dest` SELECT * FROM `table_source`; DROP TABLE `table_source`; This also mean I need to add in the CREATE TABLE query. So seems like multiple steps instead of the clean RENAME TABLE.
Anyone see what I'm missing? Is there are way to use RENAME TABLE within $wpdb ?
P-
_______________________________________________
wp-hackers mailing list
http://lists.automattic.com/mailman/listinfo/wp-hackers
Paul Menard
2013-09-22 13:29:39 UTC
Permalink
Post by J.D. Grimes
Are you sure the user has the correct permissions? A search online for "wpdb rename table" (https://www.google.com/search?q=wpdb+rename+table) brings up code in several plugins that seem to have done this. I don't see anything that should stop it.
And what version of WordPress are you using? Those line number don't seem to match trunk or any of the recent versions (maybe trunk has changed since you posted this).
Yes, of course. The RENAME TABLE still executes. Just the pesky PHP Warning. The WordPress version I'm using 3.5.2 only because that is where the environment is setup. But same issue with 3.6 and 3.6.1. Yes, the line number is off because I had attempted to add some debug into the core code to see what was happening. Apologies. The correct line number for the PHP Warning is 1227 This is the code from WP 3.6 wp-db.php in the function query() It is in the while loop where the PHP Warning is generated. The warning is shown because $this->result is not set. There should at least be some check to ensure $this->result is set before passing into the while.

if ( preg_match( '/^\s*(create|alter|truncate|drop)\s/i', $query ) ) {
$return_val = $this->result;
} elseif ( preg_match( '/^\s*(insert|delete|update|replace)\s/i', $query ) ) {
$this->rows_affected = mysql_affected_rows( $this->dbh );
// Take note of the insert_id
if ( preg_match( '/^\s*(insert|replace)\s/i', $query ) ) {
$this->insert_id = mysql_insert_id($this->dbh);
}
// Return number of rows affected
$return_val = $this->rows_affected;
} else {
$num_rows = 0;
while ( $row = @mysql_fetch_object( $this->result ) ) { <-- result not set when RENAME
$this->last_result[$num_rows] = $row;
$num_rows++;
}

// Log number of rows the query returned
// and return number of rows selected
$this->num_rows = $num_rows;
$return_val = $num_rows;
}


The real issue however is at line 1215 (see first preg_match line above). The wpdb class has some crufty logic in that is attempts to figure out which queries would not return results. So in the current case CREATE, ALTER, TRUNCATE and DROP. I suggest adding 'rename' to that preg_match argument list since 'rename' is a valid SQL command.

Without the 'RENAME' in the first if the logic falls through to the ELSE where the while loop generates the PHP warning.


My solution as of this morning is to sub-cloass wpdb and replace the query function with my own version which includes the 'rename' as part of the preg_replace argument. This at least solves my issue.
J.D. Grimes
2013-09-22 13:39:16 UTC
Permalink
Yes, I see what you are saying now - the line numbers where throwing me off. I suggest that you open up a ticket for this on Trac, and maybe submit a patch as well.

http://core.trac.wordpress.org/
Post by Paul Menard
Post by J.D. Grimes
Are you sure the user has the correct permissions? A search online for "wpdb rename table" (https://www.google.com/search?q=wpdb+rename+table) brings up code in several plugins that seem to have done this. I don't see anything that should stop it.
And what version of WordPress are you using? Those line number don't seem to match trunk or any of the recent versions (maybe trunk has changed since you posted this).
Yes, of course. The RENAME TABLE still executes. Just the pesky PHP Warning. The WordPress version I'm using 3.5.2 only because that is where the environment is setup. But same issue with 3.6 and 3.6.1. Yes, the line number is off because I had attempted to add some debug into the core code to see what was happening. Apologies. The correct line number for the PHP Warning is 1227 This is the code from WP 3.6 wp-db.php in the function query() It is in the while loop where the PHP Warning is generated. The warning is shown because $this->result is not set. There should at least be some check to ensure $this->result is set before passing into the while.
if ( preg_match( '/^\s*(create|alter|truncate|drop)\s/i', $query ) ) {
$return_val = $this->result;
} elseif ( preg_match( '/^\s*(insert|delete|update|replace)\s/i', $query ) ) {
$this->rows_affected = mysql_affected_rows( $this->dbh );
// Take note of the insert_id
if ( preg_match( '/^\s*(insert|replace)\s/i', $query ) ) {
$this->insert_id = mysql_insert_id($this->dbh);
}
// Return number of rows affected
$return_val = $this->rows_affected;
} else {
$num_rows = 0;
$this->last_result[$num_rows] = $row;
$num_rows++;
}
// Log number of rows the query returned
// and return number of rows selected
$this->num_rows = $num_rows;
$return_val = $num_rows;
}
The real issue however is at line 1215 (see first preg_match line above). The wpdb class has some crufty logic in that is attempts to figure out which queries would not return results. So in the current case CREATE, ALTER, TRUNCATE and DROP. I suggest adding 'rename' to that preg_match argument list since 'rename' is a valid SQL command.
Without the 'RENAME' in the first if the logic falls through to the ELSE where the while loop generates the PHP warning.
My solution as of this morning is to sub-cloass wpdb and replace the query function with my own version which includes the 'rename' as part of the preg_replace argument. This at least solves my issue.
_______________________________________________
wp-hackers mailing list
http://lists.automattic.com/mailman/listinfo/wp-hackers
Andrew Nacin
2013-09-22 18:24:08 UTC
Permalink
Post by Paul Menard
Maybe I'm missing something. Or maybe this is just an oversight on the wpdb class.
When calling the SQL RENAME TABLE `table_source` TO `table_dest`;
I get a PHP Warning: errno:2 mysql_fetch_object(): supplied argument is
not a valid MySQL result resource /usr/local/www/htdocs/projects/Incsub/
local.inc352-snapshot.com/wp-includes/wp-db.php on line 1225
I've honestly never seen someone need to use wpdb to rename a table at
runtime. Nothing wrong with wp-db.php being patched to support this. That
said, this should work: ALTER TABLE old_name RENAME new_name;

Nacin
Paul Menard
2013-09-22 20:12:04 UTC
Permalink
Post by Andrew Nacin
I've honestly never seen someone need to use wpdb to rename a table at
runtime. Nothing wrong with wp-db.php being patched to support this. That
said, this should work: ALTER TABLE old_name RENAME new_name;
Nacin
Thanks Nacin, yeah, I guess that is one way to get past the issue.
Chris McCoy
2013-09-22 22:28:37 UTC
Permalink
Curious about this, is it really good practice to rename a table in a
plugin from a security standpoint?
Post by Andrew Nacin
Post by Paul Menard
Maybe I'm missing something. Or maybe this is just an oversight on the wpdb class.
When calling the SQL RENAME TABLE `table_source` TO `table_dest`;
I get a PHP Warning: errno:2 mysql_fetch_object(): supplied argument is
not a valid MySQL result resource /usr/local/www/htdocs/projects/Incsub/
local.inc352-snapshot.com/wp-includes/wp-db.php on line 1225
I've honestly never seen someone need to use wpdb to rename a table at
runtime. Nothing wrong with wp-db.php being patched to support this. That
said, this should work: ALTER TABLE old_name RENAME new_name;
Nacin
_______________________________________________
wp-hackers mailing list
http://lists.automattic.com/mailman/listinfo/wp-hackers
Paul Menard
2013-09-22 23:18:29 UTC
Permalink
How/Why would it be a security concern? The plugin owns the table. This is not a WP core table. The RENAME command requires the same user permissions, ALTER and DROP, as most other WP DB function. Thought the DROP is rarely used. And really it is no different than doing a multiple step SQL comment to 1) DROP the destination table, 2) CREATE TABLE the new destination table, 3) SELECT INSERT INTO from the source to the destination, 4) DROP the source table.

Not that I want to get into the how/why processing of the plugin. Which really was not my point. This is for a very specific client use and not something to distribute to the general public. While the source table is being loaded from a previous export we didn't want to load directly into the live table because the size is upwards of 2G. Plus the data is meant to replace the existing table. So we are loading to a temporary named table first. This lets queries against the live table occur without issue. Once the load finishes the live table is dropped and the RENAME occurs. Simple as that.

P-
Post by Chris McCoy
Curious about this, is it really good practice to rename a table in a
plugin from a security standpoint?
Post by Andrew Nacin
Post by Paul Menard
Maybe I'm missing something. Or maybe this is just an oversight on the wpdb class.
When calling the SQL RENAME TABLE `table_source` TO `table_dest`;
I get a PHP Warning: errno:2 mysql_fetch_object(): supplied argument is
not a valid MySQL result resource /usr/local/www/htdocs/projects/Incsub/
local.inc352-snapshot.com/wp-includes/wp-db.php on line 1225
I've honestly never seen someone need to use wpdb to rename a table at
runtime. Nothing wrong with wp-db.php being patched to support this. That
said, this should work: ALTER TABLE old_name RENAME new_name;
Nacin
_______________________________________________
wp-hackers mailing list
http://lists.automattic.com/mailman/listinfo/wp-hackers
_______________________________________________
wp-hackers mailing list
http://lists.automattic.com/mailman/listinfo/wp-hackers
Loading...