Skip to content

test: count group-list rows, not planning buffers, in the #1203 arm #1287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jdatcmd merged 1 commit into from
Oct 12, 2026
Merged

test: count group-list rows, not planning buffers, in the #1203 arm #1287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
132 changes: 53 additions & 79 deletions test/pytest/test_sorted_pathkeys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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")

Expand Down
70 changes: 36 additions & 34 deletions test/sorted_pathkeys.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 -----
#
Expand Down
Loading