Skip to content

feat: nulls_ordering (NULLS FIRST/LAST) for order_by (#769)#1639

Closed
collerek wants to merge 3 commits intomasterfrom
feat/nulls-ordering-769
Closed

feat: nulls_ordering (NULLS FIRST/LAST) for order_by (#769)#1639
collerek wants to merge 3 commits intomasterfrom
feat/nulls-ordering-769

Conversation

@collerek
Copy link
Copy Markdown
Collaborator

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 new ormar.NullsOrdering enum (FIRST / LAST).

  • NullsOrdering(str, Enum) in ormar/queryset/clause.py, re-exported from ormar.queryset and top-level ormar.
  • OrderAction gets an optional nulls_ordering; a new _build_order_expression() helper assembles the final SQL.
  • Emits standard NULLS FIRST / NULLS LAST on PostgreSQL and SQLite (3.30+). On MySQL, emulates via col IS [NOT] NULL, col <dir> — a pure sort-key prefix, no effect on the result set.
  • Fixes the nested-prefix bug the reviewer flagged on the original PR: the fully-qualified "table"."col" is threaded through both halves of the MySQL expression, so joined/aliased tables keep their prefix.
  • FieldAccessor.asc() / .desc() accept Optional[NullsOrdering] and raise ValueError for other types.
  • QuerySet.order_by() / QuerysetProxy.order_by() needed no signature change — they already accept OrderAction instances, so Model.field.asc(nulls_ordering=...) propagates through both.
  • Docs: new subsection in docs/queries/filter-and-sort.md with usage and the MySQL dialect note.

Test plan

  • pytest tests/test_queries/test_order_by.py — 5 tests pass on SQLite
  • make pre-commit — ruff format, ruff check, mypy strict all clean
  • make coverage — 503 passed, 1 skipped, 100% total coverage
  • CI make test_mysql — exercises the MySQL emulation branch (marked # pragma: no cover in the default SQLite run)
  • CI make test_pg — exercises the native NULLS FIRST/LAST path on PostgreSQL

New tests added:

  • test_sort_order_nulls_first_last — dedicated main-model test with unique sort_order values covering all four {asc, desc} × {FIRST, LAST} combinations plus two ValueError guards.
  • test_sort_order_on_related_model — extended with the same four combinations using Toy.owner.age, exercising the joined-table prefix path that the original PR's MySQL branch broke.
  • Every test in test_order_by.py is now wrapped in database.transaction(force_rollback=True) so writes roll back between tests.

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.
@collerek
Copy link
Copy Markdown
Collaborator Author

Superseded — force-pushed the same work onto #769 so the original PR's discussion thread stays in one place.

@collerek collerek closed this Apr 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant