#42469 closed defect (bug) (fixed)
WP_Query found_posts inconsistent data type.
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.5 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Query | Keywords: | has-unit-tests has-patch has-dev-note |
Focuses: | Cc: |
Description
Attachments (5)
Change History (13)
#2
@
7 years ago
Thanks for the patch @PressLabs.
The database query returns a string, so this seems a valid concern.
I checked the other query classes and they also convert it to integer.
In WP_Comment_Query
we see:
$found_comments_query = apply_filters( 'found_comments_query', 'SELECT FOUND_ROWS()', $this ); $this->found_comments = (int) $wpdb->get_var( $found_comments_query );
and in WP_Site_Query
we have:
$found_sites_query = apply_filters( 'found_sites_query', 'SELECT FOUND_ROWS()', $this ); $this->found_sites = (int) $wpdb->get_var( $found_sites_query );
Similarly in WP_User_Query
:
$this->total_users = (int) $wpdb->get_var( apply_filters( 'found_users_query', 'SELECT FOUND_ROWS()' ) );
I would suggest making it more readable and change:
$this->found_posts = intval( $wpdb->get_var( apply_filters_ref_array( 'found_posts_query', array( 'SELECT FOUND_ROWS()', &$this ) ) ) );
to
$found_posts_query = apply_filters_ref_array( 'found_posts_query', array( 'SELECT FOUND_ROWS()', &$this ) ); $this->found_posts = (int) $wpdb->get_var( $found_posts_query );
in WP_Query
.
#3
@
7 years ago
I'm unsure what the stance is on the matter, but do you in core enforce type casting on apply_filter
or is it too defensive and out of scope.
I'm asking because for me it would be reasonable if I got an int
I should return an int
but in the PHP world things grow hazy on the matter.
Should I cast also the found_posts
filter a couple of lines below, as well?
#4
@
7 years ago
- Keywords dev-feedback has-unit-tests has-patch added
I added a type test in 42469.2.diff and removed some extra tabs.
There are some cases in core with (int) apply_filters( ... );
so I don't think that's a problem with that structure. It sounds logical to convert the output from the found_posts
filter to integer as well, as the definition of the found_posts
property says it's of type int
:
/** * The amount of found posts for the current query. * * If limit clause was not used, equals $post_count. * * @since 2.1.0 * @var int */ public $found_posts = 0;
I added the int
cast on the found_posts
filter in 42469.3.diff, with an extra test.
But the general concern is if anyone out there is assuming that found_posts
is a string, for example:
if( '1' === $query->found_posts ) { // ... }
So I tagged the ticket with dev-feedback
.
#6
@
5 years ago
- Milestone changed from Awaiting Review to 5.5
- Owner set to SergeyBiryukov
- Status changed from new to reviewing
#8
@
5 years ago
- Keywords has-dev-note added; dev-feedback removed
This received a call out in the Miscellaneous Developer Changes in 5.5 dev note: https://make.wordpress.org/core/2020/07/29/miscellaneous-developer-focused-changes-in-wordpress-5-5/.
You can remove the patch that has the mismatching ticket number, my bad.