Make WordPress Core

Opened 7 years ago

Last modified 3 months ago

#42352 new enhancement

Support use of native MySQLi prepared queries

Reported by: dd32's profile dd32 Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Database Keywords: early
Focuses: Cc:

Description

When we added $wpdb->prepare() back in WordPress 2.3, we were forced to roll our own MySQL Prepared queries as the MySQL extension didn't support it.
While the MySQL extension still doesn't, the majority of WordPress sites are likely using the newer MySQLi engine (by default, enabled for PHP 5.5+ which accounts for 70~80% of sites). That makes now a good time to start investigating the potential implementation and usage of native prepared queries in the future.

Attached is a first-draft of implementing native prepared queries into WPDB, it has no fallbacks for MySQL at present, but it would be possible to force support in for it using a "simple" SQL parser.
Unfortunately I expect that this draft is incompatible with many DB drop-ins, which is okay, this is only a draft and proof-of-concept.

I'll attach some examples of how this first draft could be used in a comment below.

Attachments (3)

42352.diff (14.7 KB) - added by dd32 7 years ago.
42352.2.diff (17.0 KB) - added by dd32 7 years ago.
42352.3.diff (24.5 KB) - added by dd32 7 years ago.

Download all attachments as: .zip

Change History (12)

@dd32
7 years ago

#1 @dd32
7 years ago

Specifically regarding 42352.diff, here's some very basic examples of how it can be used:

$post_id = 123;
$var = $wpdb->get_var(
   "SELECT post_author FROM {$wpdb->posts} WHERE ID = ?",
   array( $post_id )
);

Using numbered parameters isn't supported, and does require repeating the values in the args if that's something that's being done:

$user_search = "test-user";
$user = $wpdb->get_row(
    "SELECT * FROM {$wpdb->users} WHERE user_login = ? OR user_nicename = ? OR user_email = ?", 
    array(
        $user_search,
        $user_search,
        $user_search
   )
);

Because prepared queries are only designed for being used with values, you can't use a placeholder for a table name (or fields), the following will NOT work (intentionally)

// does NOT work
$wpdb->get_var(
   "SELECT ? FROM ? WHERE ? = ?",
   array( 'ID', $wpdb->users, 'user_login', 'admin' )
);

If you need to be able to specify the type of a parameter, for example, as an integer or double that can be done by passing an array of data type and value:

$query = "SELECT * FROM {$wpdb->posts} WHERE post_author = ? OR post_name = ?";
$args = array(
  // post_author:
   array(
     'type' => 'i', // integer
     'value' => 1,
   ),
   // post_name
   array(
     'type' => 's', // string
     'value' => 'my-post-name'
  )
);

$row = $wpdb->get_row( $query, $args );

The expanded syntax looks a bit verbose at first, but is only needed if you need to force an arg to either an integer (i) or a double/float (d), which may be useful for INSERT/UPDATE type queries. It can be mixed, using only arrays for integers/doubles and a direct string assumed as a string.

@dd32
7 years ago

#2 @dd32
7 years ago

42352.2.diff fixes a few bugs here and there.
Notably:

  • 42352.diff uses mysqli_stmt_get_result() which is PHP 5.3+ but also only available with mysqlnd
  • 42352.2.diff uses mysqli_fetch_fields( mysqli_stmt_result_metadata() ) which appears to be supported anywhere MySQLi is.

The above change does require more variable binding, but is more portable and compatible with the core MySQLi extension. The mysqlnd function could be used conditionally, but that introduces more code paths and conditionals for testing which needlessly complicates it (unless it's performance is far better)

The only thing in the patch I haven't tested is the create|alter|truncate|drop return values, although I doubt they could/would be used with prepared statements.

@dd32
7 years ago

#3 @dd32
7 years ago

42352.3.diff builds upon 42352.2.diff and fixes bugs and adds support for things it probably shouldn't.

Notable:

  • When returning multiple rows, it now returns multiple rows rather than multiple copies of the final row (it wasn't dereferencing the values, only the holding array)
  • When returning rows, return objects instead of arrays
  • Switches the insert|replace|update|delete() helpers over to the new prepares. May break if anyone is passing something other than %s, %d, or %f as the $format placeholder which is technically possible, but shouldn't be done.
  • Adds prepared statement fallback for MySQL through the usage of $wpdb->prepare() while only supporting ? placeholders. It works, and is ultimately more restrictive than $wpdb->prepare() which is probably a good thing.

A bunch of unit tests are breaking, because it changes the way queries are constructed (ie. the $wpdb->update() tests expect the values to be in $wpdb->last_query but they're not). Documentation needs fixing, tests need writing, variables need renaming, code needs cleaning, PHP 5.2 compat needs to happen (a single anonymous callback).

All in all, I'm fairly confident that adding support for native prepares and encouraging their usage (even if only for the more relatable syntax) is within our reach, despite still supporting PHP 5.2 & PHP without MySQLi.
Coming into this experiment I expected that we'd have to drop PHP 5.2/MySQL support, I'm happy to have been proved wrong.
If we drop our entirely-cautious MySQLi for PHP 5.5+ only, MySQL for PHP 5.2~5.4 checks and use MySQLi when available we'd probably be able to get native prepares to be available for ~90% of sites rather than the above 70-80% too.

#4 @dd32
7 years ago

  • Milestone changed from Awaiting Review to Future Release

42352.3.diff probably wasn't actually the version I intended to upload here. Either way, the patches continued in https://github.com/dd32/wordpress-develop/tree/add-native-sql-prepare-42352 - I've experimented with a few things and making a few dangerous decisions that may not actually be merged into core (But it's Git, so those commits/methods can be removed)

If we drop our entirely-cautious MySQLi for PHP 5.5+ only, MySQL for PHP 5.2~5.4 checks and use MySQLi when available we'd probably be able to get native prepares to be available for ~90% of sites rather than the above 70-80% too.

See #42812

I'm marking this as Future Release as it's likely we'll move to real prepared queries, but until we have a solid way of handling back-compat for mysql_* clients we're kind of stuck.

#5 @desrosj
5 years ago

Related: #48116.

#6 follow-up: @iandunn
4 years ago

@dd32, do you think anything has changed here in the past few years, or is back-compat still the major blocker?

#7 in reply to: ↑ 6 ; follow-up: @dd32
4 years ago

Replying to iandunn:

dd32, do you think anything has changed here in the past few years, or is back-compat still the major blocker?

Support of ext/mysql is still a "blocker", but as we have no idea on how many people are using it I don't know how much of a blocker it is.

#48116 is for adding stats that could be used to determine that.

I'll have a think about the next steps here, I don't think ext/mysql is a hard blocker, but it certainly limits us.

#8 in reply to: ↑ 7 @SergeyBiryukov
15 months ago

Replying to dd32:

I'll have a think about the next steps here, I don't think ext/mysql is a hard blocker, but it certainly limits us.

This appears to be unblocked by [56475] / #59118.

This ticket was mentioned in Slack in #core by dd32. View the logs.


3 months ago

Note: See TracTickets for help on using tickets.