Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#42327 closed defect (bug) (fixed)

WP_Term_Query returns an empty array instead of 0

Reported by: xparham's profile xParham Owned by: boonebgorges's profile boonebgorges
Milestone: 5.1 Priority: normal
Severity: normal Version: 2.8.4
Component: Taxonomy Keywords: has-unit-tests has-patch
Focuses: Cc:

Description

When running a term query using the parent or child_of query parameters, and count as the term fields to query for, if the parent has no descendants the query doesn't respect the fields parameter and returns an empty array instead of 0.

Attachments (2)

42327.patch (384 bytes) - added by xParham 6 years ago.
42327.2.diff (2.7 KB) - added by birgire 6 years ago.

Download all attachments as: .zip

Change History (13)

@xParham
6 years ago

#1 @xParham
6 years ago

I have attached a patch.

This in particular for me, is causing a fatal error on the edit-tags.php screen, when a filter that I have on the main query for the terms list sets the parent parameter to one with no descendants.

Having an array returned instead of 0 will make the WP_List_Table::set_pagination_args() to fail because the pagination args set in WP_Terms_List_Table::prepare_items() uses wp_count_terms() which returns an array instead of integer in this case, which is then passed as the total_items for the pagination args:

Fatal error: Unsupported operand types in /wp-admin/includes/class-wp-list-table.php on line 284

Last edited 6 years ago by xParham (previous) (diff)

#2 @westonruter
6 years ago

@xParham is this a new issue in 4.9 or is it also present in 4.8 and before?

#3 @xParham
6 years ago

@westonruter this is also present in 4.8 and before.

#4 @westonruter
6 years ago

  • Version changed from trunk to 4.6

According to git-blame it looks like this was introduced in 4.9 via #35381 from [37572].

#5 @xParham
6 years ago

@westonruter That's when the WP_Term_Query was introduced which borrowed the code from get_terms(). Avoiding the query and returning an empty array if the queried parent/child_of term has no descendants had been in get_terms() since [6118] but there was no count option back then. The bug was actually introduced in [13491] via #10746 when the count option for the fields parameter was added but the part which handled the early bailing of the function wasn't updated to return 0 instead of an empty array if a count is the expected return value.

#6 @westonruter
6 years ago

  • Version changed from 4.6 to 2.8.4

@birgire
6 years ago

#7 @birgire
6 years ago

  • Keywords has-unit-tests has-patch added

I think the terms property should also be set as an empty array, so I slightly modified the patch from 42327.patch to:

if ( 'count' == $args['fields'] ) {
	return 0;
} else {
	$this->terms = array();
	return $this->terms;
}

to avoid the mismatch between the query() method returning an empty array and the terms property being null, for the case when fields is not count.

42327.2.diff contains tests.

Last edited 6 years ago by birgire (previous) (diff)

#8 @boonebgorges
6 years ago

  • Milestone changed from Awaiting Review to 5.0

Thanks for the patch and especially for the excellent tests.

#9 @boonebgorges
6 years ago

  • Owner set to boonebgorges
  • Resolution set to fixed
  • Status changed from new to closed

In 42209:

Improve data types returned from empty hierarchical term queries.

When querying for 'count', ensure that 0 is returned. Otherwise,
ensure that it's an array.

Props xParham, birgire.
Fixes #42327.

This ticket was mentioned in Slack in #core by xparham. View the logs.


6 years ago

#11 @johnbillion
5 years ago

  • Milestone changed from 5.0 to 5.1
Note: See TracTickets for help on using tickets.