Pin NULL-is-smallest ORDER BY on PostgreSQL

gramps_object_query_language/proxied_query.py

@@ -67,7 +67,9 @@
 `NULL` placement, for both plain sorting and keyset seeking, matches
 SQLite's own verified default (confirmed empirically, not assumed): `NULL`
 sorts as the smallest value in both directions, so `DESC` is the exact
-reverse of `ASC`, `NULL`s included. Keyset seeking (`after`) mirrors
+reverse of `ASC`, `NULL`s included. PostgreSQL's default is the opposite,
+so the SQL path pins this order there with explicit `NULLS FIRST`/`NULLS
+LAST` (`query.py`'s `_order_direction_sql`). Keyset seeking (`after`) mirrors
 `query.py`'s `_compile_keyset` SQL, which (after its own NULL-safety fix --
 see that function's docstring) applies this identical NULL-is-smallest rule
 to the seek predicate too, rather than relying on SQL's raw three-valued

gramps_object_query_language/query.py

@@ -2650,8 +2650,9 @@ def _keyset_leg_sql(
 ) -> Optional[Tuple[str, list]]:
     """NULL-safe "ranks strictly after the cursor" fragment for the one
     `order_by` column a seek's OR-term actually advances past, matching
-    `ORDER BY`'s own verified default total order (`NULL` is the smallest
-    value in both directions -- see `_null_safe_cmp` in `proxied_query.py`,
+    `ORDER BY`'s total order (`NULL` is the smallest value in both
+    directions -- SQLite's default, pinned explicitly on PostgreSQL by
+    `_order_direction_sql`; see also `_null_safe_cmp` in `proxied_query.py`,
     which mirrors this same order for the evaluator path).
 
     A plain `col > ?`/`col < ?` against a real SQL `NULL` is always
@@ -2811,6 +2812,25 @@ def _where_clauses(
     return clauses, params
 
 
+def _order_direction_sql(ob: OrderBy, dialect: Optional[Dialect]) -> str:
+    """`ASC`/`DESC` for one `ORDER BY` column, pinned to the "`NULL` is
+    smallest" total order that `_keyset_leg_sql` and `proxied_query.py`'s
+    `_null_safe_cmp` both assume.
+
+    That is SQLite's default, so SQLite gets the bare keyword. PostgreSQL's
+    default is the opposite (`NULL` is largest: last in `ASC`, first in
+    `DESC`), which both reordered results relative to SQLite and made the
+    keyset seek skip rows -- so it gets an explicit `NULLS FIRST`/`NULLS
+    LAST`. `handle` is left bare: it's the `NOT NULL` primary key, so the
+    clause changes nothing there, and a bare `ASC`/`DESC` keeps the
+    default-ordered primary-key index usable for the sort.
+    """
+    keyword = ob.direction.upper()
+    if dialect != Dialect.POSTGRESQL or ob.column == "handle":
+        return keyword
+    return f"{keyword} NULLS FIRST" if ob.direction == "asc" else f"{keyword} NULLS LAST"
+
+
 def compile_query(
     spec: ObjectTypeSpec,
     query: Query,
@@ -2886,7 +2906,7 @@ def compile_query(
     order_params: list = []
     for ob in ordering:
         order_sql, p = _column_expr(ob.column, spec, collation, dialect, treeid)
-        order_parts.append(f"{order_sql} {ob.direction.upper()}")
+        order_parts.append(f"{order_sql} {_order_direction_sql(ob, dialect)}")
         order_params.extend(p)
 
     sql = f"SELECT {', '.join(select_parts)} FROM {spec.table}"

gramps_object_query_language/tests/test_query.py

@@ -1576,7 +1576,7 @@ def test_order_by_gets_trailing_handle_tiebreaker():
     # which isn't what this test is about.
     query = Query(select=["handle"], order_by=[OrderBy("surname", "asc")])
     sql, _ = compile_query(PERSON, query, dialect=Dialect.POSTGRESQL)
-    assert "ORDER BY surname ASC, handle ASC" in sql
+    assert "ORDER BY surname ASC NULLS FIRST, handle ASC" in sql
 
 
 def test_order_by_does_not_duplicate_explicit_handle():
@@ -1586,10 +1586,38 @@ def test_order_by_does_not_duplicate_explicit_handle():
     )
     sql, _ = compile_query(PERSON, query, dialect=Dialect.POSTGRESQL)
     order_by_clause = sql.split("ORDER BY", 1)[1]
-    assert order_by_clause.startswith(" surname ASC, handle DESC")
+    assert order_by_clause.startswith(" surname ASC NULLS FIRST, handle DESC")
     assert order_by_clause.count("handle") == 1  # not duplicated
 
 
+def test_postgresql_order_by_pins_null_is_smallest():
+    # Issue #4: PostgreSQL's default puts NULL last in ASC and first in DESC,
+    # the opposite of SQLite's and of what _keyset_leg_sql/_null_safe_cmp
+    # assume -- so results reordered and keyset paging dropped rows. The
+    # PostgreSQL dialect must pin NULL-is-smallest explicitly, both
+    # directions, for plain columns and JSON paths alike.
+    query = Query(
+        select=["handle"],
+        order_by=[OrderBy("birth.place.title", "desc"), OrderBy("surname", "asc")],
+    )
+    sql, _ = compile_query(PERSON, query, dialect=Dialect.POSTGRESQL)
+    order_by_clause = sql.split("ORDER BY", 1)[1]
+    assert order_by_clause.count("DESC NULLS LAST") == 1
+    assert "surname ASC NULLS FIRST" in order_by_clause
+    # The NOT NULL primary-key tiebreaker stays bare (index-friendly).
+    assert "handle ASC NULLS" not in order_by_clause
+
+
+def test_sqlite_order_by_has_no_nulls_clause():
+    # SQLite's default already is NULL-is-smallest; output stays unchanged.
+    query = Query(
+        select=["handle"],
+        order_by=[OrderBy("birth.place.title", "desc"), OrderBy("surname", "asc")],
+    )
+    sql, _ = compile_query(PERSON, query, dialect=Dialect.SQLITE)
+    assert "NULLS" not in sql
+
+
 def test_default_order_by_is_handle_only():
     query = Query(select=["handle"])
     sql, _ = compile_query(PERSON, query, dialect=Dialect.POSTGRESQL)