Make WordPress Core

Opened 10 years ago

Closed 8 years ago

#26114 closed feature request (maybelater)

Auto prepare utilities for wp-db.php

Reported by: dougwollison's profile dougwollison Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.7
Component: Database Keywords:
Focuses: Cc:

Description

This is a feature I've adapted from my kissMySQL class that I use on non-WordPress projects that use a database. It's a handful of non-intrusive utility methods to add to the wpdb class.

Simply put, I personally find it tedious writing this:

$wpdb->get_results( $wpdb->prepare( $query, $args... ) );

I'd prefer to write this:

$wpdb->get_results( $query, $args... );

The idea is any additional arguments are taken to be values to be escaped and inserted into the query. It also intelligently detects and shifts/pops off the $object argument if it's the first or last one passed in the list. So I could easily do this:

$wpdb->get_results( $query, ARRAY_A, $args... );

// is equal too this

$wpdb->get_results( $wpdb->prepare( $query, $args... ), ARRAY_A );

Now, my patch however simply adds alternate methods that alias to the proper ones after doing the auto preparation. The only real downside is that you can't pass x/y args to the get_var/row/col methods. My solution would be to add methods like get_var_x_y and get_row_y, but obviously this would be too drastic an API change.

This patch adds the following new methods (naming schemes leave something to be desired):

  • auto_prepare()
  • get_var_prepared()
  • get_row_prepared()
  • get_col_prepared()
  • get_results_prepared()

This patch also adds a few lines to wpdb::query() so it can use the auto_prepare method, should multiple arguments be passed to it. Since query() only takes one argument, it seemed safe to modify it directly rather than create a query_prepared() method.

In addition, I've had to make some changes to the values of the $object constant values; I've appended a random string to the end so as to greatly minimize the chance of confusion between what a prepare argument and an object format constant. I'd like feedback on this handling in particular if possible; pretty sure I'm missing a better way to do this.

I've tested this and as expected it doesn't appear to cause any problems. More testing will be needed of course; I'm applying it to a number of my installs to see if anything breaks, but so far so good.

Attachments (1)

26114.diff (7.5 KB) - added by dougwollison 10 years ago.
Initial patch: adds auto_prepare, get_(var/row/col/results)_prepared methods and updates query()

Download all attachments as: .zip

Change History (8)

@dougwollison
10 years ago

Initial patch: adds auto_prepare, get_(var/row/col/results)_prepared methods and updates query()

#1 follow-ups: @nacin
10 years ago

Andy Skelton and I had a conversation about this a year or two ago, and it's something we've always wanted to revisit. I think there are a few other things we need to do first to get to this point, though. But if we did this, here's how we'd do it:

Rather than new methods, we can use the existing ones. get_col(), get_var(), get_row(), and get_results() only accept two arguments: The query, and the output. We could add a third argument there, or — something I think is preferred — actually overload the $output argument, instead allowing an array of prepared values, followed by the $output argument in position three.

If func_get_arg(1) is an array, then assume it is values to prepare and that func_get_arg(2) is the $output. Or, if func_get_arg(1) is ARRAY_N/A/OBJECT/_K, then slice off the first two arguments and let the function accept additional arguments the same way prepare() currently does.

There are a number of potential benefits to this. One, you'd always be preparing a complete query to be executing it, so prepared statements where bindings happen server-side (like PDO proper or mysqli) could be used. We could then cache that inside wpdb, thus enabling future queries with the same signature to be re-executed with new values.

Two, which is more practical and less theoretical, you'd be able to actually make decisions based on a query's "signature" (a query minus the prepared values). Example: Filtering all queries of a particular signature, or invalidating the cache of an entire group of queries.

#2 in reply to: ↑ 1 @dougwollison
10 years ago

Replying to nacin:

Rather than new methods, we can use the existing ones. get_col(), get_var(), get_row(), and get_results() only accept two arguments: The query, and the output.

Actually...

  • get_col() takes $query and $x for selecting the exact column from the result to return.
  • get_row() takes $query, $output, and $y for selecting the exact row from the result to return.
  • get_var() takes $query, $x and $y for selecting the exact field from the result to return.
  • get_results() could easily be modified, similar to how query() was.

In practice I've never seen $x/$y used, but we may not want to change that.

If func_get_arg(1) is an array, then assume it is values to prepare and that func_get_arg(2) is the $output. Or, if func_get_arg(1) is ARRAY_N/A/OBJECT/_K, then slice off the first two arguments and let the function accept additional arguments the same way prepare() currently does.

We could add a third argument there, or — something I think is preferred — actually overload the $output argument, instead allowing an array of prepared values, followed by the $output argument in position three.
If func_get_arg(1) is an array, then assume it is values to prepare and that func_get_arg(2) is the $output. Or, if func_get_arg(1) is ARRAY_N/A/OBJECT/_K, then slice off the first two arguments and let the function accept additional arguments the same way prepare() currently does.

That overload would certainly work, and without the slightly possible collision issue mine has.

There are a number of potential benefits to this. One, you'd always be preparing a complete query to be executing it, so prepared statements where bindings happen server-side (like PDO proper or mysqli) could be used. We could then cache that inside wpdb, thus enabling future queries with the same signature to be re-executed with new values.

Not sure if I follow...

Two, which is more practical and less theoretical, you'd be able to actually make decisions based on a query's "signature" (a query minus the prepared values). Example: Filtering all queries of a particular signature, or invalidating the cache of an entire group of queries.

True; we should add a filter to the prepare method before it runs it through vsprintf.

Last edited 10 years ago by dougwollison (previous) (diff)

#3 @jdgrimes
10 years ago

  • Cc jdg@… added

#4 @johnbillion
10 years ago

  • Cc johnbillion added

#5 @SergeyBiryukov
10 years ago

  • Version changed from trunk to 3.7

#6 in reply to: ↑ 1 @rmccue
10 years ago

Replying to nacin:

Andy Skelton and I had a conversation about this a year or two ago, and it's something we've always wanted to revisit. I think there are a few other things we need to do first to get to this point, though.

Out of interest, what are the things we need to hit first?

#7 @chriscct7
8 years ago

  • Keywords dev-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed

No movement in almost 2 years. Closing as maybelater. Feel free to reopen when renewed interest (ideally with a patch) emerges.

Note: See TracTickets for help on using tickets.