[drupal-devel] [bug] New pager code broken taxonomy pagers

[Dries]

Issue status update for http://drupal.org/node/23687

 Project:      Drupal
 Version:      cvs
 Component:    base system
 Category:     bug reports
 Priority:     normal
 Assigned to:  Morbus Iff
 Reported by:  Morbus Iff
 Updated by:   Dries
 Status:       patch

Morbus: the funky comment thread stuff rocks.  We should do the same for
book pages and taxonomy terms if it can be done effectively.  This
doens't mean we should remove the traditional pid-columns.




Dries



Previous comments:
------------------------------------------------------------------------

May 26, 2005 - 21:44 : Morbus Iff

I've not yet looked into it, but I doubt I'll be able to fix it (the
original taxonomy/term pager code was a hack that moshe taught me
about). In a nutshell, though, the recent pager cleanup in head has
broken the term pagers for vocabularies.




------------------------------------------------------------------------

May 26, 2005 - 22:21 : Morbus Iff

Attachment: http://drupal.org/files/issues/_p_23687_taxopager.patch (1.88 KB)

Patch attached.




------------------------------------------------------------------------

May 27, 2005 - 09:05 : Steven

Why can't this code use pager_query() and query the database directly?
If free tagging vocabularies can get so big, it seems a very
inefficient thing to fetch the entire list.




------------------------------------------------------------------------

May 27, 2005 - 09:05 : Steven

Note: if you need to fetch a list of tags ordered by hierarchy in one
query, you should look at how comment.module does it.




------------------------------------------------------------------------

May 27, 2005 - 13:17 : Morbus Iff

Which part of comment.module? I hope you don't think the crazy "thread
1.1.2/, 1.2" stuff is /better/.




------------------------------------------------------------------------

May 27, 2005 - 15:19 : Steven

Maybe it's crazy, but it has a single, pagable query and an unambigous
sorting both ways. It works well for what it needs to do.


In fact, the only problem is the assignment of numbers because we
cannot do a "natural order" sort in SQL. Right now it uses a rather
inefficient coding that scales O(n) with size and adds another digit
every 10 values.


This prompted me to experiment with better codings. First I generalized
the current scheme a bit, then tweaked it to get near O(log n) scaling
and on average only 20-30% more digits than an equivalent decimal
notation, while still retaining the string sorting property. Quite fun
actually.


I don't see the problem with the approach though... all it does is
store a compact ordering key per row. Compare this to the convoluted
tree logic in taxonomy_get_tree() and I know which one I prefer.


Of course, I'm not at all sure if the comment.module approach can be
implemented here, but I can't believe you'd do something like this
after talking about mega-taxonomies for so long. Loading in 8000 tags
just to show 50 is nowhere near optimal.




------------------------------------------------------------------------

May 27, 2005 - 15:28 : Morbus Iff

Fair enough. I don't have the time right now to code this though - my
deadline for the full NHPR conversion is fast approaching, and the
patch attached merely brings us back to the "working" behavior prior to
the pager changes, not "better" behavior, which you're proposing.
However, I'm a little .. .. .. cautious about the idea of using a
comment.thread-like equivalent (something I immediately knee-jerked
disliked the first time I saw it) for a regular parent/child
relationship just to get a hierarchal sort based on a LIMIT. This can't
be how other database apps are doing it, is it?