From 37e4d806791618bff07f989629c92d6e3c04f656 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 12 Oct 2026 18:26:28 +0000 Subject: [PATCH] test: count group-list rows, not planning buffers, in the #1203 arm The arm asking whether a query with no ORDER BY reads the row-group list to decide counted every shared buffer planning touched. That reading failed in CI on both arms, in both directions, and on one commit red then green; it was never reproduced on a quiet box. It now counts the rows planning reads from pgcolumnar.row_group and its pkey, from pg_stat_get_xact_tuples_returned. That is this transaction's own count, so no other backend or autovacuum can add to it, and a plain EXPLAIN plans without executing. The arm becomes an equality and the control a margin of at least one row per group. Measured on PG17, 1000 groups: no ORDER BY on=1000 off=1000 12 readings, with and without three ORDER BY on=2000 off=1000 sessions running DDL, all identical guard removed on=2000 off=1000 shell: 112 passed + 1 failed, the arm pytest: the arm fails, 'differs by 1000' green shell 113 passed + 0 failed; pytest module 48 passed compare_to_bash: 110 literal matches, 0 missing. What the buffer noise was is still unknown: concurrent DDL moved neither the buffer count nor the row count on a quiet box. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 10 +++ test/pytest/test_sorted_pathkeys.py | 132 +++++++++++----------------- test/sorted_pathkeys.sh | 70 ++++++++------- 3 files changed, 99 insertions(+), 113 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e19d4a6..6c637b96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,16 @@ true until the next version shipped. The specification's section 13, `docs/limitations.md` and `docs/installation.md` now describe the range. Going back to an older binary after a newer one has written to a table is refused rather than misread. +- The `sorted_pathkeys` arm that asks whether a query with no `ORDER BY` reads + the row-group list now counts the rows planning reads from + `pgcolumnar.row_group`, not the shared buffers planning touches (#1203). The + buffer count failed in CI on both arms, in both directions, and on one commit + red then green, and was never reproduced locally. The row count is this + transaction's own, so no other session can move it, and the arm is now an + equality. On PG17 it read 1000 for both settings with no `ORDER BY` and 2000 + against 1000 with one, identically across twelve readings with and without + concurrent DDL. With the guard it tests removed, it read 2000 against 1000 and + failed in both twins. The cause of the buffer noise is still unknown. - `test/pytest/README.md` now names all four registries a new pytest test has to be entered in, and which of them a local run can catch. Adding one test reddens diff --git a/test/pytest/test_sorted_pathkeys.py b/test/pytest/test_sorted_pathkeys.py index 9b0e4898..42e06401 100644 --- a/test/pytest/test_sorted_pathkeys.py +++ b/test/pytest/test_sorted_pathkeys.py @@ -972,59 +972,45 @@ def planbuf_fx(fx): return fx -def _planning_buffers(conn, guc, sql): - """-> shared hit+read during PLANNING, from the SECOND EXPLAIN of a FRESH BACKEND. - - The second, because the first warms the catalog cache and its planning buffers are - a measurement of that rather than of this query. - - A FRESH CONNECTION PER READING, and that is the part this arm was missing. `fx` is - module-scoped, so by the time this test runs its backend has absorbed the DDL of - twenty-nine other tests. How much catalog-cache state survives into each reading is - then not a property of the query being measured, and two readings taken from one - long-lived backend can differ by an amount with no upper bound and no fixed sign -- - #1203 records five CI failures on two different arms in opposite directions, - including the same commit red then green. - - A backend that has just started carries a known amount instead: the first EXPLAIN - fills its caches and the second measures the query. What is left is the couple of - buffers two fresh backends differ by, which is what the tolerance below is sized - for and why it is a tolerance rather than an equality. - - NOT REPRODUCED LOCALLY, and this comment should not pretend otherwise. Fourteen - runs on a quiet box -- five of the full cluster corpus, six of this module, three - with an injected catalog invalidation between the two readings -- all gave a - difference of exactly 0. So this is a change of regime argued from the mechanism, - not a red turned green. What it does establish is an upper bound on what a reading - can carry, which the previous arrangement had none of. - - The cost is one connection per reading, eight per run of this test. +def _group_list_rows(conn, guc, sql): + """-> rows this backend read from `pgcolumnar.row_group` while PLANNING `sql`. + + A COUNT OF THE WORK THE ARM IS ABOUT, NOT OF BUFFERS (#1203). The decision this + test guards reads the relation's row-group list, through `row_group_pkey`. The + arm used to count every shared buffer planning touched, and most of those belong + to PostgreSQL's own catalogs, whose cache state differs from one backend to the + next. That reading failed in CI on both arms, in both directions, and on one + commit red then green. It was never reproduced on a quiet box, so the cause of + the noise is still not known; what is known is that the noise was in buffers this + arm does not care about. + + The rows come from `pg_stat_get_xact_tuples_returned`, which reads THIS + transaction's own counts. No other backend and no autovacuum can add to them, and + plain EXPLAIN plans without executing, so every row counted was read to plan. + Measured on PG17 against this fixture, twelve readings each, with and without + three sessions running DDL alongside: 1000 for both settings with no ORDER BY, + 2000 against 1000 with one. Every reading identical. + + The heap and the index are summed so the count survives a change of access path: + below `index_min_blocks` the same read is a sequential scan of the heap. """ import psycopg - fresh = psycopg.connect(conn.info.dsn, autocommit=True) - total = None + fresh = psycopg.connect(conn.info.dsn) try: with fresh.cursor() as cur: cur.execute(f"SET pgcolumnar.enable_sorted_pathkeys = {guc}") - for _ in range(2): - cur.execute("EXPLAIN (ANALYZE, BUFFERS, COSTS OFF, TIMING OFF) " + sql) - lines = [r[0] for r in cur.fetchall()] - seen_planning = False - for line in lines: - if line.strip().startswith("Planning:"): - seen_planning = True - continue - if seen_planning and "Buffers:" in line: - import re as _re - hit = _re.search(r"shared hit=(\d+)", line) - read = _re.search(r"read=(\d+)", line) - total = (int(hit.group(1)) if hit else 0) + \ - (int(read.group(1)) if read else 0) - break + cur.execute("EXPLAIN (COSTS OFF) " + sql) + cur.fetchall() + cur.execute("SELECT pg_stat_get_xact_tuples_returned(" + "'pgcolumnar.row_group'::regclass) + " + "pg_stat_get_xact_tuples_returned(" + "'pgcolumnar.row_group_pkey'::regclass)") + rows = cur.fetchone()[0] + fresh.rollback() finally: fresh.close() - return total + return rows def test_a_query_that_cannot_use_the_order_does_not_pay_to_decide(planbuf_fx, expect): @@ -1037,43 +1023,31 @@ def test_a_query_that_cannot_use_the_order_does_not_pay_to_decide(planbuf_fx, ex expect.text(_storage(cur, "pb", "sorted_kind"), "lexicographic", "premise: and it is marked, so the claim is not refused at " "condition 1") - on = _planning_buffers(conn, "on", "SELECT count(*) FROM pb WHERE j = 3") - off = _planning_buffers(conn, "off", "SELECT count(*) FROM pb WHERE j = 3") - expect.num(1 if isinstance(off, int) else 0, 1, - "premise: the planning buffer count is a measurement, not an empty " - "string") - # A TOLERANCE, not equality: two backends differ by a couple of buffers whatever - # this code does. Set far below the effect it must detect -- without the guard this - # read +44 on this fixture, and it grows with the group count. - # - # PRINTED ON EVERY RUN, pass or fail. A tolerance is a claim about a distribution - # and the distance from the bound is the only evidence for it, so a green run is a - # data point about headroom rather than a boolean. - # - # THE GROUP COUNT IS IN THE FAILURE MESSAGE, and it is the third value the shell - # twin prints. It is load-bearing rather than context: the delta GROWS with the - # group count, and at a hundred groups the unguarded read was +6 -- close enough to - # the noise floor that the arm could not discriminate. A reader looking at a future - # red needs to know the fixture had its thousand groups before believing the delta. - # It is recoverable from the `premise: the fixture has many groups` arm above, but - # that is the reader doing a join, and not needing one is the whole point. - gap = abs(on - off) - print(f"-- planning buffers, no ORDER BY: on={on} off={off} " - f"|on-off|={gap} (tolerance 5) over {groups} groups") - expect.text("within 5" if gap <= 5 - else f"differs by {gap} (on={on} off={off} over {groups} groups)", - "within 5", + on = _group_list_rows(conn, "on", "SELECT count(*) FROM pb WHERE j = 3") + off = _group_list_rows(conn, "off", "SELECT count(*) FROM pb WHERE j = 3") + expect.at_least(off, groups, + "premise: planning reads the group list at all, so a zero below " + "is not an instrument that cannot see it") + # EQUALITY, NOT A TOLERANCE. The count is this backend's own rows from one catalog, + # so nothing but the planner's own reads can move it. With the guard removed it + # read 2000 against 1000 here: one whole extra pass over a thousand groups. + print(f"-- group-list rows read while planning, no ORDER BY: on={on} off={off} " + f"over {groups} groups") + expect.text("the same" if on == off + else f"differs by {on - off} (on={on} off={off} over {groups} groups)", + "the same", "a query with no ORDER BY does not read the group list to decide") # THE CONTROL that stops the arm above from being satisfied by a function that never - # reads anything: the query that CAN use the ordering must still pay. - order_on = _planning_buffers(conn, "on", "SELECT k FROM pb ORDER BY k LIMIT 10") - order_off = _planning_buffers(conn, "off", "SELECT k FROM pb ORDER BY k LIMIT 10") - margin = order_on - order_off - print(f"-- planning buffers, ORDER BY: on={order_on} off={order_off} " - f"margin={margin} (needs > 5)") - expect.text("reads more" if margin > 5 - else f"margin {margin} (on={order_on} off={order_off})", + # reads anything: the query that CAN use the ordering must still read the list, and + # reading it costs at least one row per group. + order_on = _group_list_rows(conn, "on", "SELECT k FROM pb ORDER BY k LIMIT 10") + order_off = _group_list_rows(conn, "off", "SELECT k FROM pb ORDER BY k LIMIT 10") + print(f"-- group-list rows read while planning, ORDER BY: on={order_on} " + f"off={order_off} (needs at least {groups} more)") + expect.text("reads more" if order_on - order_off >= groups + else f"margin {order_on - order_off} (on={order_on} off={order_off} " + f"over {groups} groups)", "reads more", "control: a query that CAN use the ordering does read to decide") diff --git a/test/sorted_pathkeys.sh b/test/sorted_pathkeys.sh index d8c3d742..37ad6930 100755 --- a/test/sorted_pathkeys.sh +++ b/test/sorted_pathkeys.sh @@ -512,25 +512,28 @@ check_text "a cached ordered plan sees rows appended after it was planned" \ # any claim truncated to NIL at the end anyway, so the read is pure waste, and # it is per plan. # -# The oracle is a COUNT (buffers touched during PLANNING), not a duration. -# Planning-time milliseconds would answer the same question, and on this host -# they are not a fair instrument; a buffer count cannot be moved by another -# process. It is also a work-done check rather than a correctness one: the -# answers are identical either way, so nothing else in this suite could see it. - -# The query is planned TWICE in one session and the SECOND reading is taken. -# A first plan in a fresh backend also fills the catalog caches, and that -# first-touch cost is a couple of buffers that differ between two psql -# invocations. Asserted as equality it made this arm flake by 2 either way. -planbuf() { # planbuf GUC QUERY -> shared buffers hit+read during planning +# The oracle is a COUNT of the rows planning reads from pgcolumnar.row_group, not +# of buffers and not a duration (#1203). The decision reads the row-group list +# through row_group_pkey. This arm used to count every shared buffer planning +# touched, and most of those belong to PostgreSQL's own catalogs, whose cache +# state differs from one backend to the next: it failed in CI on both arms, in +# both directions, and on one commit red then green. It was never reproduced on a +# quiet box, so the cause of that noise is not known. The noise was in buffers +# this arm does not care about. +# +# pg_stat_get_xact_tuples_returned reads THIS transaction's own counts, so no +# other backend and no autovacuum can add to them, and a plain EXPLAIN plans +# without executing. Heap and index are summed so the count survives a change of +# access path. Measured on PG17, twelve readings each, with and without three +# sessions running DDL alongside: 1000 for both settings with no ORDER BY, 2000 +# against 1000 with one, every reading identical. +grouprows() { # grouprows GUC QUERY -> row_group rows read while planning QUERY env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ - -d "$PGC_DB" -At -c "SET pgcolumnar.enable_sorted_pathkeys=$1" \ - -c "EXPLAIN (ANALYZE, BUFFERS, COSTS OFF, TIMING OFF) $2" \ - -c "EXPLAIN (ANALYZE, BUFFERS, COSTS OFF, TIMING OFF) $2" 2>/dev/null \ - | awk '/^Planning:/{p++;next} p==2&&/Buffers:/{h=0;r=0; - if (match($0,/shared hit=[0-9]+/)) h=substr($0,RSTART+11,RLENGTH-11); - if (match($0,/read=[0-9]+/)) r=substr($0,RSTART+5,RLENGTH-5); - print h+r; exit}' + -d "$PGC_DB" -At -c "SET pgcolumnar.enable_sorted_pathkeys=$1" -c "BEGIN" \ + -c "EXPLAIN (COSTS OFF) $2" \ + -c "SELECT pg_stat_get_xact_tuples_returned('pgcolumnar.row_group'::regclass) + + pg_stat_get_xact_tuples_returned('pgcolumnar.row_group_pkey'::regclass)" \ + -c "ROLLBACK" 2>/dev/null | grep -E '^[0-9]+$' | tail -1 } psql_run "CREATE TABLE pb (id int, k int, j int) USING pgcolumnar;" # The row_group catalog holds one row per STRIPE, so stripe_row_limit is what @@ -549,25 +552,24 @@ check "premise: the fixture has many groups, so a per-group read would show" \ check_text "premise: and it is marked, so the claim is not refused at condition 1" \ "$(q "SELECT sorted_kind FROM pgcolumnar.storage WHERE storage_id = pgcolumnar.get_storage_id('pb');")" \ "lexicographic" -PB_NOORDER_ON="$(planbuf on 'SELECT count(*) FROM pb WHERE j = 3')" -PB_NOORDER_OFF="$(planbuf off 'SELECT count(*) FROM pb WHERE j = 3')" -check_num "premise: the planning buffer count is a measurement, not an empty string" \ - "$PB_NOORDER_OFF" "$PB_NOORDER_OFF" -# A tolerance rather than equality, because two backends differ by a couple of -# buffers whatever this code does. It is set far below the effect it has to -# detect: without the guard this arm read +44 on this fixture and grows -# with the group count, while the control below pins that the instrument can -# still see a real read. -PB_DELTA=$(( PB_NOORDER_ON - PB_NOORDER_OFF )) -[ "$PB_DELTA" -lt 0 ] && PB_DELTA=$(( -PB_DELTA )) +PB_NOORDER_ON="$(grouprows on 'SELECT count(*) FROM pb WHERE j = 3')" +PB_NOORDER_OFF="$(grouprows off 'SELECT count(*) FROM pb WHERE j = 3')" +check "premise: planning reads the group list at all, so a zero below is not an instrument that cannot see it" \ + "$([ "${PB_NOORDER_OFF:-0}" -ge "$PB_GROUPS" ] && echo yes || echo "no (${PB_NOORDER_OFF:-empty} over $PB_GROUPS groups)")" "yes" +# Equality, not a tolerance: the count is this backend's own rows from one +# catalog, so nothing but the planner's own reads can move it. With the guard +# removed it read 2000 against 1000 here, one whole extra pass over the list. +echo "-- group-list rows read while planning, no ORDER BY: on=$PB_NOORDER_ON off=$PB_NOORDER_OFF over $PB_GROUPS groups" check "a query with no ORDER BY does not read the group list to decide" \ - "$([ "$PB_DELTA" -le 5 ] && echo yes || echo "no (on=$PB_NOORDER_ON off=$PB_NOORDER_OFF over $PB_GROUPS groups)")" "yes" + "$([ "$PB_NOORDER_ON" = "$PB_NOORDER_OFF" ] && echo yes || echo "no (on=$PB_NOORDER_ON off=$PB_NOORDER_OFF over $PB_GROUPS groups)")" "yes" # The control that stops the arm above from being satisfied by a function that -# never reads anything: the query that CAN use the ordering must still pay. -PB_ORDER_ON="$(planbuf on 'SELECT k FROM pb ORDER BY k LIMIT 10')" -PB_ORDER_OFF="$(planbuf off 'SELECT k FROM pb ORDER BY k LIMIT 10')" +# never reads anything: the query that CAN use the ordering must still read the +# list, which costs at least one row per group. +PB_ORDER_ON="$(grouprows on 'SELECT k FROM pb ORDER BY k LIMIT 10')" +PB_ORDER_OFF="$(grouprows off 'SELECT k FROM pb ORDER BY k LIMIT 10')" +echo "-- group-list rows read while planning, ORDER BY: on=$PB_ORDER_ON off=$PB_ORDER_OFF (needs at least $PB_GROUPS more)" check "control: a query that CAN use the ordering does read to decide" \ - "$([ "$PB_ORDER_ON" -gt $(( PB_ORDER_OFF + 5 )) ] && echo yes || echo "no ($PB_ORDER_ON vs $PB_ORDER_OFF)")" "yes" + "$([ $(( ${PB_ORDER_ON:-0} - ${PB_ORDER_OFF:-0} )) -ge "$PB_GROUPS" ] && echo yes || echo "no ($PB_ORDER_ON vs $PB_ORDER_OFF over $PB_GROUPS groups)")" "yes" # --- the two OTHER paths this hook could reach, which must claim nothing ----- #