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.

CommentFileSizeAuthor
#9 views-get-total-rows.patch787 bytesmarcp
#5 views-get-num-rows.patch775 bytesmarcp
views-allow-count-query-to-run.patch1.4 KBmarcp

Comments

dawehner’s picture

dawehner
Primary language German
commented

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

Perhaps there should be a setting.

marcp’s picture

marcp commented
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. http://drupal.org/node/347819#comment-2071008 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

dawehner
Primary language German
commented

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

merlinofchaos commented

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

marcp commented
StatusFileSize
new views-get-num-rows.patch775 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

marcp commented
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

marcp commented

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

merlinofchaos commented

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

marcp commented
StatusFileSize
new views-get-total-rows.patch787 bytes

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

merlinofchaos’s picture

merlinofchaos commented
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.