feincms/django-tree-queries

tree_ordering by char fields - not working/supported?

glennmatthews opened this issue · 3 comments

It looks like the tree_ordering implementation (as well as order_siblings_by() assumes the field being ordered by is an integer value, since it left-pads it with zeros to solve the "110" < "20" problem (replacing it with "00000000000000000110" > "00000000000000000020"). While I see there's some code in tree_queries/compiler.py that appears to be intended to handle text differently from integers for PostgreSQL, in MySQL at least there's no such logic and so a tree ordered by a CharField is inappropriately sorted, e.g.:

>>> Location.objects.all()[4].tree_ordering
['0000000000Milky Way', '000000000Sol System', '00000000000000Earth', '0000000000000Africa']
>>> Location.objects.all()[5].tree_ordering
['0000000000Milky Way', '000000000Sol System', '00000000000000Earth', '000000000000Eurasia']
>>> Location.objects.all()[6].tree_ordering
['0000000000Milky Way', '000000000Sol System', '00000000000000Earth', '0000000000Australia']
>>> Location.objects.all()[7].tree_ordering
['0000000000Milky Way', '000000000Sol System', '00000000000000Earth', '000000North America']

(note that "Eurasia" is sorted between "Africa" and "Australia", apparently because its string length falls between them.)

Additionally the tree_ordering is calculated incorrectly if the field being ordered by exceeds 20 characters in length:

>>> Location.objects.get(name="Various Islands in the Pacific Ocean").tree_ordering
['0000000000Milky Way', '000000000Sol System', '00000000000000Earth']

(the tree_ordering is simply omitting the object's name altogether from the array in this case.)

Is this a known limitation of this library?

Hi

Thanks for the report! I'm not surprised that it doesn't work but I'm surprised that this hasn't been detected before.

I changed the test_string_ordering test to use strings of varying lengths and it is even worse than I thought. Ordering by string fields only works when using fixed-length fields on all databases, even on PostgreSQL. Oh my...

I hope 0.10 fixes this issue.

Wow, that was fast! 😃 Thank you, I'll take it for another spin.