Forum OpenACS Improvement Proposals (TIPs): TIP #71 (Rejected): Add Jon Griffin's new Paginator Procs to Core Release

Just to clarify: The section under Implementation relates to part a). I am not clever enough to suggest how to do b). Perhaps Lars could point me in the right direction (hint hint!).

Just to add my 2 cents. I don't have much time at all for doing this, and I think I have some newer code that I merged with Brad Duell's.

Once someone gets it working with the existing code I can make sure all the parts are there.

The most important thing is to get my query procs which do all the work in the DB and I can do paginated sorts of 20K pages in under one sec on both PG and Oracle.

The rest of the stuff, while nice if it is easy to add, can be added later or just a switch.

Again, the DB procs for paginating results sets in the DB will make your paginations much quicker.

Jon: Where's the code you've merged with Brad's, is that the current set of files on your site?

Jade: I'm going to look into this with forums starting tomorrow, if all goes well (I'm busy today).

Everyone: why are e-mail responses broken on this box?  I remember seeing some comments regarding qmail earlier but ignored them ...

Joel,

To be complete the new pagination procs should replace the existing ones in acs-templating and work with list-builder.

The proposal is in two parts for that reason. The intention is to propose that we incorporate Jon's new work by making it accessible via list-builder. In the meantime though, anyone who has used these procs for a specific application will have their site break every time they do an 'Update from Repository' because the procs are not in ../acs-tcl/tcl/

I am proposing a two stage process.

Jon's paginator are superior to list-builder paginator, then should replace the paginator method implemented on the list-builder, as simple as this, it's a matter of need for scalability (we have used those procs on production with excellent results).

Fools rush in where angels fear to tread......

It is not a lack of willingness that makes me hesitate to volunteer to do the integration, purely a lack of knowledge! I will agree do the work, but I have no idea at all how long it will take me. I have to learn how to actually use list builder first, then work out how the pagination works and then try to modify it in line with Jon's code (which looks elegant and simple at first sight).

But 'in at the deep end' is often the best way!

It needs to be done since the current situation is no good for anyone.

Regards
Richard

RE: breaking existing code...

When merging Jon's code, why not rename it (or place it in a namespace) such that there will be no conflict between the existing code (not in the core) and the new core code? At the same time deprecate the non-core code so that developers are aware that it will be removed at some point. In the mean time, no code should break.

I have been studying the list-builder source code and trying to gain a basic understanding of what happens. I think Roberto is correct when he says that we need to extract what is good about Jon's paginator and incorporate that into list builder. Jon also mentioned that the important bit is the db procs from his work.

However, looking throught the list builder it seems to me that      its job is to poke around in the variable scope of the calling tcl and the display template to facilitate user modification of the underlying query. It seems from the comments that the intention was always to paginate in the db and so I wonder whether our supposed problem stems from a misuse of list builder rather than anything wrong with the code. If so we need to establish whether list builder builds queries using the postgres 'offset' and 'limit' keywords and then progress accordingly.

If it does then we should try to find where the performance issues or bugs stem from.

If it does not we need to change the paginator procs to support this way of doing it.

So in essence I gues I am now recommending voting against my own proposal! Sorry.

I will do some more work and then propose something more suitable.

Regards
Richard

Don,

I knew that you were working on this (secretly hoped you would post here!!).

I can't tell you how relieved I am to hear that you have worked through this issue because it would have taken me many days and a lot of experimentation before I fully understood it. I had concluded that there seemed to be a lot of surplus munching going on and was thinking along the lines of proposing to do what you have just suggested.

From my reading so far, even if you specified all the page parameters correctly, the paginator was still asking postgres to run through the entire unfiltered dataset (actually more than once) before arranging the required page.

It seems to me that the best way to paginate data is to ask postgres to do it (as in Jon's code). We don't need to import Jon's code to achieve this but as you say, we would need to make sure that the result of the user clicking on a url is a query that returns only the data required for the selected page, and in the correct order - and nothing more.

I have used 'Limit' and 'Offset' for paginating a 3000 row dataset and it is a really fast way to restrict the results. So any means of exposing this functionality through list builder gets my vote.

If there is anything that I can do to help please let me know.

Regards
Richard

So Don, now that you've taken this on and figured out what's going on with the current code, what pagination do your recommend OpenACS use going forward? The old crappy List Builder pagination? The improved-by-Don but still Evil pagination? Something basically based on Jon Griffin's paginator procs? Or what?

I'm actually rather astonished that this thread seems to have generated so much debate. The List Builder's built-in pagination sucks, this has been well known for a long time. Lars wrote all of List Builder, and if I remember correctly, he himself has said in these Forums that its built-in pagination sucks and should be fixed.

Jon Griffin's paginations procs are not "new", they've been around for at least a year now (maybe two). Several people have used them (I have not), and everyone who has seems to agree that they are a major improvement over the existing lousy List Builder pagination.

Joel, your heart is in the right place, but you're putting up straw men. If the toolkit is broken, it needs to be fixed - and AFAICT everyone who's examined the issue seems to agree that the List Builder pagination is definitely broken. If there is insufficient, contradictory, or just plain misleading documentation, then that is also a breakage that needs to be fixed - but that second breakage is in no way a justification for stopping someone from fixing the first breakage.

By all means, change the proc names or do whatever else is necessary or desirable to integrate Jon's pagination implementation smoothly into OpenACS - but do it! Now, if Don has figured out an even better solution, way cool, but please, somebody pick one of the improvements available and actually put it into the toolkit.

Since Don is willing to take this on and he's looked closely at it, I vote to reject this particular way of implementing it, and let Don take care of it. Also, since this is pretty much a bug fix in my opinion, I don't think Don's fix requires a TIP. This one probably didn't either, but it probably saved you some work!

Well, Andrew, any time you find a place where you think the toolkit's broken, feel free to offer to fix it rather than just bitch about it, OK? List builder pagination can be made efficient, and the paradigm's very easy to work with, I see no reason to add yet another way to do it. REJECT

I've commited my improvements to the list builder's pagination code. Essentially I've sped it up by creating a separate paginator instance for each PAGE GROUP rather than one for the ENTIRE LIST. This means that on openacs.org's Q&A forum, rather than filling a cache of 5500 entries whenever the cache is flushed, we're only filling 330, using LIMIT+OFFSET/ROWNUM tricks. We then get the benefit of having cached the keys for the current page group until there's another post in the case of forums (not all clients of the list builder enable caching, of course). I've also gotten rid of the Tcl-based sorting of the keys returned for use in the page display query's IN (keys) clause, a minor speedup. There's probably still a lot of room for improvement, i.e. I can make lighter-weight list builder specific code to execute and cache the pagination query, etc. I'll poke at it periodically. Currently the OpenACS Q&A forum takes about 1.6 seconds to display if the cache is flushed. It took about 18 seconds using the existing list builder. When the cache is available it takes about 750 milliseconds. Much of that seems to be running the listtemplate tag as the shorter the number of rows, the faster it goes. Notice that performance of this can be impacted by changing the number of rows per page, and page groups per navigation widget, so I'll probably add these as parameters to forums at some point. You get quicker page displays by shrinking those numbers. And of course I haven't added the performance improvements to forums itself that I have in mind ... those will help, too.

I'm sure I speak for lots of people when I say - Thank you Don. 😊

Yes, this works for Oracle as well as Postgres, I have both on my server at home for testing/development.

Malte, I'll try to look at the bugtracker in the next few days, still plan on concentrating on forums through the next couple of days.