$wpdb->prepare should merge all arguments to pass to vsprintf

[westi]

Currently you can't do something like this:

$wpdb->prepare("SELECT * FROM $table WHERE `x` = %s AND type IN (%d,%d,%d)", $where, $in_array);

You can do:

$wpdb->prepare("SELECT * FROM $table WHERE type IN (%d,%d,%d)", $in_array);

It would be really good if prepare took all the replacement arguments and built up a single array for you so you had the freedom to pass in arrays and strings for different replacement types.

Inspired by #11100

[westi]

Inspiration for this was a discussion with Viper007Bond and DD32 in #wordpress-dev

[dd32]

attachment 11102.diff added

Nothing really to say about the patch.. Pretty simple really.

As long as you pass the correct number of args then it works as expected.

If for some reason, an Array is passed in when a string was expected, There's the downside that things overflow into the next fields.. ie:

$a = 'A';
$thought_to_be_string = array('B1', 'B2');
$c = 'C';
var_dump( $wpdb->prepare('SELECT * FROM $TABLE WHERE a = %s AND b = %s AND c = %s', $a, $thought_to_be_string, $c) );
// string(60) "SELECT * FROM $TABLE WHERE a = 'A' AND b = 'B1' AND c = 'B2'"

One way around it, could be:

		if ( count($args) != substr_count($query, '%') )
			return false;

which prevents mistaken overflows.. vsprintf() returns false if too few args are passed in as it is.

But that solution doesn't take into account %% being present in the string.

If those gotcha's are thought to be OK for the added benefit, then so be it :)

[dd32]

with bail on unmatched args/%'s

[azaozz]

Not sure if that really improves things. Passing an array seems to make sense only in "IN (5,10,20, ...)" where the length of the array is not fixed.

MySQL seems to accept both "IN (5,10,20, ...)" and "IN ('5','10','20', ...)" (it casts string to int when comparing with a numeric column) so we can join( "','", $array ) and pass it as a string. This is already used in about 50 places in core including #11100.

In this case a query would look like:

$wpdb->prepare("SELECT * FROM $table WHERE `x` = %s AND type IN (%s)", $where, $in_array);

that would expand to:

SELECT * FROM $table WHERE `x` = 'bar' AND type IN ('5','10','20');

or

SELECT * FROM $table WHERE `x` = 'bar' AND type IN ('bar1','bar2','bar3');

[westi]

Replying to azaozz:

Not sure if that really improves things. Passing an array seems to make sense only in "IN (5,10,20, ...)" where the length of the array is not fixed.

MySQL seems to accept both "IN (5,10,20, ...)" and "IN ('5','10','20', ...)" (it casts string to int when comparing with a numeric column) so we can join( "','", $array ) and pass it as a string. This is already used in about 50 places in core including #11100.

The point is to make it easy to use the correct data type specifier in the prepare statement. If we are preparing in what we believe/expect are integers we should use %d.

Replying to azaozz:

In this case a query would look like:

$wpdb->prepare("SELECT * FROM $table WHERE `x` = %s AND type IN (%s)", $where, $in_array);

that would expand to:

SELECT * FROM $table WHERE `x` = 'bar' AND type IN ('5','10','20');

or

SELECT * FROM $table WHERE `x` = 'bar' AND type IN ('bar1','bar2','bar3');

With the current code this would expand to

SELECT * FROM $table WHERE `x` = 'bar' AND type IN (''bar1','bar2','bar3'');

Assuming $in_array was a string containing 'bar1','bar2','bar3' as we force %s to be quoted.

[Denis-de-Bernardy]

what I'd find interesting, personally, is:

prepare("SELECT * FROM table WHERE col IN (%s)", $array);

anything else doesn't make much sense. depending on whether we have %d or %s, $array should then expand to:

implode(", ", array_map('intval', $array))

or:

"'" . implode("', '", $wpdb->escape($array)) . "'"

before it does, though, it should check if the array is empty, and fallback to NULL. Queries with an IN () clause are invalid SQL, and they're the kind of crippling bugs that are difficult to spot when scanning a query log. If the same query contains a big fat NULL in the SQL, it immediately becomes more obvious what the problem is.

[hakre]

Related: #11608

[pento]

Passing an array to fill an IN() clause is cute, but I'd be inclined not to do this. Neither mysqli nor PDO support this behaviour, which would just make things confusing.

[nacin]

The reasoning in the ticket description is a bit tough to follow — I'm inclined to think that this is far too convoluted to be added to prepare().