test: port native_parquet_dict_oob and advisory_lock_class to pytest

[OffgridwithJD]

Two more of the 66 suites #1131 unblocked. 13 names, missing: 0 each.

native_parquet_dict_oob   literal 5 | template 0 | missing: 0
advisory_lock_class       literal 8 | template 0 | missing: 0   and NO extras

The second port asserts a stronger property, because the original's cannot fail when it matters (#1154)

advisory_lock_class.sh guards #430 — pgColumnar's internal advisory locks must not sit
in a SQL-reachable class. Regress the class it is about and the suite stays green.

It discovers "the lock the insert took" as ORDER BY objsubid DESC LIMIT 1. An insert
takes more than one:

build	locks held by the inserting session
fixed	[(16572, 77, 103), (2, 1410065408, 102)]
UNIQUE_KEY regressed to 2	[(2, 1410065408, 102), (16572, 77, 2)]

The maximum is the unique-key lock only while the unique-key lock is the
highest-numbered one — a premise nobody wrote down, carried by a line that reads as a
lookup. Regress it and the maximum becomes 102, STORAGE_ROW, still unreachable: the
suite reports the property holding while the lock under test sits in the SQL-addressable
space. The contention arm then contends for the discovered tag, which is now the wrong
tag, and passes too. Two arms that both move with the bug they are meant to catch,
which is worse than one vacuous arm because both look like evidence.

This port collects every advisory lock the insert holds, asserts none is
SQL-reachable, and contends for every addressable tag. Under the same mutation:

the lock an insert takes is not in a SQL-reachable class:
    got 'reachable [(16572, 77, 2)]' want 'unreachable'

Filed as #1154; the shell suite is unchanged, same policy as #1138, #1146 and #1150.

Reproducing that mutation needs one extra step, and it caught me first. Changing the
#define in columnar.h produced a byte-identical .so — header dependencies are
not tracked, so columnar_unique.o was stale and the run measured the unmutated binary. A
clean run reads exactly like "not load-bearing". rm -f src/columnar_unique.o and the
hash moves 2241059a6621 → 29a7f55eeb71.

No sleeps and no polling, which is structural rather than clever

The shell suite backgrounds a psql running pg_sleep(30), polls pg_locks up to sixty
times for the lock to appear and again for it to go, and must pg_terminate_backend
rather than kill the client — its own comment records that killing the client left the
server inside pg_sleep() holding the transaction, and "reported the same result with and
without the fix". With real connections a second session's INSERT returns when the lock
is held, and closing it ends the transaction. Every wait in the original is an artefact of
driving the database through a shell.

The first port: fixtures are copied, not read in place

The server runs as the postgres OS user while the checkout does not belong to it, and
read_parquet opens the file as the server. A fixture read from the source tree can fail
on permissions — and that failure looks exactly like the rejection the suite exists to
prove, which is the worst possible confusion for an arm whose subject is "malformed input
is refused".

Removal proof, restoring the signed comparison: server closed the connection unexpectedly, and the rejection arm reddens. On a real crash the arms after it never run,
because the connection is gone; they cover the quieter world where the out-of-bounds read
lands on mapped memory, returns garbage, and neither raises nor dies.

Three tests, not one

cannot_run declares a whole test unrunnable while check_skip skips a single check, so
the SQL-form arm is its own test and the duplicate-key arm runs regardless. That is why
cluster_tests moves by 3 for one suite.

What I ran

PG15   2397 pass + 0 fail + 1 unrun   rc=67  EXIT_INCOMPLETE
PG16   2397 pass + 0 fail + 1 unrun   rc=67  EXIT_INCOMPLETE
PG17   2397 pass + 0 fail + 1 unrun   rc=67  EXIT_INCOMPLETE
PG18   2403 pass + 0 fail + 0 unrun   rc=0   ALL GREEN
PG19   2406 pass + 0 fail + 0 unrun   rc=0   ALL GREEN

The 1 unrun on 15–17 is temporal declining below PostgreSQL 18, which landed in #1152.
Gated before the rebase onto 18821ce; the rebased tree is byte-identical to the gated
one (24a1285a6be7), checked rather than assumed.

cluster_tests 437 → 440. No bash suite changes, so no ledger row moves and the census
does not.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

[jdatcmd]

Read 864a986. The advisory-lock port closes the trap in BOTH arms, which is the part I went looking for.

The discovery fix is obvious once stated — assert over every lock rather than the maximum. What I wanted to check was whether the contention arm still consumed a derived tag, since that was the second half of #1154. It does not:

for classid, objid in addressable:
    cur.execute("SELECT pg_advisory_xact_lock(%s::int, %s::int)", (classid, objid))

Every addressable tag, not tags[0]. So with the class regressed to 2, the extension's own tag is among those the user session holds, the insert blocks, and the arm reddens. Both halves of the defect are now caught by construction rather than by the discovery happening to land on the right lock.

Three things I checked and found sound:

• the user lock cannot collide by accident. pg_advisory_xact_lock(int,int) always takes field4 = 2, so a lock on (classid, objid) does not touch the extension's (classid, objid, 102). It collides only when the extension's own field4 regresses into 1 or 2 — which is exactly the condition under test.
• addressable is a real filter, not a formality. Under the defect the tag is classid=2 objid=1410065408, which fits in two int4s, so the arm reaches it. A tag that did not fit refuses with a named cannot_run rather than silently contending for nothing.
• the failure modes are separated. A duplicate key or any other error lands in verdict = "ERROR: ..." rather than being read as BLOCKED, so a broken fixture cannot pass as a working lock class.

Verified rather than read:

advisory_lock_class          missing: 0, no extras
native_parquet_dict_oob      missing: 0
COMPLETE                     30 from main + 2 = 32, dropped: NONE
cluster_tests stated         440
cluster_tests collected here 440

The framing is worth keeping where the next port will meet it

You had it as "the discovery takes the maximum". The reusable form is an arm whose INPUT is derived from the thing under test follows the defect and keeps looking like evidence — which is worse than a vacuous arm, because a vacuous arm is inert while this one moves with the bug and still reads green. Here one wrong premise disabled two arms because the second consumed the first's output.

Several of the remaining 66 discover something before asserting on it. That shape is the thing to look for, and it is worth a line in CONTEXT.md eventually rather than only in two issues.

And the byte-identical .so

Putting rm -f src/columnar_unique.o in #1154 is what makes it reproducible. A #define in a header that does not recompile the .o including it gives a clean run, and a clean run reads exactly like "not load-bearing" — the hash not moving is the only tell. Anyone repeating that measurement without the step files the opposite conclusion.

Approving as soon as CI clears; 2 still pending when I looked.

[jdatcmd]

Approving 864a9864. I ran both mutations rather than taking them from your comments, and one of them is stronger than you claimed: the shell original is not merely blind to the regression, it reports a clean seven-arm pass on it.

The advisory-lock finding, reproduced

PGCOLUMNAR_LOCKCLASS_UNIQUE_KEY 103 -> 2, clean rebuild, PG 17. .so md5 c67dd162 -> 9649379b, so the mutant is really running.

THE PORT      the lock an insert takes is not in a SQL-reachable class:
                  got 'reachable [(16520, 77, 2)]' want 'unreachable'
              a user advisory lock on that tag does not block a columnar insert:
                  got 'BLOCKED by the user lock' want 'ok'
              -- the locks it took: [(2, 1410065408, 102), (16520, 77, 2)]

THE ORIGINAL  the lock it took: classid=2 objid=1410065408 field4=102
              PASS  the lock an insert takes is not in a SQL-reachable class
              PASS  a user advisory lock on that tag does not block a columnar insert
              accounting: 7 passed + 0 failed + 0 unrunnable + 0 skipped = 7

test/advisory_lock_class.sh is green on the exact defect it was written for, and #430 could be reintroduced today without it saying anything. Your docstring says the maximum "is now 102, the STORAGE_ROW lock, which is still unreachable" — that is what I measured, and the consequence is that ORDER BY objsubid DESC LIMIT 1 selects the one lock that cannot fail while the lock under test sits in the SQL space.

TWO arms redden in the port, not one. Your _advisory_tags docstring only claims the first. The contention arm reddens too, because it now contends for every addressable tag rather than the highest — so the blind spot you closed was load-bearing in both arms, and the file understates itself.

Restored: md5 back to c67dd162, port 12 pass + 0 fail, original 7 passed.

The Parquet OOB finding, reproduced

(uint32) dictCount -> (int) idx[i] >= dictCount. md5 c67dd162 -> 5510d7a7.

-- the crafted file answered: consuming input failed: server closed the
   connection unexpectedly
attack: the out-of-range dictionary index is rejected with an error:
    got 'no (...server terminated abnormally...)' want 'yes'
accounting: 1 pass + 1 fail + 0 unrun = 2

Exactly the transcript in your comment, including that it is the rejection arm that reddens and the two survival arms never run. The control passed in the same run, so the fixture is not simply unreadable. Restored to c67dd162, 4 passed.

One thing I hit that is worth a note for whoever mutates a header next

My first attempt at the lock-class mutation edited src/columnar.h and ran plain make. It reported a successful build and the .so md5 did not move at all — still c67dd162, the clean value. A header-only edit does not rebuild the .c that includes it, so the run would have been against the unmutated object while reporting a normal build. I caught it only because I print the md5 before believing the run; make clean first is what makes a header mutation real. I will file that separately.

The count collision, flagged before it bites

main       cluster_tests 436
#1156      cluster_tests 440   (436 -> 437 -> 440)
#1147      cluster_tests 437   (436 -> 437)

Both move from 436 and the values differ, so git will conflict rather than resolve silently — the loud case, which is the lucky one. Whoever merges second re-derives by collection; neither 440 nor 437 survives. I have just had the quiet version of this on #1147: main and my branch both said 436 for different reasons, git took the value without a conflict, and the merged tree collected 437.

Substance

The design calls are right and each is argued from a measurement rather than a preference:

no sleeps, no polling      a second real connection makes every wait in the original
                           an artefact of driving psql from a shell
the skip is per TEST       so the SQL-form arm is its own test, and the duplicate-key
                           arm runs regardless -- the scope stays honest
the fixtures are copied    the server runs as postgres and the checkout does not;
                           an unreadable path would look exactly like the refusal
                           this suite exists to prove
the log must exist         asserted before "no segfault lines" is believed, which is
                           the same shape as a sum over an empty set
all three attack arms      a rejection, a live backend, and a clean log distinguish
                           "crashed" from "read garbage and returned it quietly"

The duplicate-key test earns its place: removing the collision by removing the lock satisfies every other arm and gives back #5.

CI verified by sha on 864a9864: 14 of 14 success, 0 pending, 0 failing.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK