WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 9 months ago

#16814 closed defect (bug) (fixed)

WP_Query doesn't check orderby for meta_query

Reported by: dalesaurus Owned by: kovshenin
Milestone: 4.1 Priority: normal
Severity: normal Version: 3.1
Component: Query Keywords: has-patch commit
Focuses: Cc:

Description

With the new join style in 3.1 for Custom Post Types, an array must be set on WP_Query with meta_query. In query.php line 2274 all cases of orderby are processed, specifically creating an array called $allowed_keys.

This check looks only for the deprecated meta_key. This will cause any sorting on a Custom Post Type meta_value to fail.

// Fix proposal (until meta_key goes away)
if ( !empty($q['meta_query']) || !empty($q['meta_key']) ) {
    $allowed_keys[] = $q['meta_key'];
    $allowed_keys[] = 'meta_value';
    $allowed_keys[] = 'meta_value_num';
}

Temporary workaround for people with broken sorts is to set meta_key in addition to setting the meta_query variable.

// Example 1
set_query_var( 'meta_query', array( array( 'key' => 'my_key_name',  'compare' => '>','value' => '1','type' => 'NUMERIC' ) ) );
set_query_var( 'orderby','meta_value' );
set_query_var( 'meta_key', 'my_key_name' ); // need this


// Example 2
$wpq->set( 'meta_query', array( 
          array(
                'key' => 'my_key_name',
                'compare' => '>',
                'value' => '1',
                'type' => 'NUMERIC'
          ) 
       )
    );
$wpq->set( 'orderby','meta_value' );
$wpq->set( 'meta_key', 'my_key_name' ); // need this

Attachments (3)

16814.diff (2.4 KB) - added by kovshenin 9 months ago.
16814.2.diff (3.3 KB) - added by boonebgorges 9 months ago.
16814.3.diff (3.5 KB) - added by kovshenin 9 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 @hakre4 years ago

in your example in the description: $wpq ?

comment:2 @scribu4 years ago

Related: #16812

comment:3 @dalesaurus4 years ago

$wpq can be any WP_Query object.

comment:4 @scribu4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to duplicate
  • Status changed from new to closed
  • Summary changed from WP_Query doesn't check orderby for Custom Post Types querying with meta_query to WP_Query doesn't check orderby for meta_query

First of all, this has nothing to do with custom post types. It affects regular posts as well.

I'm closing this ticket as a duplicate of #15031, since it's basically the same problem.

Feel free to join the discussion there.

comment:5 @scribu4 years ago

Also note that 'meta_key', 'meta_value' and 'meta_compare' aren't deprecated.

Last edited 4 years ago by scribu (previous) (diff)

@kovshenin9 months ago

comment:6 @kovshenin9 months ago

  • Keywords has-patch added
  • Milestone set to Awaiting Review
  • Resolution duplicate deleted
  • Status changed from closed to reopened

I'd like to reopen this, because r25255 has added even more inconsistencies between a meta_query and a combination of meta_key, meta_value, etc. When providing a meta_type key, orderby=meta_value will cast to that type, but a type key in a meta_query won't.

In fact, a meta_query won't even add meta strings to $allowed_keys as stated by the reporter. This is a bit ironic, because a combination of meta_key, meta_value, etc. does resolve into a meta_query.

16814.diff is a proposed fix + tests.

@boonebgorges9 months ago

comment:7 @boonebgorges9 months ago

  • Milestone changed from Awaiting Review to 4.1

Confirmed. Good catch.

Your fix looks correct to me. To spell it out a bit more for posterity: when passing meta_key, etc to WP_Query, they are parsed into a WP_Meta_Query object: https://core.trac.wordpress.org/browser/tags/4.0/src/wp-includes/query.php#L2377. Any combination of meta_key, meta_value etc that were passed to WP_Query always gets set as the 0th member of the meta_query->queries array. So looking at the 0th index, as in your patch, will find the right orderby regardless of whether you're using meta_key etc or a proper meta_query param.

16814.2.patch adds another assertion to your test, to show how the meta_key gets parsed together with, and takes precedence over, the meta_query arguments.

comment:8 @dalesaurus9 months ago

I was able to recreate the queries that prompted this ticket back in 2011. The 16814.2.patch? does resolve the orderby issue patched against trunk r29617.

Thanks to you both for digging this up!

comment:9 follow-up: @boonebgorges9 months ago

  • Keywords commit added

dalesaurus - Thanks for the confirmation!

kovshenin - You want to do the honors? :)

@kovshenin9 months ago

comment:10 in reply to: ↑ 9 @kovshenin9 months ago

Replying to boonebgorges: Your patch introduced a $primary_meta_query variable but it's not used. Was the intent to use that instead of $meta_query and avoid confusion between $meta_query, $this->meta_query and $this->get('meta_query')?

In 16814.3.diff I also changed $meta_key to $primary_meta_key and $meta_type to $sql_type.

comment:11 @boonebgorges9 months ago

Ah right, I'd forgotten that I did that. Yes, I'd originally changed the variable name because it took me a second to grok what you were using it for; I changed it back, but then forgot to remove the original $primary_meta_query declaration. Sorry about that.

I like the new variable names better, personally, but I'm happy to defer to your judgment.

comment:12 @kovshenin9 months ago

  • Owner set to kovshenin
  • Resolution set to fixed
  • Status changed from reopened to closed

In 29855:

Use the primary meta_query clause when parsing orderby in WP_Query.

When using legacy meta_key, meta_value, etc. arguments in WP_Query,
they're converted into the first clause of a meta_query. By using that
clause instead of the original arguments, we make sure that behavior is
consistent between the two available formats.

props boonebgorges.
fixes #16814.

Note: See TracTickets for help on using tickets.