get_query_set patching is very broken

[spookylukey]

The method included to patch get_query_set is very broken, and leads to some very bug code in circumstances where managers get subclassed - the behaviour of the subclass can be skipped. This is a very serious error for the case of Related managers, because it means that the limiting of the queryset that is added by the subclass get_query_set method gets skipped, so the related manager queries apply to all objects.

I've created a branch that demonstrates the problem, adding 4 tests, 2 of which fails with Django 1.8, one fails with Django 1.5:

https://github.com/spookylukey/django-compat/tree/get_query_set_patch_bug

I've created a PR so that you can see the tests failing.

The full details are explained in this blog post:

http://lukeplant.me.uk/blog/posts/handling-django's-get_query_set-rename-is-hard/

[spookylukey]

By the way, I don't think there is a good way to fix this. But it would be better to remove the support for get_query_set/get_queryset than advertise something that is flawed.

[NotSqrt]

I am using django 1.7 with perfectly valid managers, and the monkey patch adds the warning "Manager.get_query_set method should be renamed get_queryset". After some inspection, it appears that at some point during the django init, the Manager class gets to a point where Manager.__dict__.get("get_query_set") is <unbound method Manager.get_queryset> and Manager.__dict__.get("get_queryset") is None.

Disabling the monkey patch removes the warning, but doing the following also does !! :

try:
    Manager.get_query_set = Manager.get_queryset
    Manager.get_queryset = Manager.get_queryset  # yep !
except AttributeError:
    Manager.get_queryset = Manager.get_query_set

[spookylukey]

Can I point out that this issue is a major data loss bug?

If you get get_queryset wrong, then Django's dynamic RelatedManager stops working.

This is shown by the failing tests in my branch: https://github.com/arteria/django-compat/compare/master...spookylukey:get_query_set_patch_bug

In my example, house.rooms.downstairs() starts returning Room.objects.downstairs() instead of just the rooms that are related to the house. So if you do house.rooms.downstairs().delete(), you just deleted far more things than you intended to (Similarly an update() method affects far more records than intended).

Please fix this bug, by removing the get_query_set monkey patching. That method can't work, since it can't monkey patch the RelatedManager classes, which are dynamically created.

[philippeowagner]

Sure @spookylukey, we will remove the get_query_set support. It simply not has been done yet. We will rework the README (#17, #28) at the same time. Thanks again for your contribution.

[spookylukey]

Great, thanks! I just didn't want this to get lost, since it is a potential data loss situation.