Zero value for posts_per_page value in wp_query custom instance and for 'Blog pages show at most' option

[alexvorn2]

To show no posts if the posts_per_page value is 0.

Currently for custom instances of wp_query, if the value is 0 then this is changed with the value from posts_per_page option from the database. "get_options( 'posts_per_page' )"

For home page if we set value 0 on the settings page, in wp-admin/options-reading.php, after the saves are changed, this value is changed to 1.

I think for both cases if the posts per page value is 0 then no posts should not display.

[ocean90]

What's the use case of having a query which should return 0 results?

[alexvorn2]

Replying to ocean90:

What's the use case of having a query which should return 0 results?

To hide all posts, for example for a short period of time if the site is under construction...

[wonderboymusic]

Why would you write a WP_Query by hand that wants zero posts? Why not just comment out the code that displays posts or something

[DrewAPicture]

A more valid use case for this might be if you're passing the post_per_page value through absint() or similar and zero gets returned.

I can follow the logic of -1 = unlimited, or 1 = 1, 2 = 2 and so on. So it's odd that we say, "Oh, by the way, 0 doesn't equal 0 here, it actually equals this other thing over there."

It kind of stands to reason that if I pass 0 I'd expect to get 0 results, intentional or not. Just sayin'.

[randell]

I agree with @DrewAPicture's logic on how zero is handled relative to how -1, 1, 2, 3, etc. is handled. Wasted so much time trying to figure out what's wrong with my code because of this unintuitive behavior.

[boonebgorges]

I agree that it's a bit odd to create a WP_Query with posts_per_page=0, but I also think there's no reason we shouldn't support it. The culprit appears to be https://core.trac.wordpress.org/browser/tags/4.2.2/src/wp-includes/query.php#L2460. Something like 0 === $q['posts_per_page'] || '0' === $q['posts_per_page'] is probably going to work, but this needs unit tests.

[teraviva]

I want to give you an other case. For example I have CPT Destination with Tours and Posts attached to it. And I have no need to show posts at Destination page, because I provide further links to inner categories for Tours and Info posts at this page.

[birgire]

I agree that this is a strange behavior.

To better understand it, we can map it out for version 4.8.2:

value		posts_per_page	showposts	posts_per_archive_page	posts_per_rss
--------------------------------------------------------------------------------------
null	|	default		default		default			default
''	|	default		default		default			default
0	|	default		default		default			default
'0'	|	default		default		default			default
true	|	none		1		none			1
false	|	default		default		default			default
10	|	10		10		10			10
'10'	|	10		10		10			10
'string'|	1		1		default			1
array() |	default		default		1			default
-1	|	none		none		none			none
-2 	|	2		2		2			2

where default means the stored option's value. We could e.g. use this table when writing tests and data providers.

So there are some strange inconsistencies, e.g. the true row ;-)

The scope? of the ticket seems to be adjust the the 0 and '0' rows to:

value		posts_per_page	showposts	posts_per_archive_page	posts_per_rss
-------------------------------------------------------------------------------
0	|	0		0		0			0
'0'	|	0		0		0			0

I think these two row cases, should be the same as returning an empty array from
the posts_pre_query filter,

https://core.trac.wordpress.org/browser/tags/4.8.2/src/wp-includes/class-wp-query.php#L2781

so we don't have to run those db queries. Like using $this->posts = array().

We would need to check for e.g.

if( isset( $q['posts_per_page'] ) && ( 0 === $q['posts_per_page'] || '0' === $q['posts_per_page'] ) ) {

as mentioned by @boonebgorges and similar for the other query variables.

We would also have to adjust this part accordingly:

https://core.trac.wordpress.org/browser/tags/4.8.2/src/wp-includes/class-wp-query.php#L1785-L1798

i.e.

$q['posts_per_page'] = (int) $q['posts_per_page'];

if ( $q['posts_per_page'] < -1 )
	$q['posts_per_page'] = abs($q['posts_per_page']);
elseif ( $q['posts_per_page'] == 0 )
	$q['posts_per_page'] = 1;

The case of e.g. 'showposts' => 'string' is a little bit tricky here to detect, because of the previous $q['showposts'] = (int) $q['showposts'];

Before writing any patches I think we should consider what the scope of this ticket should be,
i.e. should it be to only adjust the 0 and '0' rows or if it should also touch the true, false, 'string', array(), ... rows?

Hope this helps anyone that's going to tackle this problem.

Update: Then there's ticket #39945 for yet another edge case input value.

[Howdy_McGee]

I think we can get away with == if we convert the given value to integer after checking if we need to provide a default. Similar to the showposts conditional directly below pre_get_posts.

Giving posts_per_page a TRUE boolean already translates to 1 (post per page) so it would make sense for FALSE to translate to 0 and return 0 posts.

This certainly needs tested but from what I've run it works as expected. To compare to the previously listed chart:

value		posts_per_page	showposts	posts_per_archive_page	posts_per_rss
--------------------------------------------------------------------------------------
null	|	default		default		default			default
''	|	default		default		default			default
0	|	0		0		0			0
'0'	|	0		0		0			0
true	|	1		1		1			1
false	|	0		0		0			0
10	|	10		10		10			10
'10'	|	10		10		10			10
'string'|	default		default		default			default
array() |	default		default		default			default
-1	|	all		all		all			all
-2 	|	2		2		2			2

I think in the end it makes posts_per_page a bit more predictable for edges cases such as this. The biggest change users should be aware of is how booleans passed to posts_per_page works.

[Howdy_McGee]

Removal of empty() in favor of isset() is_numeric() and is_bool()

[molinerozadkiel]

use case for "posts_per_page" => 0 to display no posts:

I am building a ajax search bar in a header, I have all the logic already done and working from older projects, but in this case the designer had the brilliant idea to show no posts if the search bar is empty (because the results are shown within the header, so is good to now show anything if the user is not looking for something so that is won't produce visual clutter)
So, I would like to jus be able to pass a 0 there when the search bar is empty...

Besides, "0 to show 0 posts" makes sense, and not doing it like this makes the system more prone to errors and harder to debug, as stated by other users before.
0 counts as "False" in many cases so I imagine the problem is problbly comming from there somehow

[Howdy_McGee]

Patch refreshed to only include updates to posts_per_page (not comments_per_page or posts_per_rss). Includes test for posts_per_page based on the table from reply #10. Progresses WP_Query tests in #49149. Ultimately makes posts_per_page more predictable.

[hellofromTonya]

As this ticket proposes changes to WP_Query, a commit would need to land early in the dev cycle to allow for a longer amount of time for testing and feedback.

[audrasjb]

As per today's bug scrub, I removed the needs-unit-test keyword and added needs-testing-info

@Howdy_McGee would you be able to provide steps/details to help contributors to reproduce the issue and test that your patch solves the issue?

Thank you!

[Howdy_McGee]

@audrasjb The goal of this patch is to make WP_Query's pre_get_posts more predictable.

Prior to this patch:

Passing true would show all posts, but the WP_Query object would only show posts_per_page => 1

Passing 0 would show the default of 10 posts and update the WP_Query property posts_per_page => 10

Post-patch fixes these inconsistencies.

Passing true will interpret the boolean as 1 and only show 1 post. posts_per_page => 1

Passing 0 will query no posts. posts_per_page => 0

For a wider view of how the pre_get_posts argument gets handled, you can view the table above.

The added Unit Tests also cover an area previously untested for WP_Query.

---

One way to test this patch is to run a pre_get_posts against X blog posts.

<?php
add_action( 'pre_get_posts', function( $query ) {

        if( $query->is_main_query() ) {
                $query->set( 'posts_per_page', 0 );     // Shows 0 posts
                $query->set( 'posts_per_page', true );  // Shows 1 post per page
        }

} );

---

While I do think this is an update worth making, it may cause initial confusion for developers where their code already gets a dynamic value for pre_get_posts. For example:

'pre_get_posts' => get_option( 'nonexistent_pre_get_posts_key' )

Where get_option() (or however a developer may be getting their value) returns false, the system interprets it as 0 and returns no posts. With that being said, spitting out WP_Query will reflect pre_get_posts as 0, denoting no posts to show, which is technically correct.

[oglekler]

If in the result $limit is equal to LIMIT 0, 0 there is no point to make a request to the DB at all. If it weren't for limit filters, it would be good to make $this->posts = array() and exit the method straight away at the moment when posts_per_page is equal to zero.

Due to lack of final decision and activity lately, I am moving this ticket to the 6.4 milestone.

[oglekler]

This ticket was discussed during a bug scrub,

I am adding performance focus because from my point of view, some patch of the code can avoid execution in this scenario but due to impact it can have, it should be done very accurate. So, we need more brains on this one.

Add props to @mukesh27

[oglekler]

Because this is an early ticket, I am moving it into the 6.5 milestone. If there will not be progress, next time it should be rescheduled to the Future release.

[alexvorn2]

Replying to oglekler:

Because this is an early ticket, I am moving it into the 6.5 milestone. If there will not be progress, next time it should be rescheduled to the Future release.

what do you mean "early ticket"?
This ticket is from 10 years ago.

[joemcgill]

Replying to alexvorn2:

Replying to oglekler:

Because this is an early ticket, I am moving it into the 6.5 milestone. If there will not be progress, next time it should be rescheduled to the Future release.

what do you mean "early ticket"?
This ticket is from 10 years ago.

Tickets are marked "early" if they need to land early in a release cycle to have longer time for testing. For it to land in 6.5, the latest patch from @Howdy_McGee needs testing and someone to review the approach from a technical point of view to ensure it doesn't introduce other side-effects. @audrasjb are you interested in picking this back up for this cycle?

My suggestion to future bug gardeners is that if this doesn't make the 6.5 cutoff and no further progress is made, that this be set as Future Release.