nulls_first or nulls_last ordering produces incorrect SQL Server syntax

[davidjb]

Using the latest dev (f2d27ee), attempts to use nulls_first or nulls_last ordering produce invalid SQL.

#19 reports a related issue, but the change in #20 that modifies the database backend's features causes Django's other conditional pathway at https://github.com/django/django/blob/main/django/db/models/expressions.py#L1214-L1227 to be followed, resulting in invalid SQL being included in the SQL template, leading to the following syntax error:

Incorrect syntax near the keyword 'IS'

caused by of either of the following SQL statements that get created by sqlserver_orderby are invalid syntax:

• .desc(nulls_first=True)

  '%(expression)s IS NOT NULL, CASE WHEN %(expression)s IS NULL THEN 0 ELSE 1 END, %(expression)s %(ordering)s'

• .asc(nulls_last=True)

  '%(expression)s IS NULL, CASE WHEN %(expression)s IS NULL THEN 1 ELSE 0 END, %(expression)s %(ordering)s'

Tested with Django 3.1.8 but appears to still affect 3.2+. I believe https://code.djangoproject.com/ticket/32584 needs to be reopened because the template is still being overwritten, unless there's a better strategy for overriding Django's core behaviour. @timnyborg, do you have any thoughts?

Example Django model illustrating the issue:

from django.db import models

class Event(models.Model):
    # ... fields go here ...

    class Meta(PersonMetadata.Meta):
        ordering = (
            F("end_year").desc(nulls_first=True),
            F("start_year").asc(nulls_last=True),
        )

Event.objects.first()  # Incorrect syntax near the keyword 'IS'

[davidjb]

Reopened https://code.djangoproject.com/ticket/32584 and expanded the description. Thanks @timnyborg for starting that.

[timnyborg]

Thanks for reopening - I hadn't encountered that particular case.

[davidjb]

@timnyborg I just opened #32 to resolve this issue

[absci]

Thanks for all your work. I'll review and merge it if everything's good.

[davidjb]

@absci Just wondering if you've had a chance to review this PR. Without it, I'm unable to query Django with this ordering enabled on models.

[absci]

@absci Just wondering if you've had a chance to review this PR. Without it, I'm unable to query Django with this ordering enabled on models.

Looks very good, I have merged it. I got busy with other things recently.

[davidjb]

No worries, thanks very much @absci!