On 22 March 2012 16:54, Aymeric Augustin <aymeric.augus...@polytechnique.org> wrote: > Le 22 mars 2012 13:22, Łukasz Rekucki <lreku...@gmail.com> a écrit : >> If the whole patch can't be merged, lets at least fix that bug[2]. Is >> there any work I can do to make it happen? >> [2]: >> https://github.com/lqc/django/commit/84dc450ec861e34de068fde891537f0481342ef7 > > Hi Łukasz, > > Let's try to get this fix in 1.4. Since the change in the tests isn't > significant, it boils down to replacing two `dict`s which > `SortedDict`s. Since a `SortedDict` is a subclass of `dict`, this > change looks safe. > > In a comment [1], you said: > >> there are a few places that relabel aliases and ruin the order, like >> QuerySet.change_aliases(). (...) I fixed this, by using change_map = >> SortedDict() in QuerySet.split_exclude. This should probably apply to all >> change_maps when working with aliases. > > This implies that your fix isn't complete, is it? Could you clarify? >
Yes, sorry. It's complete in the sense, that it fixes this bug and I haven't been able to spot any other bugs by reading the code or doing tests. There is at least one other place that creates a "change_map" variable (or something similar), that is used to relabel aliases in a Q object (AFAIR), but it never uses it in a way that could be affected by ordering. My other concern is other similar structures (like alias_map or included_inherited_models). While I failed to find any dangerous uses, I'm not (yet, I hope) very comfortable with the ORM code, so I wouldn't trust myself on that 100%. OTOH, I didn't want to blindly change all the dicts to SortedDict. -- Łukasz Rekucki -- You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com. To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com. For more options, visit this group at http://groups.google.com/group/django-developers?hl=en.
