Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 44 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,50 @@ true until the next version shipped.

### Added

- `test/pytest/test_sorted_pathkeys.py`: the ordered-scan surface, ported from
`test/sorted_pathkeys.sh` (#432). All 110 of its check names, one for one.

The bash suite pins one decision: when a columnar scan may hand the planner
PATHKEYS, the promise that its rows already arrive in a stated order. The planner
then drops the Sort above the scan, and nothing downstream re-checks. So a wrong
promise is not a slow plan, it is WRONG ROWS.

The port keeps the original's three shapes rather than reorganising by feature: a
CLAIM arm (the Sort goes), a REFUSAL arm (something made the claim untrue and the
Sort comes back), and an ANSWER arm (the rows themselves, against a heap table
built from the same data). The ANSWER arm is not a duplicate of the CLAIM arm --
dropping the Sort is only correct if the rows arrive sorted anyway, and a plan
check alone cannot say whether they did.

THE ONE ARM THAT NEEDED MORE THAN A PORT is `pgcolumnar.parallel_copy`, which
prepares one transaction per worker. `max_prepared_transactions` cannot be raised
without restarting the postmaster, and the default is 0, so asking for fewer
workers does not help. `pgc_cluster` now sets it where it writes
`postgresql.conf`, at the value `lib.sh` gives this suite through
`PGC_EXTRA_CONF`. Refusing the arm instead was measured and rejected: it loses
three names outright (`cannot_run` records under the REASON CODE, #1040 phase 0b)
AND turns the `pytest (cluster tests)` job red, because an unrunnable check exits
67 and the job runs pytest under `set -euo pipefail`. That half exits 0 today with
zero unrun, so this file would have been the first to break it.

- `test_docs_cover_the_corpus.py` now refuses a NUMBERED SECTION WITH NO BODY.

The arm above it asks whether each test file is NAMED by a numbered heading. A
heading with no body is still a heading, so a section inserted into the gap between
another heading and its body leaves both files named and one of them documented
under the wrong title -- every existing arm green. `## 37. test_iceberg_fdw.py`
reached `main` sitting directly above `## 38.`, with the Iceberg body attached to
the userinfo heading. Fixed here, and the section order now matches the bodies.

### Fixed

- `compare_to_bash.py`'s corpus arm called a WRAPPED name fabricated. A name too long
for one line is written as adjacent literals, and Python joins them at parse time,
so the joined name is text the file contains but not text `in src` can find. The
arm exists to catch a reader that CONSTRUCTS a name, so it now collapses the file's
own concatenation and keeps exactly that guarantee: an f-string name still yields a
`{}` template, which the collapse does not rescue. Both directions are asserted.

- `orphan-scan` is armed in the matrix runner, so a ledger row naming a deleted
check is refused rather than reported (#983, #1015).

Expand Down
146 changes: 133 additions & 13 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ behaviour, the source of that number is named.
- [37. test_iceberg_fdw.py: the Iceberg FDW's pruning surface](#37-test_iceberg_fdwpy-the-iceberg-fdws-pruning-surface)
- [38. test_objstore_endpoint_userinfo.py: userinfo in an object-store endpoint](#38-test_objstore_endpoint_userinfopy-userinfo-in-an-object-store-endpoint)
- [39. test_hilbert_cluster.py: the Hilbert clustering SQL surface](#39-test_hilbert_clusterpy-the-hilbert-clustering-sql-surface)
- [40. test_sorted_pathkeys.py: when a scan may claim its rows are ordered](#40-test_sorted_pathkeyspy-when-a-scan-may-claim-its-rows-are-ordered)

## 1. How to read a test in here

Expand Down Expand Up @@ -223,6 +224,36 @@ INCOMPLETE branch set a variable the verdict never read.
silently, so the layer refused the cheap dishonest escape and permitted the
expensive-looking one. An escape hatch that costs nothing is the default.

### Before you write a `cannot_run`: it exits 67, and CI acts on that

A run containing one unrunnable check exits **67** — `EXIT_INCOMPLETE`, deliberately the
same number as `lib.sh`'s `PGC_EXIT_INCOMPLETE`, so a runner learns the code once. pytest
itself only uses 0-6, so it collides with nothing.

**The `pytest (cluster tests)` job runs pytest bare under `set -euo pipefail`.** So 67
fails the step, and the job goes red on a check that did exactly what it was supposed to
do. Measured on 2026-09-14: that leg exits 0 with **0 unrun**, so nothing in the cluster
half was producing one and nothing was absorbing it. The next legitimately-unrunnable
cluster arm is the first, and it turns the job red.

The guard half is not in the same position today, but the reasoning is the same.

So, in order:

1. **Try to remove the precondition.** `sorted_pathkeys`' `parallel_copy` arm needed
`max_prepared_transactions` raised before the postmaster starts. That is a line in
`pgc_cluster`'s `postgresql.conf`, so the arm runs and the question disappears. Prefer
this whenever the precondition is something this harness controls.
2. **If it is not yours to control, weigh what refusing costs.** `cannot_run` records
under the REASON CODE, not under a check name, so a ported arm that refuses emits
NONE of the bash names it would have carried and the suite must be declared in
`INCOMPLETE` (#1040 phase 0b). Refusing is not free even before CI sees it.
3. **Only then refuse**, and say in the PR that the cluster job's exit code changes.

The general shape, worth recognising away from here: a truthful "could not evaluate"
sharing one channel with "something is wrong", and a caller that cannot tell them apart.
`orphan-scan` has the same problem with its exit 1 (#1015).

The run now ends `EXIT_INCOMPLETE`, which is 67 — deliberately the same number as
`PGC_EXIT_INCOMPLETE` in `lib.sh:58`, because a runner that learns the code should
learn it once. pytest itself uses 0–6, so 67 collides with nothing. The reason and
Expand Down Expand Up @@ -1069,6 +1100,7 @@ many times.
| `test_every_in_document_link_in_this_directory_reaches_a_heading` | every contents-list link resolves, with a coverage premise |
| `test_the_contents_list_is_numbered_in_order` | the contents list and the sections both count 1..N with no gap or inversion — the link arms above ask only whether a link RESOLVES, and a shuffled list resolves perfectly |
| `test_every_test_file_has_a_NUMBERED_section_of_its_own` | a section written as an unnumbered `###` is invisible to every other arm: not in the numbering, not in the contents, and the file is still NAMED so the coverage arm is satisfied — `test_iceberg_fdw.py` shipped that way in #1057 |
| `test_a_numbered_section_has_a_body_of_its_own` | the next one down: a heading with no body still NAMES its file, so a section inserted into the gap between another heading and its body leaves every existing arm green — `## 37. test_iceberg_fdw.py` sat directly above `## 38.` with the Iceberg body under the userinfo title |
| `test_a_shuffled_contents_list_is_caught_on_a_fixture` | **removal proof**: the `29, 31, 30` shape that shipped, with a clean control and an omitted entry named apart from an inversion |
| `test_the_next_steps_list_is_anchored_to_the_inventory` | every section 5 entry names a mode id, so the entry can be checked at all |
| `test_no_open_next_step_names_work_the_document_calls_done` | an un-struck entry whose id reached section 2 is stale work to do |
Expand Down Expand Up @@ -3880,19 +3912,6 @@ the tool grades THIS tree.

## 37. test_iceberg_fdw.py: the Iceberg FDW's pruning surface

## 38. test_objstore_endpoint_userinfo.py: userinfo in an object-store endpoint

Not a port and not a pair: `objstore_endpoint_userinfo.sh` does not exist. These assert
the same properties as `test/objstore_userinfo.sh`'s endpoint arms, independently,
through the python harness.

| test | asserts |
| --- | --- |
| `test_a_userinfo_endpoint_is_refused` | both shapes refuse at `22023`, naming userinfo and naming the ENDPOINT rather than the s3:// URL |
| `test_the_guard_fires_without_a_region_configured` | the placement: with no region set the refusal is userinfo, not the region demand |
| `test_a_clean_endpoint_is_not_refused_as_userinfo` | the control -- a clean endpoint gets past the guard and fails for another reason |
| `test_an_at_sign_in_the_object_key_is_not_userinfo` | the other direction: `@` is legal in a key and is untouched |

Ports `test/iceberg_fdw.sh`. 74 of its 76 check names, one for one; the two it cannot
carry are `pgc_skip`'s refusal names, which are structural and declared in
`INCOMPLETE` with their reason.
Expand Down Expand Up @@ -3927,6 +3946,20 @@ carry are `pgc_skip`'s refusal names, which are structural and declared in
| `test_a_plan_with_no_pruning_marker_is_not_read_as_zero` | a plan that never mentions `Files Pruned` is not read as 0; needs no server |



## 38. test_objstore_endpoint_userinfo.py: userinfo in an object-store endpoint

Not a port and not a pair: `objstore_endpoint_userinfo.sh` does not exist. These assert
the same properties as `test/objstore_userinfo.sh`'s endpoint arms, independently,
through the python harness.

| test | asserts |
| --- | --- |
| `test_a_userinfo_endpoint_is_refused` | both shapes refuse at `22023`, naming userinfo and naming the ENDPOINT rather than the s3:// URL |
| `test_the_guard_fires_without_a_region_configured` | the placement: with no region set the refusal is userinfo, not the region demand |
| `test_a_clean_endpoint_is_not_refused_as_userinfo` | the control -- a clean endpoint gets past the guard and fails for another reason |
| `test_an_at_sign_in_the_object_key_is_not_userinfo` | the other direction: `@` is legal in a key and is untouched |

## 39. test_hilbert_cluster.py: the Hilbert clustering SQL surface

The port of `test/hilbert_cluster.sh` (#432, #889's SQL half). The bash suite pins the SQL
Expand Down Expand Up @@ -4021,3 +4054,90 @@ the surface and the recorded kind and must never be read as evidence of Hilbertn
| `test_the_install_script_and_the_catalog_agree_on_the_symbol_set` | S8, symbols resolved from the AS clause and never derived |
| `test_each_new_verb_is_installed_and_its_symbol_declared` | installed once, C, and declared |
| `test_each_new_verb_has_its_siblings_signature` | args, VARIADIC element and return type, compared against the sibling rather than retyped |


## 40. test_sorted_pathkeys.py: when a scan may claim its rows are ordered

Ports `test/sorted_pathkeys.sh` (#432), all 110 of its check names, one for one.
The bash suite pins one decision: when a columnar
scan may hand the planner PATHKEYS -- a promise that the rows come out in a stated order,
which lets the planner drop the Sort above it. A wrong promise is not a slow plan, it is
WRONG ROWS, because nothing downstream re-checks the order.

So every arm here is in one of three shapes, and the file is organised by them rather than
by feature:

- **CLAIM.** The relation really is ordered, the Sort really does disappear.
- **REFUSAL.** Something made the claim untrue -- an append, an UPDATE, a rewrite, a
collation change -- and the Sort must come BACK.
- **ANSWER.** The rows themselves, against a heap table built from the same data. This is
the shape that catches a wrong claim, because a plan check alone cannot: a scan that
promises an order it does not keep produces a plan that looks right.

An ANSWER arm is not a duplicate of its CLAIM arm. Dropping the Sort is only correct if the
rows arrive sorted anyway, and only the heap comparison can say whether they did.

### The arm that needed a cluster setting, not a workaround

`pgcolumnar.parallel_copy` prepares one transaction per worker, and
`max_prepared_transactions` cannot be raised without restarting the postmaster. The
default is 0, so asking for fewer workers does not help: any number of workers is one
too many.

`pgc_cluster` therefore sets it where it writes `postgresql.conf`, at the value
`lib.sh` gives this suite through `PGC_EXTRA_CONF`. The alternative -- refusing the arm
with `expect.cannot_run` -- was measured and rejected for two reasons. It would have
lost three of the bash suite's names outright, because `cannot_run` records under the
REASON CODE rather than under a name (#1040 phase 0b). And it would have turned the
`pytest (cluster tests)` job RED: an unrunnable check exits 67, the job runs pytest
under `set -euo pipefail`, and no file in that half had ever produced one. Measured:
the cluster leg exits 0 today with 0 unrun.

The arm asserts `pg_prepared_xacts` is empty afterwards. A prepared transaction left
behind holds its locks until someone resolves it, and this cluster is session-scoped --
so a leak would not fail this test, it would wedge every file that runs after it.

### What the port asserts that the original gets for free

`psycopg` returns a PostgreSQL array as a python list, so `{k,j}` arrives as `['k','j']`
and a text comparison against the bash suite's expected output would fail for a reason that
has nothing to do with ordering. The port casts to `::text` in SQL instead of comparing
python objects, so both harnesses are reading the same string the server produced.

The COPY arms need a directory the SERVER can write. `tmp_path` is under
`/tmp/pytest-of-root/`, mode 700, which the backend cannot reach -- so a `server_dir`
fixture makes a world-writable one. The bash suite never meets this because it runs its
psql as the same user.

| test | asserts |
| --- | --- |
| `test_the_fixture_really_is_ordered` | the premise: the rows are in the order the test is about, measured by inversions rather than assumed |
| `test_a_real_ordering_loses_the_sort` | the CLAIM: an order the rows are actually in drops the Sort |
| `test_an_order_the_rows_are_not_in_keeps_the_sort` | the control: a different order must still pay for a Sort |
| `test_the_columnar_answer_matches_heap_in_order` | the ANSWER: the rows, against a heap built from the same data |
| `test_a_constant_leading_key_is_skipped` | a leading key with one distinct value cannot prove the second key's order |
| `test_a_run_with_an_appended_tail_is_not_an_ordered_relation` | rows appended past the run end the ordering, however sorted the run still is |
| `test_the_tail_answer_matches_heap` | and the rows after the append are still right |
| `test_a_zorder_run_is_not_a_sort_on_its_lead_column` | Z-order interleaves bits, so it orders NEITHER column on its own |
| `test_a_declared_sort_key_is_an_intention_not_a_layout` | a declared key on an unsorted relation is a statement of intent, not evidence |
| `test_an_unsorted_vacuum_retracts_the_ordered_path` | a vacuum that rewrites without sorting must retract the claim |
| `test_a_type_change_rewrite_drops_the_mark` | a rewriting ALTER TYPE changes the values, so the old mark cannot survive it |
| `test_one_updated_row_is_a_row_outside_the_run` | a single UPDATE appends, and one row outside the run is enough |
| `test_the_mark_follows_a_rename` | #778: the mark is stored by name, so a RENAME COLUMN must carry it |
| `test_a_recorded_name_that_no_longer_resolves_is_not_a_claim` | a name that resolves to nothing must retract rather than fall through |
| `test_the_guc_turns_the_claim_off` | the GUC is a real off switch, checked with the Sort back |
| `test_a_ctas_relation_claims_nothing` | CTAS writes rows in whatever order the query produced; nothing records an order |
| `test_a_collatable_sort_column_is_not_claimed` | text order is collation-dependent, so a run sorted under one collation is not sorted under another |
| `test_a_collation_alter_changes_the_order_without_rewriting` | the mechanism: ALTER COLLATION changes the ORDER while the bytes stay put |
| `test_a_domain_and_an_array_carry_their_base_collation` | a domain over text and a text[] inherit the collatability, and the refusal with it |
| `test_a_composite_is_claimed_and_postgres_closes_the_hole` | a composite of two texts, and where PostgreSQL itself refuses first |
| `test_an_enum_add_value_before_does_not_renumber` | `ADD VALUE ... BEFORE` inserts a sort order without renumbering, so a sorted run stays sorted |
| `test_a_cached_ordered_plan_is_retracted` | a plan cached while ordered must be retracted by INSERT, INSERT ... SELECT and a plain append |
| `test_a_cached_plan_is_retracted_by_copy` | the same through COPY, which takes a different write path |
| `test_a_cached_plan_is_retracted_under_parallel_flush` | and under `parallel_flush`, where the rows arrive from workers |
| `test_a_cached_plan_is_retracted_across_backends_by_parallel_copy` | the cross-BACKEND case, the only write path where the invalidation crosses a process boundary; asserts the rows loaded before asserting the retraction, and that no prepared transaction leaked |
| `test_truncate_restarts_numbering_in_a_new_storage` | TRUNCATE gives a new relfilenode, so nothing from the old storage may carry |
| `test_a_reclaiming_rewrite_retracts_while_the_rows_stay_ordered` | the hard case: the rows stay in order and the claim must still go, because the run boundaries moved |
| `test_a_query_that_cannot_use_the_order_does_not_pay_to_decide` | deciding the claim must not read buffers for a query that cannot use it |
| `test_a_projection_does_not_lend_its_order_to_the_base_relation` | a sorted projection is a different relation; its order is not the base table's |
| `test_a_plain_gather_never_sits_above_a_scan_claiming_an_order` | Gather does not preserve order, so the two must never be stacked |
17 changes: 15 additions & 2 deletions test/pytest/expected_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,14 @@
# 340 -> 341: a test file documented as an unnumbered `###` is invisible to every
# other arm -- outside the numbering, outside the contents, and still NAMED, so the
# coverage arm passes (#1024). Re-derived by collection: `341 tests collected`.
guard_tests 341
# 341 -> 342: a numbered section with NO BODY is the next defect down from the one
# above, and 341's arm cannot see it. That arm asks whether each file is NAMED by a
# numbered heading; a heading with no body is still a heading, so a section inserted
# into the gap between another heading and its body leaves both files named and one
# documented under the wrong title. `## 37. test_iceberg_fdw.py` sat directly above
# `## 38.`, with the Iceberg body attached to the userinfo heading, and every arm in
# this file stayed green. Re-derived by collection: `342 tests collected`.
guard_tests 342

# The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran
# in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and
Expand Down Expand Up @@ -189,4 +196,10 @@ guard_tests 341
# and not a pair -- `objstore_endpoint_userinfo.sh` does not exist; these assert the
# same properties through the python harness independently.
# Re-derived by collection: `325 tests collected`.
cluster_tests 325
# 325 -> 373 when test_sorted_pathkeys.py landed: the port of the ordered-scan
# surface (#432), 48 collected tests against the bash suite's 110 check names.
# FORTY-EIGHT COLLECTED AGAINST 110 NAMES, which is not a shortfall -- most arms
# are one function carrying several of the bash suite's names, and the parity
# grader reads names, not functions. Re-derived by collection on this tree, never
# by adding 48 to a number measured on another: `373 tests collected`.
cluster_tests 373
Loading
Loading