feat: nulls_ordering (NULLS FIRST/LAST) for order_by (#769)#1639
Closed
feat: nulls_ordering (NULLS FIRST/LAST) for order_by (#769)#1639
Conversation
…ultset with nulls ordering
The non-MySQL return path in _build_order_expression is never hit on MySQL CI, which enforces 100% coverage per dialect. Exclude it from coverage analysis, matching the already-excluded MySQL branch.
Collaborator
Author
|
Superseded — force-pushed the same work onto #769 so the original PR's discussion thread stays in one place. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports #769 to the current codebase and incorporates the review feedback that was never addressed upstream. Adds
Model.field.asc(nulls_ordering=...)/.desc(nulls_ordering=...)backed by a newormar.NullsOrderingenum (FIRST/LAST).NullsOrdering(str, Enum)inormar/queryset/clause.py, re-exported fromormar.querysetand top-levelormar.OrderActiongets an optionalnulls_ordering; a new_build_order_expression()helper assembles the final SQL.NULLS FIRST/NULLS LASTon PostgreSQL and SQLite (3.30+). On MySQL, emulates viacol IS [NOT] NULL, col <dir>— a pure sort-key prefix, no effect on the result set."table"."col"is threaded through both halves of the MySQL expression, so joined/aliased tables keep their prefix.FieldAccessor.asc()/.desc()acceptOptional[NullsOrdering]and raiseValueErrorfor other types.QuerySet.order_by()/QuerysetProxy.order_by()needed no signature change — they already acceptOrderActioninstances, soModel.field.asc(nulls_ordering=...)propagates through both.docs/queries/filter-and-sort.mdwith usage and the MySQL dialect note.Test plan
pytest tests/test_queries/test_order_by.py— 5 tests pass on SQLitemake pre-commit— ruff format, ruff check, mypy strict all cleanmake coverage— 503 passed, 1 skipped, 100% total coveragemake test_mysql— exercises the MySQL emulation branch (marked# pragma: no coverin the default SQLite run)make test_pg— exercises the nativeNULLS FIRST/LASTpath on PostgreSQLNew tests added:
test_sort_order_nulls_first_last— dedicated main-model test with uniquesort_ordervalues covering all four{asc, desc} × {FIRST, LAST}combinations plus twoValueErrorguards.test_sort_order_on_related_model— extended with the same four combinations usingToy.owner.age, exercising the joined-table prefix path that the original PR's MySQL branch broke.test_order_by.pyis now wrapped indatabase.transaction(force_rollback=True)so writes roll back between tests.