WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#30681 closed defect (bug) (fixed)

Using a value in a meta_query (in WP_Query) combined with EXISTS OR NOT EXISTS breaks query

Reported by: barrykooij Owned by: johnbillion
Milestone: 4.1 Priority: normal
Severity: normal Version: 4.1
Component: Query Keywords: has-patch
Focuses: Cc:

Description

From 3.9 and up you don't have to specify a value when using the 'EXISTS' or 'NOT EXISTS' comparisons in meta_query in WP_Query. I still do, to prevent those annoying bugs for people on 3.8 and lower. This will break in WordPress 4.1.

Attached patch fixes this problem.

Attachments (2)

meta.php.diff (418 bytes) - added by barrykooij 6 years ago.
30681.diff (2.4 KB) - added by boonebgorges 6 years ago.

Download all attachments as: .zip

Change History (14)

@barrykooij
6 years ago

#1 follow-up: @johnbillion
6 years ago

  • Component changed from General to Query
  • Keywords has-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.1

To clarify, the current code results in an invalid WHERE clause that looks like this (note the empty single quotes):

CAST(wp_postmeta.meta_value AS CHAR) NOT EXISTS '' )

We'll need some unit tests here to cover this.

Last edited 6 years ago by johnbillion (previous) (diff)

#2 @barrykooij
6 years ago

  • Component changed from Query to General
  • Keywords has-patch needs-unit-tests removed

The reason it breaks:

In the switch statement on $meta_compare the default case will be used for EXISTS and NOT EXISTS. Therefor this code will run on an empty string

$where = $wpdb->prepare( '%s', $meta_value );

resulting in a string with just 2 quotes. That string with just 2 quotes will pass this check

if ( $where ) {

and therefor attach the complete where statement to the SQL chunks.


Last edited 6 years ago by barrykooij (previous) (diff)

#3 in reply to: ↑ 1 @barrykooij
6 years ago

Replying to johnbillion:

To clarify, the current code results in an invalid WHERE clause that looks like this (note the empty brackets):

CAST(wp_postmeta.meta_value AS CHAR) NOT EXISTS '' )

We'll need some unit tests here to cover this.

Not only is the appended 2 x single quotes incorrect, the whole where statement should not be added to the SQL chunks.

#4 @johnbillion
6 years ago

Introduced in r29940.

#5 @johnbillion
6 years ago

  • Component changed from General to Query
  • Keywords needs-unit-tests has-patch 2nd-opinion added

Code to reproduce the issue:

new WP_Query( array(
	'meta_query' => array(
		array(
			'key'     => 'foo',
			'value'   => 'bar',
			'compare' => 'NOT EXISTS',
		)
	),
) );

I think this would benefit from an explicit switch case for EXISTS and NOT EXISTS which ignores the presence of the meta value argument and doesn't populate the WHERE clause.

#6 @johnbillion
6 years ago

Previously: #23268

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


6 years ago

#8 @boonebgorges
6 years ago

  • Keywords needs-unit-tests 2nd-opinion removed

barrykooij, thanks for the report.

Your interpretation of NOT EXISTS is correct. In 4.0, a 'value' passed with 'NOT EXISTS' would be ignored. 30681.diff fixes this for 4.1.

EXISTS is different. Prior to 4.1, there was a quirk in the way that EXISTS was parsed. See https://core.trac.wordpress.org/browser/tags/4.0.1/src/wp-includes/meta.php#L1028 - 'EXISTS' is not a whitelisted value for 'compare', so becomes '='. If no value was passed, the result was a SQL query along the lines of WHERE meta_key = 'foo', which is semantically the same as an EXISTS query. But passing a 'value' would make a SQL clause like WHERE meta_key = 'foo' AND meta_value = 'bar'. The tests and the fix in 30681.diff reflect this behavior in < 4.1. (IMO this is sort of a bug, but I'm concern at this point with preserving existing behavior.)

I've verified these additional tests as passing on 4.0.1, failing on the current 4.1 branch, and fixed by the additional cases in 30681.diff.

@boonebgorges
6 years ago

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


6 years ago

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


6 years ago

#11 @boonebgorges
6 years ago

[30846] missed the ticket. It fixes the issue in trunk. Still needs to be applied to the 4.1 branch.

#12 @johnbillion
6 years ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 30848:

In WP_Meta_Query, interpret 'value' correctly when used with EXISTS/NOT EXISTS.

As in earlier versions, EXISTS with a value is equivalent to '=', while NOT
EXISTS should always ignore 'value'.

Merges [30846] to the 4.1 branch.

Props barrykooij.
Fixes #30681

Note: See TracTickets for help on using tickets.