Pick latest pgjsonb get_fun return by alter_time, not by MAX(jid)

changelog/69062.fixed.md

@@ -0,0 +1,4 @@
+Fixed `salt.returners.pgjsonb.get_fun` raising a SQL syntax error on
+PostgreSQL because of MySQL-style backtick quoting (`` MAX(`jid`) ``)
+left over from a copy-paste of the `mysql` returner. The query now
+uses unquoted identifiers, which is valid on PostgreSQL.

changelog/69064.fixed.md

@@ -0,0 +1,12 @@
+Fixed `salt.returners.pgjsonb.get_fun` returning the wrong row per
+minion when jids are not lexicographically sortable as timestamps.
+The previous SQL used `MAX(jid)` to pick the "latest" return, which
+was correct only for Salt's default jid format
+(`YYYYMMDDHHMMSSffffff` and the `nano` variant). Deployments that
+override `master_job_cache.gen_jid` (custom prep_jid emitting UUIDs,
+snowflake ids, or any non-sortable scheme) -- or that hold rows
+written under different jid formats from a past config change --
+got a silently wrong answer. The query now orders by
+`alter_time DESC` and picks one row per minion via `DISTINCT ON`,
+so "latest" is determined from the timestamp Postgres populates via
+`DEFAULT NOW()`.

salt/returners/pgjsonb.py

@@ -396,13 +396,22 @@ def get_fun(fun):
     """
     with _get_serv(ret=None, commit=True) as cur:
 
-        sql = """SELECT s.id,s.jid, s.full_ret
-                FROM salt_returns s
-                JOIN ( SELECT MAX(`jid`) as jid
-                    from salt_returns GROUP BY fun, id) max
-                ON s.jid = max.jid
-                WHERE s.fun = %s
-                """
+        # The previous query picked the latest return per minion with
+        # ``MAX(jid)``. That assumed jids are lexicographically sortable
+        # as timestamps (the default ``YYYYMMDDHHMMSSffffff`` format and
+        # the ``nano`` variant), which silently returns the wrong row
+        # for any deployment that overrides ``master_job_cache.gen_jid``
+        # or that has a mix of jid formats in ``salt_returns`` from a
+        # past config change. Use ``alter_time`` -- which Postgres
+        # populates from ``DEFAULT NOW()`` -- as the source of truth
+        # for "latest" instead, and pick one row per minion with
+        # ``DISTINCT ON``.
+        sql = """SELECT DISTINCT ON (id)
+                        id, jid, full_ret
+                 FROM salt_returns
+                 WHERE fun = %s
+                 ORDER BY id, alter_time DESC
+                 """
 
         cur.execute(sql, (fun,))
         data = cur.fetchall()

tests/pytests/unit/returners/test_pgjsonb.py

@@ -79,3 +79,66 @@ def test_save_load_with_bytes():
         with patch.object(psycopg2.extras, "Json") as json_mock:
             pgjsonb.save_load(load["jid"], load)
             json_mock.assert_called_with(decoded_load)
+
+
+def test_get_fun_returns_one_full_ret_per_minion_with_postgres_compatible_sql():
+    """``get_fun`` builds a per-minion last-execution dict.
+
+    The previous SQL used MySQL-style backtick quoting (``MAX(`jid`)``),
+    which raises a syntax error on PostgreSQL where the function lives.
+    Verify both the produced mapping and that the issued SQL is free of
+    backticks so the fix does not regress through future copy-paste from
+    the mysql returner.
+    """
+    rows = [
+        ("minion-1", "20260505000000000001", {"return": "ok-1", "fun": "test.ping"}),
+        ("minion-2", "20260505000000000002", {"return": "ok-2", "fun": "test.ping"}),
+    ]
+    cur = MagicMock()
+    cur.fetchall.return_value = rows
+    serv = MagicMock()
+    serv.return_value.__enter__.return_value = cur
+
+    with patch.object(pgjsonb, "_get_serv", serv):
+        result = pgjsonb.get_fun("test.ping")
+
+    assert result == {
+        "minion-1": {"return": "ok-1", "fun": "test.ping"},
+        "minion-2": {"return": "ok-2", "fun": "test.ping"},
+    }
+    issued_sql = cur.execute.call_args.args[0]
+    assert (
+        "`" not in issued_sql
+    ), "MySQL-style backtick quoting in pgjsonb SQL — invalid on PostgreSQL"
+
+
+def test_get_fun_orders_by_alter_time_desc_not_max_jid():
+    """``get_fun`` must determine "latest execution per minion" from
+    ``alter_time`` rather than from a lexicographic ordering of jids.
+
+    The previous SQL used ``MAX(jid)``, which works only when jids are
+    timestamp-formatted strings of equal length (Salt's default
+    ``YYYYMMDDHHMMSSffffff`` and the ``nano`` variant). Deployments that
+    override ``master_job_cache.gen_jid`` (custom prep_jid emitting UUIDs,
+    snowflake ids, or any non-sortable scheme), or that hold rows written
+    under different jid formats from a past config change, get a
+    silently wrong answer with ``MAX(jid)`` -- the lexicographic max is
+    not the time-latest.
+
+    Pin the algorithm: order by ``alter_time DESC`` (which Postgres
+    populates via ``DEFAULT NOW()``), and guard against regression to
+    the ``MAX(jid)`` form.
+    """
+    cur = MagicMock()
+    cur.fetchall.return_value = []
+    serv = MagicMock()
+    serv.return_value.__enter__.return_value = cur
+
+    with patch.object(pgjsonb, "_get_serv", serv):
+        pgjsonb.get_fun("test.ping")
+
+    sql = cur.execute.call_args.args[0].lower()
+    assert "alter_time" in sql
+    assert "order by" in sql
+    assert "desc" in sql
+    assert "max(jid)" not in sql