Changes between Initial Version and Version 3 of Ticket #32584
- Timestamp:
- Apr 19, 2021, 2:26:00 AM (4 years ago)
Legend:
- Unmodified
- Added
- Removed
- Modified
-
Ticket #32584
- Property Cc added
- Property Has patch unset
- Property Summary OrderBy.as_sql() overwrites template → OrderBy.as_sql() overwrites template, creating invalid syntax for certain database backends
- Property Version 3.1 → 3.2
-
Ticket #32584 – Description
initial v3 1 A change in 3.1 has caused OrderBy.as_sql() in django.db.models.expressions to ignore sql templates provided by db backends when nulls_first or nulls_lastis set:1 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: 2 2 3 3 https://github.com/django/django/blob/ed0cc52dc3b0dfebba8a38c12b6157a007309900/django/db/models/expressions.py#L1212 4 4 5 {{{ 5 def as_sql(self, compiler, connection, template=None, **extra_context):6 def as_sql(self, compiler, connection, template=None, **extra_context): 6 7 template = template or self.template 7 8 if connection.features.supports_order_by_nulls_modifier: … … 19 20 ): 20 21 template = '%%(expression)s IS NOT NULL, %s' % template 22 connection.ops.check_expression_support(self) 23 expression_sql, params = compiler.compile(self.expression) 24 placeholders = { 25 'expression': expression_sql, 26 'ordering': 'DESC' if self.descending else 'ASC', 27 **extra_context, 28 } 29 template = template or self.template 30 params *= template.count('%(expression)s') 31 return (template % placeholders).rstrip(), params 21 32 }}} 22 33 23 Note that template is always overwritten if nulls_first == True. In 3.0, the function first tested for template == None.34 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). 24 35 25 This causes trouble for the MSSQL 3rd party driver, which provides its own order by functionality: 26 https://github.com/microsoft/mssql-django/issues/19 36 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. 27 37 28 The following change (simply testing for template) fixed the issue for me (forked off 3.1 as I'm on py 3.7) 29 https://github.com/timnyborg/django/commit/fd41b39ade8d37138951223eba7f2e3fb66d0d1c 38 Ref: https://github.com/microsoft/mssql-django/issues/19 & https://github.com/microsoft/mssql-django/issues/31