Skip to content

test: port two delete-visibility suites to pytest - #1148

Merged
jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:port/432-delete-visibility
Sep 19, 2026
Merged

jdatcmd merged 1 commit into
commandprompt:mainfrom
OffgridwithJD:port/432-delete-visibility

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Two suites ported: native_delete_vector_index and native_delete_visibility_paths, 11
names, each graded missing: 0.

native_delete_vector_index       literal 4 | template 0 | missing: 0
native_delete_visibility_paths   literal 7 | template 0 | missing: 0

The shell suite names four access paths and takes two

native_delete_visibility_paths.sh exists to show the delete-vector fold and the per-row
bit test agree across independent paths. It sets enable_seqscan=off and
enable_indexonlyscan=on and trusts the names. None of the enable_* scan GUCs governs
Custom Scan (PgColumnarScan)
, so measured on this fixture:

arm, as the shell suite runs it plan actually taken
sequential scan (row-emitting path) Custom Scan
index / bitmap scan Custom Scan
index-only scan Custom Scan
aggregate over the whole table Custom Scan (vectorized aggregate)

Three arms named for three paths exercise one. What they assert is true — each returns
4286 — but it is one piece of evidence counted three times.

pgcolumnar.enable_custom_scan = off is the switch that does move it, and every named
path is then reachable and still correct:

forced            plan               live set
heap sequential   Seq Scan               4286
index / bitmap    Index Scan             4286
index-only        Index Only Scan        4286

So this port forces the path each arm is named for and asserts the node before reading
the count
. It also asserts two things the original leaves in a comment: that the deletes
span more than one row group (each group builds its own mask), and that the rolled-back
delete left its row in place, so the in-transaction arm cannot be satisfied by a delete
that simply persisted.

The indexed delete-vector read is only ever measured on a fresh session (#1146)

native_delete_vector_index.sh asserts seq_scan = 0 on pgcolumnar.delete_vector after
a scan builds the liveness cache. It passes. It passes for a reader that did not do the
writing:

the scan runs in...                  idx_scan   seq_scan
the session that wrote and deleted         62         20
a fresh session                            21          0

Twenty sequential catalog scans, one per row group — the shape the index switch existed to
remove — survive in the writing session. The shell harness cannot see it, because every
statement goes through its own psql and it has no long-lived session to see it with. I
found it because a pytest test holds one connection for the whole test and the arm failed.

This port asserts the property the suite states, on a second connection, and asserts
nothing about the writing session in either direction
— I would rather file the
observation than pin a number nobody has decided is correct. Filed as #1146.

Four cells, because I first suspected the wrong variable; it is not the option mechanism:

                  same session        fresh session
GUC at write       idx=62 seq=20       idx=21 seq=0
set_options        idx=62 seq=21       idx=21 seq=0

One adaptation that is not a style choice

The delete_vector row count is scoped by storage_id. The shell suite counts the
catalog whole, against a cluster it created moments earlier where its table is the only
writer. A pytest worker shares ONE cluster across the corpus and that catalog is
database-wide, so unscoped this arm would pass or fail on test ORDER — the worst kind of
flake, because it reproduces only in a full run.

What I ran

PG15   815 passed   2375 pass + 0 fail + 0 unrun   rc=0
PG16   815 passed   2375 pass + 0 fail + 0 unrun   rc=0
PG17   815 passed   2375 pass + 0 fail + 0 unrun   rc=0
PG18   815 passed   2375 pass + 0 fail + 0 unrun   rc=0
PG19   815 passed   2375 pass + 0 fail + 0 unrun   rc=0

No shell suite changes, so no ledger row moves and the census does not. cluster_tests
433 → 435, re-derived by collection on the tree reseated onto 6082c5a.

TESTS.md section collision with #1147. That PR takes 64 and so does this one; I took
64 and 65 before it was opened and I am not renumbering pre-emptively. Whichever merges
second reseats — the numbers are not independent and the conflict is loud, so neither can
land blind.

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

native_delete_vector_index and native_delete_visibility_paths, 11 names, each
graded missing: 0 by compare_to_bash.py.

THE SHELL SUITE NAMES FOUR ACCESS PATHS AND TAKES TWO. None of the enable_* scan
GUCs governs Custom Scan (PgColumnarScan), so its sequential, index/bitmap and
index-only arms all plan the same node. The property is true -- each returns
4286 -- but three arms are one piece of evidence counted three times.
pgcolumnar.enable_custom_scan = off moves it, and each named path is then
reachable and still correct. This port forces the path each arm is named for and
asserts the node before reading the count.

AND THE INDEXED DELETE-VECTOR READ IS ONLY MEASURED ON A FRESH SESSION (commandprompt#1146).
The shell harness sends every statement through its own psql, so the scan it
measures is always made by a session that did no writing; in the writing session
twenty sequential catalog scans survive, one per row group. The port asserts the
suite's property on a second connection and asserts nothing about the writing
session.

The delete_vector count is scoped by storage_id, because that catalog is
database-wide while a pytest worker shares one cluster across the corpus.

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

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 213fa81. Everything below I derived here rather than read from the body.

reseated onto 6082c5a          yes (merge-base)
COMPLETE                       27 from main + 2 = 29, dropped from main: NONE
TESTS.md                       64, 65
ledger / budget / registry     untouched; no bash check added
compare_to_bash                both pairs, missing: 0
cluster_tests stated           435
cluster_tests collected here   435

The Custom Scan finding is the third sighting today and the sharpest statement of it

The shell suite names four paths and takes two.

Three arms named for three access paths all planned Custom Scan, so the property they assert is true and counted three times. That is a harder shape to catch than a vacuous arm, because nothing is green that should be red — the evidence is simply worth a third of what the names imply.

This is the third distinct appearance of the same trap between us today, and it cost me a vacuous arm twice: my fetch arms were answered by an Index Only Scan that never called the table AM, and then by Custom Scan (PgColumnarScan) because enable_seqscan does not govern it. Your port does the thing that fixes all three: force the path with pgcolumnar.enable_custom_scan = off, then assert the node before reading the count.

One detail worth naming because it is easy to get wrong the other way: expect.contains(types, "Index Scan", ...) over a list of node types is exact element matching, so Index Only Scan does not satisfy the index arm. A string haystack would have. The premise is sound as written.

The second finding is better than the fix it belongs to

the scan runs in...          idx_scan   seq_scan
the session that wrote             62         20
a fresh session                    21          0

Twenty sequential catalog scans, one per row group, survive in the writing session — the exact cost the index switch removed, still being paid by whoever just wrote. And the shell suite is structurally incapable of seeing it, because every statement goes through its own psql and there is never a long-lived writing session to observe.

Asserting the suite's property on a fresh connection and asserting nothing about the writing session is the right call, with #1146 carrying the residue. A port that quietly widened its claim to cover the writing session would have been wrong in the other direction.

The scoping notes are the kind that save someone a day: pgcolumnar.delete_vector is database-wide and a pytest worker shares one cluster, so an unscoped count passes or fails on test ORDER — reproducible only in a suite run, which is the worst flake there is.

Small things I checked and found sound

  • LIVE = 4286 is right: id % 7 = 0 over 5,000 rows deletes 714.
  • The in-transaction arm has its control — the rolled-back delete is asserted to have left the row in place, so "invisible in the same transaction" cannot be satisfied by a delete that simply persisted.
  • expect.at_least(groups, 2) reads the group count back from the catalog instead of trusting the stripe_row_limit that asked for it. The shell suite's comment says five groups and nothing checks it.
  • RESET ALL followed by restoring search_path is correct and the comment explains why it must be both.

Merge order

TESTS.md 64 collides with #1147, which also takes 64. You took it first and I reseat, as I said — #1147 has been rebased once already tonight and should not hold yours. Merging this now.

@jdatcmd
jdatcmd merged commit 3a2ef2f into commandprompt:main Sep 19, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants