From 4cb0b8c705e88780523742ddf3ac141c91facf57 Mon Sep 17 00:00:00 2001 From: "Jens W. Klein" Date: Tue, 21 Apr 2026 10:55:05 +0200 Subject: [PATCH] fix(query): sort FieldIndex via jsonb `->` so numbers sort numerically (#158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Text operator `idx->>'{key}'` casts to TEXT before ORDER BY, so a FieldIndex over a numeric attribute ranked "10" before "2". The jsonb operator `idx->'{key}'` preserves type — numbers sort numerically, strings lexically — so homogeneous FieldIndexes sort correctly regardless of value type. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGES.md | 13 +++++++++++++ src/plone/pgcatalog/query.py | 5 ++++- tests/test_query.py | 25 ++++++++++++++++--------- tests/test_query_integration.py | 30 ++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 10 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 4995443..8950b98 100644 --- a/CHANGES.md +++ b/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 diff --git a/src/plone/pgcatalog/query.py b/src/plone/pgcatalog/query.py index 81ef473..7b5b7af 100644 --- a/src/plone/pgcatalog/query.py +++ b/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}") diff --git a/tests/test_query.py b/tests/test_query.py index f5a0a77..7910cfb 100644 --- a/tests/test_query.py +++ b/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 diff --git a/tests/test_query_integration.py b/tests/test_query_integration.py index a56bad2..305cf60 100644 --- a/tests/test_query_integration.py +++ b/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