Support order_by with None value

[vuongdv-spinshell]

I try to sort with an annotate field that possible None value. But look like django-querysetsequence doesn't suport it

Reproduce step:

room_options is a JSON field that contains branding 
room = Room.objects.all().annotate(brand=F('room_options__branding'))
ticket = Ticket.objects.all().annotate(brand=F('room_options__branding'))
room_ticket =  QuerySetSequence(room, ticket, model=Room).order_by('brand')
room_ticket[0]

=> TypeError: '>' not supported between instances of 'NoneType' and 'NoneType'

[clokep]

Hmm, I wonder if this doesn't work for any nullable field? Definitely seems like an oversight, thanks for the bug report!

Do you have a fuller stack trace of the error? I suspect some special handling is needed near: https://github.com/clokep/django-querysetsequence/blob/81f75dd7094deaacb066d3da6154dce25cd90b60/queryset_sequence/__init__.py#L101

But I'm not 100% sure.

[vuongdv-spinshell]

Yes, You're right. This is the full stack trace for it.

Environment:

Request Method: GET
Request URL: http://localhost:8000/api/calls2/?sort=brand

Django Version: 3.2.11
Python Version: 3.8.13
Installed Applications:
['django.contrib.admindocs',
 'server.apps.ServerConfig',
 'django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'rest_framework',
 'corsheaders',
 'zappa_django_utils',
 'pushevent.apps.PushEventConfig',
 'rest_framework.authtoken',
 'guardian',
 'django_filters',
 'drf_spectacular',
 'imagekit',
 'reservations',
 'questionnaires',
 'django.contrib.admin',
 'oauth2_provider',
 'social_django',
 'rest_framework_social_oauth2',
 'authentication.apps.AuthenticationConfig',
 'monotonic_counter.apps.MonotonicCounterConfig',
 'parler',
 'anymail',
 'ordered_model',
 'dynamo_cache',
 'drf_multiple_model',
 'django.contrib.humanize',
 'puml_generator',
 'debug_toolbar',
 'channels',
 'channels_websocket.apps.ChannelsWebsocketConfig']
Installed Middleware:
['debug_toolbar.middleware.DebugToolbarMiddleware',
 'django.middleware.security.SecurityMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'corsheaders.middleware.CorsMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware',
 'server.permissions.StandardiseErrorMiddleware',
 'server.permissions.MissingTOSAgreementResponseMiddleware']

Traceback (most recent call last):
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/rest_framework/viewsets.py", line 125, in view
    return self.dispatch(request, *args, **kwargs)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/rest_framework/mixins.py", line 40, in list
    page = self.paginate_queryset(queryset)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/rest_framework/generics.py", line 171, in paginate_queryset
    return self.paginator.paginate_queryset(queryset, self.request, view=self)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/rest_framework/pagination.py", line 216, in paginate_queryset
    return list(self.page)
  File "/opt/homebrew/Cellar/python@3.8/3.8.13/Frameworks/Python.framework/Versions/3.8/lib/python3.8/_collections_abc.py", line 874, in __iter__
    v = self[i]
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/django/core/paginator.py", line 188, in __getitem__
    self.object_list = list(self.object_list)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/queryset_sequence/__init__.py", line 549, in __iter__
    self._fetch_all()
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/queryset_sequence/__init__.py", line 526, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/queryset_sequence/__init__.py", line 197, in _ordered_iterator
    iterables = sorted(iterables, key=comparator)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/queryset_sequence/__init__.py", line 181, in comparator
    return _comparator(tuple_1[2], tuple_2[2])
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/queryset_sequence/__init__.py", line 136, in comparator
    return cls._cmp(v1, v2) * reverses[0]
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/queryset_sequence/__init__.py", line 101, in _cmp
    return cmp(value1, value2)
  File "/Users/vuongdo/Library/Caches/pypoetry/virtualenvs/livecall-5JrKXylz-py3.8/lib/python3.8/site-packages/queryset_sequence/__init__.py", line 26, in cmp
    return (a > b) - (a < b)

Exception Type: TypeError at /api/calls2/
Exception Value: '>' not supported between instances of 'str' and 'NoneType'

[clokep]

I think that code needs special handling for null, unfortunately not all databases order null the same way, but pretty much we would want to return 1 is value1 is None and -1 if value2 is None (or perhaps the opposite depending how you want nulls sorted). (And maybe 0 if they're both null.)

[vuongdv-spinshell]

I'm currently using PostgreSQL, and look like your above-defined rule is the default of Postgres sorting with null. Please fixed it as you mention. Thank you. For more improvements, you probably want to add nulls_last and nulls_first when you implement this update https://github.com/clokep/django-querysetsequence/issues/92

[clokep]

I'm currently using PostgreSQL, and look like your above-defined rule is the default of Postgres sorting with null.

It should work on everything except SQLite, I think.

Please fixed it as you mention. Thank you.

I unfortunately won't have time to implement this anytime soon, but would appreciate a pull request for it! Let me know if you have any questions!

For more improvements, you probably want to add nulls_last and nulls_first when you implement this update #92

Good catch! 👍

[vuongdv-spinshell]

@clokep I created https://github.com/clokep/django-querysetsequence/pull/97 pull request to fix this issue, please kindly check it. Thank you.