Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#37378 closed defect (bug) (fixed)

4.6 breaks tax query alterations (i.e. INNER_JOIN)

Reported by: nimmolo's profile nimmolo Owned by: boonebgorges's profile boonebgorges
Milestone: 4.6 Priority: normal
Severity: normal Version: 4.6
Component: Taxonomy Keywords:
Focuses: Cc:

Description (last modified by ocean90)

It seems altering the tax query $pieces['join'] or $pieces['orderby'] is breaking the query.

The following function is from repository plugin Simple Taxonomy Ordering. The plugin function works in 4.5.3., no errors.

*       Re-Order the taxonomies based on the tax_position value
*       @since 0.1
public function yikes_alter_tax_order( $pieces, $taxonomies, $args ) {
        foreach( $taxonomies as $taxonomy ) {
                // confirm the tax is set to hierarchical -- else do not allow sorting
                if( $this->yikes_is_taxonomy_position_enabled( $taxonomy ) ) {
                        global $wpdb;
                        $pieces['join'] .= " INNER JOIN $wpdb->termmeta AS term_meta ON t.term_id = term_meta.term_id";
                        $pieces['orderby'] = "ORDER BY term_meta.meta_value";
        return $pieces;

Throws this error message:

WordPress database error You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ORDER BY term_meta.meta_value ASC' at line 1 for query SELECT t.*, tt.* FROM wp_fv47w7_terms AS t INNER JOIN wp_fv47w7_term_taxonomy AS tt ON t.term_id = tt.term_id INNER JOIN wp_fv47w7_termmeta AS term_meta ON t.term_id = term_meta.term_id WHERE tt.taxonomy IN ('category') ORDER BY ORDER BY term_meta.meta_value ASC made by WP_List_Table->display, WP_Terms_List_Table->display_rows_or_placeholder, get_terms, WP_Term_Query->query, WP_Term_Query->get_terms

Is the syntax in fact incorrect?

I'm not great with query syntax, but if incorrect it seems it would have thrown the error in WP 4.5.3

Change History (5)

#1 @tw2113
8 years ago

As an alternative, it's possible that the plugin authors don't need to include the extra "ORDER BY", which is getting duplicated in this case.

#2 @nimmolo
8 years ago

  • Resolution set to invalid
  • Status changed from new to closed

Yep. that's it. Thanks

#3 @ocean90
8 years ago

  • Component changed from Query to Taxonomy
  • Description modified (diff)
  • Keywords 2nd-opinion added
  • Milestone changed from Awaiting Review to 4.6
  • Resolution invalid deleted
  • Status changed from closed to reopened

With the new WP_Term_Query class the ORDER BY keyword is added automatically: trunk/src/wp-includes/class-wp-term-query.php?marks=605,621,624#L572

This wasn't the case before: tags/4.5.3/src/wp-includes/taxonomy.php?marks=1512-1535,1598,1604,1612#L1589. See also the meta handling which is now in WP_Term_Query::parse_orderby_meta().

@boonebgorges Should we strip any existing keywords before adding our own?

#4 @boonebgorges
8 years ago

  • Keywords 2nd-opinion removed
  • Owner set to boonebgorges
  • Status changed from reopened to assigned

Should we strip any existing keywords before adding our own?

I think the better option is probably to rework the order (har har) in which the ORDER BY clause is built, so that the value passed to the 'terms_clauses' filter is the same as in 4.5.

#5 @boonebgorges
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 38099:

Taxonomy: Improve back compat of values passed to 'terms_clauses' filter.

Prior to the introduction of WP_Term_Query, the 'orderby' clause
passed to the 'terms_clauses' filter was prefixed by ORDER BY. After
WP_Term_Query, this was not the case; ORDER BY was added after the
filter. As such, plugins filtering 'terms_clauses' and returning an
'orderby' clause beginning with ORDER BY resulted in invalid syntax
when WP_Term_Query prepended a second ORDER BY keyword to
the clause.

This changeset rearranges the way the 'orderby' clause is built so that
it will be passed to 'terms_clauses' in the previous format.

Fixes #37378.

Note: See TracTickets for help on using tickets.