Normally the count query only runs when the view's pager is turned on. There's a fix that must have worked at some time here in #347819: How count results of one view ? but it doesn't work properly now.

The attached patch attempts to fix this by pulling the query call out of the "not empty items_per_page" check.

Members fund testing for the Drupal project. Drupal Association Learn more


dawehner’s picture

This introduces a new query for every non-pager view.

Perhaps there should be a setting.

marcp’s picture

Status: Active » Needs review

It should only introduce the new query when the $view->get_total_rows flag is set. It's already querying when there's a pager involved. makes me believe that this was working at one point.

Normal non-pager views shouldn't have $view->get_total_rows set -- see that link above for the trick to get it to work.

dawehner’s picture

Ah ok. Thanks for it. Wouldn't it be cool, if this kind of documentation whould be somehow in views? For example make a patch to advanced help, or do some inline comments.

The patch is fine.

merlinofchaos’s picture

Before I commit this i Just want to be sure: The only situation where this is actually broken is where items_per_page is set to 0, right? So you're always getting all results and want to know how many results were loaded?

Note that count($view->result) will provide this value in that case, without running an extra query. I'm not completely sure that we want to run an extra query to get data you already have.

marcp’s picture

775 bytes

Yep, that's right. I tested count($view->result) and it works perfectly, so we definitely should not run another count query.

If the format of $view->result ever changes, then any code that depends upon count($view->result) being the number of rows will be broken.

It seems more likely that total_rows would remain a fixture in the view, so maybe the fix is just to set total_rows to count($view->result) when items_per_page is set to 0 and get_total_rows is set TRUE. That's what this new patch does.

Really nothing needs to be done -- there are just multiple ways to get the "total rows" depending on the pager settings. The Views API would be cleaner with a single way to get the number of rows.

Another option would be to ditch the check on get_total_rows and just set total_rows all the time since there's no need to run an extra query. I didn't look into the implications of having total_rows set. There's something going into the views cache, but total_rows is not used much in the views codebase.

marcp’s picture

Title: Let $view->get_total_rows run the count query » Should $view->total_rows always have the count of total rows?

Just changing the title.

marcp’s picture

One benefit to always setting $view->total_rows is that there would be no need to implement hook_views_pre_execute() just to set $view->get_total_rows = TRUE;.

merlinofchaos’s picture

Ok, I think this is right.

One minor nit: The comment should specify that we're only setting this when all items were requested; right now it suggests that all non-paged-queries will be counted like that, which isn't accurate.

marcp’s picture

787 bytes

Not sure if this is any better, but here's a shot at some better wording in the comment.

merlinofchaos’s picture

Status: Needs review » Fixed

Changed slightly (there's no need to check to see if get_total_rows is set, since that is only there to avoid a query that isn't run in this case) and committed to all branches.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.