Added support for NATURAL_SORT_KEY with MariaDB #1018
Conversation
There was a problem hiding this comment.
I'm not interested in adding database functions that take so little effort to implement in your own project. The functions we have in the package have more restricted types, which helps users call them correctly. Could you do the same here?
Also every pull request needs documentation and a changelog note.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
| connection: BaseDatabaseWrapper, | ||
| **extra_context, | ||
| ) -> tuple[str, tuple[Any, ...]]: | ||
| if connection.vendor == "mysql" and not connection.mysql_is_mariadb: |
There was a problem hiding this comment.
These database version checks are not worth it. In the successful path, all of this checking is time wasted (creating a Version is not free!). In the incredibly rare failure path, they’re not really providing more information than the database’s own error (“no such function”) would provide.
This is why none of the other functions have such checks.
There was a problem hiding this comment.
Thank you for your feedback. I understand your concerns about the database version checks in the code. But my reasoning was that While these checks can introduce overhead and may not always provide significant additional information, they serve as a preventive measure to catch potential issues early on.
I will look through the open issue pool on the Django projects to find a first-time-contributor friendly to contribute to.
Thanks Adam 👍
There was a problem hiding this comment.
Yeah, I'm all for preventative checks, but their cost has to be balanced against the probability of making a given mistake.
|
I’m sorry I still don’t think we are adding much of utility above the original two line function definition. Simply restricting to a single expression isn’t reason enough to add a custom class and maintain it forever. Thanks. |
Fixes #975
This PR adds support for MariaDB's NATURAL_SORT_KEY. It has been implemented as a database function.
Alphabet.objects.order_by(NaturalSortKey("d"))