WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 2 years 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 5 years ago.
17235-2.diff (693 bytes) - added by batmoo 5 years ago.
17235-meta.diff (508 bytes) - added by batmoo 5 years ago.
17235-taxonomy.diff (508 bytes) - added by batmoo 5 years ago.
17235.2.diff (1.0 KB) - added by wonderboymusic 3 years ago.
17235-notice-and-unset.diff (860 bytes) - added by dannydehaan 3 years ago.
17235-notice-and-unset

Download all attachments as: .zip

Change History (23)

#1 in reply to: ↑ description @greuben
5 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.

#2 @scribu
5 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',
			)
		),
	),
);

#3 follow-up: @nacin
5 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.

#4 @scribu
5 years ago

  • Keywords 2nd-opinion added; close removed

#5 in reply to: ↑ 3 ; follow-up: @greuben
5 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.

#6 @greuben
5 years ago

  • Keywords has-patch added

@greuben
5 years ago

#7 in reply to: ↑ 5 @batmoo
5 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.

@batmoo
5 years ago

#8 @batmoo
5 years ago

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

@batmoo
5 years ago

#9 @batmoo
5 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.

#10 @wonderboymusic
3 years 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.

#11 @lucasstark
3 years 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.

#12 @wonderboymusic
3 years ago

  • Milestone changed from 3.7 to 3.8

Maybe 3.8 for this

#13 @dannydehaan
3 years 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 .

@dannydehaan
3 years ago

17235-notice-and-unset

#14 @dannydehaan
3 years ago

  • Cc d.dehaan@… added

#15 @wonderboymusic
2 years ago

  • Keywords meta-query added

#16 @wonderboymusic
2 years ago

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

#17 @wonderboymusic
2 years 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.