fix(query): sort FieldIndex via jsonb -> (#158)

CHANGES.md

@@ -1,5 +1,18 @@
 # Changelog
 
+## 1.0.0b64
+
+### Fixed
+
+- ``_process_sort`` now emits ``idx->'{key}'`` (JSONB operator) instead
+  of ``idx->>'{key}'`` (text operator) for the ``FieldIndex`` fallback.
+  Text-cast sorting compared everything lexicographically, so a
+  ``FieldIndex`` over a numeric attribute ranked ``"10"`` before
+  ``"2"``.  JSONB comparison is type-aware: numbers sort numerically,
+  strings lexically, so a homogeneous ``FieldIndex`` now always sorts
+  correctly regardless of value type.  Affects any ``FieldIndex`` with
+  numeric source data (counters, priorities, prices, weights).  #158
+
 ## 1.0.0b63
 
 ### Changed

src/plone/pgcatalog/query.py

@@ -774,7 +774,10 @@ def _process_sort(self, sort_on_list, sort_order_list):
             elif idx_type == IndexType.BOOLEAN:
                 expr = f"(idx->>'{idx_key}')::boolean"
             else:
-                expr = f"idx->>'{idx_key}'"
+                # jsonb operator `->` instead of text `->>` so PG uses type-aware
+                # comparison: numbers sort numerically, strings lexicographically
+                # (#158 — otherwise numeric FieldIndexes sort "10" < "2").
+                expr = f"idx->'{idx_key}'"
 
             parts.append(f"{expr} {direction}")
 

tests/test_query.py

@@ -428,7 +428,14 @@ def test_sql_injection_in_path_rejected(self):
 class TestSort:
     def test_sort_ascending(self):
         qr = build_query({"portal_type": "Document", "sort_on": "sortable_title"})
-        assert qr["order_by"] == "idx->>'sortable_title' ASC"
+        assert qr["order_by"] == "idx->'sortable_title' ASC"
+
+    def test_sort_field_uses_jsonb_operator(self):
+        """FIELD sort must use `->` (jsonb) not `->>` (text) so numeric
+        FieldIndexes sort numerically instead of lexicographically (#158)."""
+        qr = build_query({"sort_on": "sortable_title"})
+        assert "idx->'sortable_title'" in qr["order_by"]
+        assert "idx->>'sortable_title'" not in qr["order_by"]
 
     def test_sort_descending(self):
         qr = build_query(
@@ -481,7 +488,7 @@ def test_sort_on_list_two_fields(self):
                 "sort_on": ["portal_type", "sortable_title"],
             }
         )
-        assert qr["order_by"] == "idx->>'portal_type' ASC, idx->>'sortable_title' ASC"
+        assert qr["order_by"] == "idx->'portal_type' ASC, idx->'sortable_title' ASC"
 
     def test_sort_on_list_with_mixed_orders(self):
         qr = build_query(
@@ -494,7 +501,7 @@ def test_sort_on_list_with_mixed_orders(self):
         assert qr["order_by"] is not None
         parts = qr["order_by"].split(", ")
         assert len(parts) == 2
-        assert parts[0] == "idx->>'sortable_title' ASC"
+        assert parts[0] == "idx->'sortable_title' ASC"
         assert "pgcatalog_to_timestamptz" in parts[1]
         assert "DESC" in parts[1]
 
@@ -506,7 +513,7 @@ def test_sort_on_list_single_order_applies_to_all(self):
                 "sort_order": "descending",
             }
         )
-        assert qr["order_by"] == "idx->>'portal_type' DESC, idx->>'sortable_title' DESC"
+        assert qr["order_by"] == "idx->'portal_type' DESC, idx->'sortable_title' DESC"
 
     def test_sort_on_list_with_unknown_index_skipped(self):
         qr = build_query(
@@ -515,7 +522,7 @@ def test_sort_on_list_with_unknown_index_skipped(self):
                 "sort_on": ["nonexistent", "sortable_title"],
             }
         )
-        assert qr["order_by"] == "idx->>'sortable_title' ASC"
+        assert qr["order_by"] == "idx->'sortable_title' ASC"
 
     def test_sort_on_list_all_unknown(self):
         qr = build_query(
@@ -529,7 +536,7 @@ def test_sort_on_list_all_unknown(self):
     def test_sort_on_single_string_still_works(self):
         """Backward compat: single string sort_on unchanged."""
         qr = build_query({"portal_type": "Document", "sort_on": "sortable_title"})
-        assert qr["order_by"] == "idx->>'sortable_title' ASC"
+        assert qr["order_by"] == "idx->'sortable_title' ASC"
 
 
 # ---------------------------------------------------------------------------
@@ -742,13 +749,13 @@ def test_tgpath_multiple_paths_subtree(self):
 
     def test_tgpath_sort(self):
         qr = build_query({"tgpath": "/uuid1", "sort_on": "tgpath"})
-        assert qr["order_by"] == "idx->>'tgpath' ASC"
+        assert qr["order_by"] == "idx->'tgpath' ASC"
 
     def test_tgpath_sort_descending(self):
         qr = build_query(
             {"tgpath": "/uuid1", "sort_on": "tgpath", "sort_order": "descending"}
         )
-        assert qr["order_by"] == "idx->>'tgpath' DESC"
+        assert qr["order_by"] == "idx->'tgpath' DESC"
 
     def test_builtin_path_uses_typed_columns(self):
         """Built-in 'path' index dispatches to typed columns, not idx JSONB (#132)."""
@@ -1054,7 +1061,7 @@ def test_sort_dynamic_field(self, populated_registry):
         get_registry().register("my_sort_field", IndexType.FIELD, "my_sort_field")
 
         qr = build_query({"sort_on": "my_sort_field"})
-        assert qr["order_by"] == "idx->>'my_sort_field' ASC"
+        assert qr["order_by"] == "idx->'my_sort_field' ASC"
 
     def test_sort_dynamic_date(self, populated_registry):
         from plone.pgcatalog.columns import get_registry

tests/test_query_integration.py

@@ -455,3 +455,33 @@ def test_pagination(self, pg_conn_with_catalog):
         zoids1 = {r["zoid"] for r in rows1}
         zoids2 = {r["zoid"] for r in rows2}
         assert zoids1.isdisjoint(zoids2)
+
+    def test_sort_numeric_field_numerically(self, pg_conn_with_catalog):
+        """Numeric FieldIndex values must sort numerically, not lexically (#158).
+
+        Under text-cast sorting, 10 < 2 because "10" < "2" lexicographically.
+        The jsonb `->` operator preserves type and sorts numbers as numbers.
+        """
+        from plone.pgcatalog.columns import get_registry
+        from plone.pgcatalog.columns import IndexType
+
+        conn = pg_conn_with_catalog
+        get_registry().register("priority", IndexType.FIELD, "priority")
+
+        priorities = {200: 10, 201: 2, 202: 20, 203: 3, 204: 1}
+        for zoid, priority in priorities.items():
+            insert_object(conn, zoid=zoid)
+            catalog_object(
+                conn,
+                zoid=zoid,
+                path=f"/plone/item{zoid}",
+                idx={"portal_type": "Item", "priority": priority},
+            )
+        conn.commit()
+
+        rows = execute_query(
+            conn,
+            {"portal_type": "Item", "sort_on": "priority"},
+            columns="zoid",
+        )
+        assert [r["zoid"] for r in rows] == [204, 201, 203, 200, 202]  # 1,2,3,10,20