Opened 4 years ago
Closed 4 years ago
#37660 closed defect (bug) (fixed)
RegEx in wpdb::get_table_from_query( $q ) in wp-db.php returns table_alias='a' rather than table_name=wp_999_options
Reported by: | webp | Owned by: | pento |
---|---|---|---|
Milestone: | 4.7 | Priority: | normal |
Severity: | normal | Version: | 4.1.2 |
Component: | Database | Keywords: | needs-unit-tests needs-patch |
Focuses: | Cc: |
Description
WordPress database error
WordPress database error Table 'wp_xyz.wp_999_options' doesn't exist for query DELETE a, b FROM wp_999_options a, wp_999_options b WHERE a.option_name LIKE '\\_transient\\_%' AND a.option_name NOT LIKE '\\_transient\\_timeout\\_%' AND b.option_name = CONCAT( '_transient_timeout_', SUBSTRING( a.option_name, 12 ) ) AND b.option_value < 1470982769 made by populate_options
Problem
function populate_options() in wp-admin/includes/schema.php
fires the SQL query above that goes through hyperdb and then $wpdb in wp-db.php.
class hyperdb extends wpdb in wp-includes/wp-db.php:
# hyperdb/db.php class hyperdb extends wpdb { function get_table_from_query( $q ) { if( method_exists( get_parent_class( $this ), 'get_table_from_query' ) ) { // WPDB has added support for get_table_from_query, which should take precedence return parent::get_table_from_query( $q ); } # wp-includes/wp-db.php protected function get_table_from_query( $query ) { // Quickly match most common queries. if ( preg_match( '/^\s*(?:' . 'SELECT.*?\s+FROM' . '|INSERT(?:\s+LOW_PRIORITY|\s+DELAYED|\s+HIGH_PRIORITY)?(?:\s+IGNORE)?(?:\s+INTO)?' . '|REPLACE(?:\s+LOW_PRIORITY|\s+DELAYED)?(?:\s+INTO)?' . '|UPDATE(?:\s+LOW_PRIORITY)?(?:\s+IGNORE)?' . '|DELETE(?:\s+LOW_PRIORITY|\s+QUICK|\s+IGNORE)*(?:\s+FROM)?' . ')\s+((?:[0-9a-zA-Z$_.`-]|[\xC2-\xDF][\x80-\xBF])+)/is', $query, $maybe ) ) { return str_replace( '`', '', $maybe[1] ); }
Cause
$query = DELETE a, b FROM wp_999_options a, wp_999_options b ....
RegEx in wpdb::get_table_from_query( $q ) in wp-db.php below returns table_alias='a' rather than table_name=wp_999_options from $query above:
. '|DELETE(?:\s+LOW_PRIORITY|\s+QUICK|\s+IGNORE)*(?:\s+FROM)?'
WordPress database error occurs when Table wp_999_options is not found in default database wp_xyz. So my hyperdb config directs unknown db name 'a' to default db wp_xyz, rather than my hyperdb mapped database wp_abc.
Suggested Fix
Change RegEx to:
. '|DELETE(?:.+FROM)?'
Is it a good fix? Or am I missing other scenarios?
Core Developers should also watch out if other similar SQL might failed in wpdb::get_table_from_query( $q ):
SQL COMMAND table_alias FROM table, table_alias
If fix confirmed, someone should ask hyperdb to fix as similar RegEx is used. Not too many WordPress developers use hyperdb so I'm too lazy to submit a ticket for hyperdb myself.
But wpdb is used everywhere and it should be perfect, or strive to be perfect. Otherwise, developers like me will be scratching my head for hours when minor wpdb errors arise. Even though this bug is minor, but unknown problems might cause database corruption months down the line, at least for me and my massive number of replicated databases and servers.
Change History (3)
#1
@
4 years ago
- Keywords needs-unit-tests needs-patch added
- Milestone changed from Awaiting Review to 4.7
- Owner set to pento
- Status changed from new to accepted
- Version changed from trunk to 4.1.2
Thank you for the detailed investigation and bug report, @webp!
You are correct, this is definitely a bug, even more so that it affects a query in Core.
I think the fix will be to replace the current
(?:\s+FROM)
part of the match with(?:.+?FROM)
- similar to your suggestion, but the additional?
should make it a non-greedy match, stopping at the firstFROM
. This will need testing, of course. :-)The obvious shortcoming of
wpdb::get_table_from_query()
is that it only returns the first table in a multi-table query - as was the case with the method's HyperDB origins, it assumes that you're only querying tables on the same server. I'm fine with this behaviour continuing for this variation ofDELETE
syntax.I had a quick look at the other queries that
wpdb::get_table_from_query()
matches - I don't think there are any affected by similar bugs. Extra eyes are always welcome, of course, please point out if you notice any others!