Forum OpenACS Development: Re: Small feature enhancement for template::list

Collapse
3: Re: Small feature enhancement for template::list (response to 2)
Posted by Antonio Pisano on 04/24/15 07:39 PM
Dear Gustaf,

it seemed strange to me too that this value was not stored somewhere, but truth is it is not!

I think the reason is that the full count is not required to the paginator, as it really just needs to know how many records get to populate all the pages and then stop.

The original, untampered with, paginator query is never called: it is always wrapped into "offset" clauses that limit its results to number_of_pages * rows_per_page, and also overwritten, so I had to retrieve the original one with some code replication...

The uplevel is required so I can see filter vars in the caller scope. I can do better than that I've seen, because I have a reference to filter names and values in the list.

I think from a performance perspective, as the value is not anyway, it is ok to have a different proc doing this calculation. I could change the template::list::prepare so a reference to the original query is maintained and I can retrieve it, then remove the upvars by looking into list properties instead.

What do you think?

For reference:

http://www.openacs.org/api-doc/proc-view?proc=template%3a%3alist%3a%3aprepare&source_p=1

Collapse
4: Re: Small feature enhancement for template::list (response to 3)
Posted by Gustaf Neumann on 04/24/15 08:36 PM
The paginator has to know, how many pages should be offered.

Storing the bound sql query to save on the uplevel is performance-wise not a big difference. sql-queries are (in good cases) in the range of milliseconds, tcl/xotcl statements in the range of micro-seconds. so, saving a sql query (especially an aggregating query) is certainly useful, since people use pagination on large tables, where the count query might be as well slow.

Collapse
5: Re: Small feature enhancement for template::list (response to 4)
Posted by Antonio Pisano on 04/25/15 12:22 PM
Ok then, I will look for an elegant way to exec the count when list is created, then store the value in the list properties.

To my next proposal 😊

Antonio

Collapse
6: Re: Small feature enhancement for template::list (response to 5)
Posted by Antonio Pisano on 04/25/15 08:56 PM
Think I have come out with the proper solution:

on template::list::prepare, I save an entry for "count_query" in the list properties, then pass it to template::paginator::create together with "page_query_substed" as before.

Here, I check for a cache entry for the full_row_count, as happens for paginated row_ids. As happened before, if row_ids are not in the cache, I call paginator query into template::paginator::init. Into this proc I have added also the call to the count_query and the saving of the value in the cache.

This way now paginator has a new property called "full_row_count", which can be retrieved from template::paginator::get_full_row_count.

To have a common interface for getting the count in template::list, I have created a proc called template::list::get_rowcount. On a paginated list, it calls template::paginator::get_full_row_count, otherwise it just returns the size of the undelying multirow.

Its easier done than explained... Anyway, checks on both paginated and not paginated lists on my instance work. Is it ok if I commit the change?

Antonio

Collapse
7: Re: Small feature enhancement for template::list (response to 6)
Posted by Gustaf Neumann on 04/27/15 07:53 AM
sounds perfect!
Collapse
8: Re: Small feature enhancement for template::list (response to 7)
Posted by Antonio Pisano on 04/28/15 11:46 AM
Change has been committed.

On up-to-date systems, you will find the new proc

template::list::get_rowcount

Giving the proper count of the number of records retrieved by a template::list

Thanks to Gustaf for his suggestions on this!

All the best