WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 11 months ago

#17235 closed defect (bug) (wontfix)

meta_query fails if you don't pass in an array of arrays

Reported by: batmoo Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: Query Keywords: has-patch meta-query
Focuses: Cc:

Description

This tripped me up the first time I used meta_query (see #16563) and I've seen others fall into the same trap. If you don't pass in an array of arrays into meta_query, it generates some funky (but valid) SQL and fails to return anything.

The main instance where people will fall into this is if they only have a single key/value pair to look for. In this case, passing in an array of arrays does not seem intuitive, and meta_query should be smart enough to work with either. Examples below.

This doesn't work:

$my_query = WP_Query( array
	array (
		'post_type' => 'post',
		'meta_query' => array (
			'key' => 'my_key',
			'value' => 'my_value',
		),
	),
);

But this does:

$my_query = WP_Query( array
	array (
		'post_type' => 'post',
		'meta_query' => array (
			array(
				'key' => 'my_key',
				'value' => 'my_value',
			}
		),
	),
);

Attachments (6)

17235.diff (749 bytes) - added by greuben 4 years ago.
17235-2.diff (693 bytes) - added by batmoo 4 years ago.
17235-meta.diff (508 bytes) - added by batmoo 4 years ago.
17235-taxonomy.diff (508 bytes) - added by batmoo 4 years ago.
17235.2.diff (1.0 KB) - added by wonderboymusic 21 months ago.
17235-notice-and-unset.diff (860 bytes) - added by dannydehaan 18 months ago.
17235-notice-and-unset

Download all attachments as: .zip

Change History (23)

comment:1 in reply to: ↑ description @greuben4 years ago

Replying to batmoo:

If you don't pass in an array of arrays into meta_query, it generates some funky (but valid) SQL and fails to return anything.

"svn up" and it doesn't generate funky queries anymore.

comment:2 @scribu4 years ago

  • Keywords close added

Indeed, the meta_query will just be ignored.

I think handling single arrays would create more confusion than solve, especially since you also have the old 'meta_key' and 'meta_value' query vars.

What if a user does something like this:

$my_query = WP_Query( array
	array (
		'post_type' => 'post',
		'meta_query' => array (
			'key' => 'foo',
			'value => 'some_val',
			array(
				'key' => 'bar',
				'value' => 'another_val',
			)
		),
	),
);

comment:3 follow-up: @nacin4 years ago

meta_query should ideally support the simplified version. (Same with tax_query.)

If someone mixes the simplified version with the non-simplified version, then we should just take the root-level (simplified) version.

comment:4 @scribu4 years ago

  • Keywords 2nd-opinion added; close removed

comment:5 in reply to: ↑ 3 ; follow-up: @greuben4 years ago

Replying to nacin:

meta_query should ideally support the simplified version. (Same with tax_query.)

If someone mixes the simplified version with the non-simplified version, then we should just take the root-level (simplified) version.

I agree but I think we should use non-simplified(array of arrays) because the person using non-simplified version is aware of the meta_query syntax.

comment:6 @greuben4 years ago

  • Keywords has-patch added

@greuben4 years ago

comment:7 in reply to: ↑ 5 @batmoo4 years ago

Replying to greuben:

Replying to nacin:

If someone mixes the simplified version with the non-simplified version, then we should just take the root-level (simplified) version.

I agree but I think we should use non-simplified(array of arrays) because the person using non-simplified version is aware of the meta_query syntax.

Regardless of the approach, if a dev is mixing the two, it might be worth adding a _doing_it_wrong to notify them that their meta_query syntax is incorrect and should be fixed.

@batmoo4 years ago

comment:8 @batmoo4 years ago

Patch allows meta_query to work with single arrays. Defaults to simplified version if both simple and complex versions are passed in.

@batmoo4 years ago

@batmoo4 years ago

comment:9 @batmoo4 years ago

Ignore 17235-2.diff. New patch is much cleaner. Also attached is a patch that allows single arrays for tax queries as well.

@wonderboymusic21 months ago

comment:10 @wonderboymusic21 months ago

  • Keywords 2nd-opinion removed
  • Milestone changed from Awaiting Review to 3.7

Slightly altered these in 17235.2.diff​ - Mo's original patches make sense. Moving to 3.7 for discussion.

comment:11 @lucasstark20 months ago

I think the _doing_it_wrong is the right approach here. Don't think we should three different ways to do the same thing. The codex clearly outlines how to do a meta query properly.

comment:12 @wonderboymusic19 months ago

  • Milestone changed from 3.7 to 3.8

Maybe 3.8 for this

comment:13 @dannydehaan18 months ago

I've made a different approach for this problem.

The fix:
Check if the meta_query exists, then check if it has the key "0" (so we know they're doing it right) and check if the meta_key, meta_value, meta_value_num exists, if false, then throw a _doing_it_wrong (@lucasstark) and unset the meta_query so we don't use unnecessary resources.

Off course we could fix the problem for the programmer directly, but i think this doesn't solve the problem because then they won't learn to code correctly .

@dannydehaan18 months ago

17235-notice-and-unset

comment:14 @dannydehaan18 months ago

  • Cc d.dehaan@… added

comment:15 @wonderboymusic18 months ago

  • Keywords meta-query added

comment:16 @wonderboymusic17 months ago

  • Keywords changed from has-patch, meta-query to has-patch meta-query
  • Milestone changed from 3.8 to Future Release

comment:17 @wonderboymusic11 months ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I have evolved on this: we shouldn't do it.

The use-case for not having an array of arrays is performing a meta query with only one set of conditions. That can be accomplished by using the top-level meta-related query vars. If you need more control, pass an array of arrays, even if it is only one.

Meta queries should be rare as is - if you are going to go to the trouble of using them, learn how they are constructed. Dropping the outer array requirement is barely a convenience.

Note: See TracTickets for help on using tickets.