WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 5 months ago

#17235 new defect (bug)

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

Reported by: batmoo Owned by:
Milestone: Future Release 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 3 years ago.
17235-2.diff (693 bytes) - added by batmoo 3 years ago.
17235-meta.diff (508 bytes) - added by batmoo 3 years ago.
17235-taxonomy.diff (508 bytes) - added by batmoo 3 years ago.
17235.2.diff (1.0 KB) - added by wonderboymusic 8 months ago.
17235-notice-and-unset.diff (860 bytes) - added by dannydehaan 6 months ago.
17235-notice-and-unset

Download all attachments as: .zip

Change History (22)

comment:1 in reply to: ↑ description greuben3 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 scribu3 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: nacin3 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 scribu3 years ago

  • Keywords 2nd-opinion added; close removed

comment:5 in reply to: ↑ 3 ; follow-up: greuben3 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 greuben3 years ago

  • Keywords has-patch added

greuben3 years ago

comment:7 in reply to: ↑ 5 batmoo3 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.

batmoo3 years ago

comment:8 batmoo3 years ago

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

batmoo3 years ago

batmoo3 years ago

comment:9 batmoo3 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.

wonderboymusic8 months ago

comment:10 wonderboymusic8 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 lucasstark8 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 wonderboymusic6 months ago

  • Milestone changed from 3.7 to 3.8

Maybe 3.8 for this

comment:13 dannydehaan6 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 .

dannydehaan6 months ago

17235-notice-and-unset

comment:14 dannydehaan6 months ago

  • Cc d.dehaan@… added

comment:15 wonderboymusic5 months ago

  • Keywords meta-query added

comment:16 wonderboymusic5 months ago

  • Keywords changed from has-patch, meta-query to has-patch meta-query
  • Milestone changed from 3.8 to Future Release
Note: See TracTickets for help on using tickets.