[wp-trac] [WordPress Trac] #25559: Enhance prepare method to better support SQL IN operator

[WordPress Trac]

#25559: Enhance prepare method to better support SQL IN operator
------------------------------------------------+--------------------------
 Reporter:  cannona                             |       Owner:
     Type:  enhancement                         |      Status:  new
 Priority:  normal                              |   Milestone:  Awaiting
Component:  Database                            |  Review
 Severity:  normal                              |     Version:  trunk
 Keywords:  has-patch 2nd-opinion dev-feedback  |  Resolution:
------------------------------------------------+--------------------------
Changes (by johnbillion):

 * keywords:  has-patch => has-patch 2nd-opinion dev-feedback
 * type:  feature request => enhancement


Comment:

 I like the idea of a new conversion specification that converts an array
 to a comma-separated list that can be used in the `IN` and `NOT IN`
 operators.

 That said, the approach taken in [attachment:wp-db_mods.patch] over-
 complicates matters and doesn't solve the underlying problem. Here's some
 feedback on that patch:

  * The `%#ns` format (where `n` is the number of times to repeat the
 value) falls down as soon as the number of values is variable and unknown,
 which defeats the whole point. If you're hardcoding the number of values
 into the conversion specification then how can it handle a variable number
 of values?
  * Accepting and flattening a variety of array and non-array arguments in
 `wpdb::prepare()` is an unnecessary complication, and is asking for
 trouble. What's the use case for accepting a mixture of multiple
 arguments, arrays, and multidimensional arrays, and flattening them all
 into one operator? If you're passing values like this then you're doing
 something wrong. What happens when you have arguments that follow your
 arguments for your `IN/NOT IN` clause? It becomes unclear which arguments
 are which. This also doesn't allow for a variable number of values (see
 first point). The argument should accept one single-dimensional array
 only.

 One other thing to bear in mind is that `wpdb::prepare()` will also accept
 its arguments as a single array instead of separate arguments (think
 `vsprintf()` vs `sprintf()`). [attachment:wp-db_mods.patch] handles this
 by flattening all the arguments, but doesn't allow for a variable number
 of values in an argument.

 My suggestion would be to simplify the conversion specification to `%#s`,
 where `#` is the element which specifies this is an argument of multiple
 values and varying length, which is to be converted to a comma-separated
 list. Of course, `s` can be `s`, `d`, or `f` as currently.

 One other thought: We're introducing a new conversion specification that
 deviates from those in PHP's string formatting functions. Is this a good
 idea?

--
Ticket URL: <http://core.trac.wordpress.org/ticket/25559#comment:3>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software