Add proper field validation to QuerySet.order_by.

[Maxim]

When you annotate a QuerySet with a uuid key, the order_by functionality breaks for the uuid column because the uuid is "not a valid order_by argument".

Changing the constant django.db.models.sql.constants.ORDER_PATTERN by allowing a "-"

from

ORDER_PATTERN = re.compile(r'\?|[-+]?[.\w]+$')

to

ORDER_PATTERN = re.compile(r'\?|[-+]?[.\-\w]+$')

fixes this in PostgreSQL.


Is there a reason the former pattern was used, is it incompatible with other dbs?

[Simon Charette]

ORDER_PATTERN is a long standing hack that was added more than 12 years ago during the first iteration of the ORM and augmented later on to address #7098.

It should be replaced by proper field reference validation but could you go into more details about how you triggered this error? How did you manage to annotate a field containing an hyphen and pass it along to order_by.

Just to make it clear, order_by(str(uuid.uuid4()) is effectively not allowed and annotation using uuid as name should also not be supported

.annotate(**{str(uuid.uuid4()): expression})

is not supported and if you want to order by a programmatically added alias without risking field collision you should pass expression directly to .order_by().

[Simon Charette]

Closing as needsinfo for now until its determined whether or not it should be considered a cleanup of the regex based validation of order_by of if there's a legitimate issue with uuid annotation ordering.

[Maxim]

The way I was able to annotate with an uuid as a key was by dict expansion like your example above. So while it might not be supported it did not throw any errors and from what I can tell works perfectly fine.

I'm currently running Django==2.2

[Simon Charette]

I'm not aware of any tested support for invalid Python identifiers as annotation name but I guess we could accept this ticket on the basis that order_by does weird things and should perform proper field validation instead of a naive regexp checks.

To summarize while tweaking the regex would address your immediate issue I think it's worth calling Query.names_to_path on str input to validate they are valid I'll submit a PR for that.

[Simon Charette]

​https://github.com/django/django/pull/12669

[Maxim]

Sounds great, thank you

[Mariusz Felisiak <felisiak.mariusz@…>]

In 51394873:

Fixed #31426 -- Added proper field validation to QuerySet.order_by().

Resolve the field reference instead of using fragile regex based string
reference validation.