id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
32584	OrderBy.as_sql() overwrites template, creating invalid syntax for certain database backends	Tim Nyborg	nobody	"A change in Django 3.1 has caused `OrderBy.as_sql()` in `django.db.models.expressions` to incorrectly extend SQL templates provided by db backends when `nulls_first` or `nulls_last` is set:

https://github.com/django/django/blob/ed0cc52dc3b0dfebba8a38c12b6157a007309900/django/db/models/expressions.py#L1212

{{{
    def as_sql(self, compiler, connection, template=None, **extra_context):
        template = template or self.template
        if connection.features.supports_order_by_nulls_modifier:
            if self.nulls_last:
                template = '%s NULLS LAST' % template
            elif self.nulls_first:
                template = '%s NULLS FIRST' % template
        else:
            if self.nulls_last and not (
                self.descending and connection.features.order_by_nulls_first
            ):
                template = '%%(expression)s IS NULL, %s' % template
            elif self.nulls_first and not (
                not self.descending and connection.features.order_by_nulls_first
            ):
                template = '%%(expression)s IS NOT NULL, %s' % template
        connection.ops.check_expression_support(self)
        expression_sql, params = compiler.compile(self.expression)
        placeholders = {
            'expression': expression_sql,
            'ordering': 'DESC' if self.descending else 'ASC',
            **extra_context,
        }
        template = template or self.template
        params *= template.count('%(expression)s')
        return (template % placeholders).rstrip(), params
}}}

With changes from 3.1, feature flags were added along with alternative syntax (https://github.com/django/django/commit/7286eaf681d497167cd7dc8b70ceebfcf5cd21ad) but the two types of syntax included only work for the given db backends.  On a backend that doesn't support either syntax (such as the MSSQL 3rd party driver), there is currently no way of supplying its own template with correct syntax because that template will always be modified when  `nulls_first` or `nulls_last` is set (e.g. when descending=True/nulls_first=True or descending=False/nulls_last=True).

Can an additional database feature flag be added to Django to cover this use case? For example `supports_order_by_nulls` and when that is `True`, the existing code can run to modify the given template; if `False`, then the template should not be modified.

Ref: https://github.com/microsoft/mssql-django/issues/19 & https://github.com/microsoft/mssql-django/issues/31
"	Uncategorized	closed	Database layer (models, ORM)	3.2	Normal	wontfix	order_by, nulls_first	David Beitey	Unreviewed	0	0	0	0	0	0