Error when WP_Query parses orderby array in function parse_order

[oloynet]

Example of query where I want to order by two custom fields in meta query
start_date_order and is_sticky

<?php
$args = array(
    'no_found_rows'  => true ,
    'posts_per_page' => 4,
    'post_type'      => 'event',
    'post_status'    => 'publish',

    'meta_query' => array(
        'relation' => 'AND',
        array(
            'key'  => 'start_date_order',
            'type' => 'UNSIGNED',
        ),
        array(
            'key'  => 'is_sticky',
            'type' => 'UNSIGNED',
        ),
    ),
    'orderby' => array(
        'start_date_order' => 'DESC',
        'is_sticky'        => 'ASC',
    ),
);

$result_query = new WP_Query( $args );

echo $result_query->request;

The wrong SQL query generated is

SELECT wp_posts.ID
FROM wp_posts
INNER JOIN wp_postmeta ON ( wp_posts.ID = wp_postmeta.post_id )
INNER JOIN wp_postmeta AS mt1 ON ( wp_posts.ID = mt1.post_id )
WHERE 1=1
AND (
    wp_postmeta.meta_key = 'start_date_order'
    AND mt1.meta_key = 'is_sticky'
)
AND wp_posts.post_type = 'event'
AND ((wp_posts.post_status = 'publish'))
GROUP BY wp_posts.ID
ORDER BY CAST(wp_postmeta.meta_value AS UNSIGNED) DESC
LIMIT 0, 3

The '$primary_meta_query' var in method parse_order( $order ) is set forever with the first item of '$meta_clauses' array

see line 2336 /wp-includes/query.php

<?php
primary_meta_query = reset( $meta_clauses );

I quickly fix the problem with the following PHP code. Could use a native array function from PHP.

<?php
//$primary_meta_query = reset( $meta_clauses );

$primary_meta_query = array();
foreach( $meta_clauses as $meta_clause ) {
    if( $meta_clause['key'] == $orderby ) {
        $primary_meta_query = $meta_clause;
        break;
    }
}

Now the good SQL query is generated with two columns for ordering:

SELECT wp_posts.ID
FROM wp_posts
INNER JOIN wp_postmeta ON ( wp_posts.ID = wp_postmeta.post_id )
INNER JOIN wp_postmeta AS mt1 ON ( wp_posts.ID = mt1.post_id )
WHERE 1=1
AND (
    wp_postmeta.meta_key = 'start_date_order'
    AND mt1.meta_key = 'is_sticky'
)
AND wp_posts.post_type = 'event'
AND ((wp_posts.post_status = 'publish'))
GROUP BY wp_posts.ID
ORDER BY CAST(wp_postmeta.meta_value AS UNSIGNED) DESC, CAST(mt1.meta_value AS UNSIGNED) ASC
LIMIT 0, 3

[oloynet]

The correct fix is

<?php
$primary_meta_query = false;
foreach( $meta_clauses as $meta_clause ) {
    if ( $meta_clause['key'] == $orderby ) {
        $primary_meta_query = $meta_clause;
        break;
    }
}
if( ! $primary_meta_query ) {
    $primary_meta_query = reset( $meta_clauses );
}

[swissspidy]

Hey there,

Thank you very much for your report and welcome to WordPress Trac! The change looks reasonable at first glance.

It would be nice to have ​unit tests showing this bug and how the patch resolves it. Also, the patch should ​ideally have a .patch or .diff extension and not contain any start patch and end patch comments.

Let me know if you're familiar with automated testing or not. I'm sure someone else might be able to step up here.

[oloynet]

Patch for bur report 38442 with svn diff

[oloynet]

Hello,

I'm not a fluent user with unit tests on Wordpress.
I've made a new patch with svn diff to do the job

Olivier

[desrosj]

I dug into this to understand it a bit better. In the current state, ordering by multiple meta keys only works when using explicit indexes. For example, if the query in your example is changed to this, @oloynet, I am seeing the correct SQL generated:

'meta_query' => array(
	'relation' => 'AND',
	'date_meta' => array(
		'key'  => 'start_date_order',
		'type' => 'UNSIGNED',
	),
	'sticky_meta' => array(
		'key'  => 'is_sticky',
		'type' => 'UNSIGNED',
	),
),
'orderby' => array(
	'date_meta'.  => 'DESC',
	'sticky_meta' => 'ASC',
),

38442.diff​ is a refresh of the original patch accompanied by unit tests and a few changes:

• We don't need to declare $primary_meta_query = false;. That is redundant since the same declaration is just outside the if statement.
• Use strict comparison wghen comparing key with $orderby.
• Add a check that $meta_clause['key'] is actually set to avoid PHP notices.

If you only add the unit test, the tests will fail. Adding the other part of the patch will result in the tests passing.

@boonebgorges Are you able to review?

[boonebgorges]

Thanks, @desrosj. I'll review next week.

[boonebgorges]

Logic of 38442.diff​ looks good to me.

Ideally, we'd also have a test that demonstrates the bug's effect on query results, not just the MySQL query string.