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
71 changes: 71 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,77 @@ true until the next version shipped.

### Fixed

- `META.json` named an install script the distribution does not contain, and
nothing had ever read it.

It is the PGXN distribution metadata. It hardcodes the version twice and names
the base install script by filename, and no suite, no Makefile rule and no CI
step consumed it. It went stale for the whole alpha4 cycle:

version 1.0.0-alpha.3 while VERSION said 1.0-alpha4
provides.pgcolumnar.file pgcolumnar--1.0-alpha3.sql

THE FILENAME IS THE HALF THAT MATTERS. `pgcolumnar--1.0-alpha3.sql` does not
exist: this repository opens each cycle by RENAMING the base script to the new
version. So the published metadata pointed at a file PGXN would not receive, and
the two version strings were only the visible symptom. A reader comparing those
against `VERSION` would have called it a cosmetic lag.

Fixed, and now gated in both harnesses. `docs_style.sh` already had a section for
exactly this class -- documents that hardcode a version beside a citation of the
file holding it -- and `META.json` is the same shape with a filename added.
`test/pytest/test_pgxn_metadata.py` asserts the same properties through its own
parse and its own `git archive`.

`git archive` rather than a filesystem test throughout, because the question is
what the DISTRIBUTION contains. An `export-ignore` can drop a file that is plainly
present on disk, which this repository has been bitten by before.

A fourth arm covers what the third cannot: `provides.file` is ONE filename, so an
`export-ignore` dropping a different install script would leave it green while
`ALTER EXTENSION ... UPDATE` broke for anyone who installed from PGXN. It is on
BOTH sides rather than in python alone, because the pytest guards run in CI while
the five-major shell matrix is the release gate, and an arm protecting the upgrade
path for published installs belongs in the gate that runs before a tag.

ITS FIRST SHELL VERSION TRIPPED `selftest/080`, which is the rule working on
arrival: the arm was fine in the pytest twin and only became subject to the
no-pipe sweep when it entered the shell harness. The line was

printf '%s\n' "$_meta_ship" | grep -qx "$_ms"

a builtin writing a captured string into a reader that exits on its first match,
which is #486 exactly. `$_meta_ship` is the whole `git archive | tar -t` listing,
4999 bytes on this tree, right at the pipe-buffer boundary where the shape works
almost every time. Replaced with a `case` over newline-sentinelled text, which
gives the anchored match `grep -x` provided and starts no process. The failure
direction was a false RED rather than a false green, so noisy rather than
dangerous, but it was red in the gate the arm had just been moved into.

The PGXN form is DERIVED from `VERSION` rather than hardcoded -- `1.0-alpha4` to
`1.0.0-alpha.4`, and a future `1.0-beta1` to `1.0.0-beta.1` -- so the arms do not
need editing at the next release. The transform has its own arm, because a
derivation is a claim too: were it wrong, every other arm would compare against a
wrong expectation.

Removal proof, both harnesses: restore `META.json` as it shipped and every
substantive arm reddens while every premise stays green. The premises hold because
the file still parses and still names *a* script; it names the wrong one, which is
the distinction the arms draw.

docs_style.sh 3 arms red, premises green
pytest 2 tests red, 9 pass + 2 fail + 0 unrun = 11

THE TWO COUNTS DIFFER AND THAT IS NOT DRIFT. `docs_style.sh` splits the version
property across two arms with its `_mk` loop, one for `version` and one for
`provides.pgcolumnar.version`, while the pytest twin asserts both inside a single
test. Three and two are the same three assertions counted by their harness's own
unit. An earlier revision of this entry said "two on each side", which the table
beside it already contradicted (@OffgridwithJD).

`guard_tests` re-derived by collection 342 -> 346; `cluster_tests` unchanged at
410, checked in the same run rather than assumed.

- One geometry arm accepted a comparison the property does not describe (#1081).

#1081 replaced a count of `entry->fileOffset != rg->fileOffset` with a per-field
Expand Down
6 changes: 3 additions & 3 deletions META.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,17 @@
"name": "pgcolumnar",
"abstract": "Analytic column storage for PostgreSQL, built as a native table access method",
"description": "pgColumnar is a columnar storage table access method for PostgreSQL, written as a clean-room, MIT-licensed implementation. It reads and writes its own native format, PGCN v1, and supports chunk-group skipping from zone maps and bloom filters, vectorized aggregation, projections, retention, online compaction and reclustering, parallel bulk ingest and export, Apache Arrow and Parquet import and export, Apache Iceberg, and object storage.",
"version": "1.0.0-alpha.3",
"version": "1.0.0-alpha.4",
"maintainer": [
"Joshua D. Drake <jd@commandprompt.com>"
],
"license": "mit",
"release_status": "unstable",
"provides": {
"pgcolumnar": {
"file": "pgcolumnar--1.0-alpha3.sql",
"file": "pgcolumnar--1.0-alpha4.sql",
"docfile": "docs/index.md",
"version": "1.0.0-alpha.3",
"version": "1.0.0-alpha.4",
"abstract": "Analytic column storage for PostgreSQL, built as a native table access method"
}
},
Expand Down
89 changes: 89 additions & 0 deletions test/docs_style.sh
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,95 @@ done
check "every document citing VERSION quotes the version VERSION holds" \
"$(printf '%s' "$_stale" | sed 's/^ //')" ""

# ---- and META.json, which NOTHING read at all ------------------------------
#
# `META.json` is the PGXN distribution metadata. It hardcodes the version TWICE
# and names the base install script by filename, and no suite, no Makefile rule
# and no CI step ever read it. It went stale for the whole alpha4 cycle:
#
# version 1.0.0-alpha.3 while VERSION said 1.0-alpha4
# provides.pgcolumnar.file pgcolumnar--1.0-alpha3.sql
#
# THE FILENAME IS THE PART THAT MATTERS. `pgcolumnar--1.0-alpha3.sql` does not
# exist: it was RENAMED to the alpha4 name when the cycle opened, which is how
# this repository makes each cycle's base script. So the published metadata named
# a file the distribution does not contain, and the version strings were only the
# visible half.
#
# The two version forms differ by convention and that is not a bug: PGXN requires
# three-part semver, so `1.0-alpha4` is published as `1.0.0-alpha.4`. The mapping
# is derived here rather than hardcoded, so a future `1.0-beta1` is covered too.
_meta="$SRCDIR/META.json"
check "premise: META.json is present and parses" \
"$(python3 -c "import json,sys;json.load(open(sys.argv[1]));print('yes')" "$_meta" 2>/dev/null || echo no)" "yes"

# VERSION `1.0-alpha4` -> PGXN `1.0.0-alpha.4`: pad the numeric part to three
# components, and put a dot before the suffix's trailing digits.
_pgxn_ver="$(printf '%s' "$_ver" | awk -F- '{
n = $1; c = split(n, p, "."); while (c < 3) { n = n ".0"; c++ }
if (NF > 1) { s = $2; sub(/[0-9]+$/, ".&", s); print n "-" s } else print n
}')"
check "premise: the PGXN form was derived from VERSION, not empty" \
"$([ -n "$_pgxn_ver" ] && echo yes || echo no)" "yes"

for _mk in version provides.pgcolumnar.version; do
check "META.json $_mk is the version VERSION holds, in PGXN form" \
"$(python3 -c "
import json,sys
m=json.load(open(sys.argv[1]))
for k in sys.argv[2].split('.'): m=m[k]
print(m)" "$_meta" "$_mk" 2>/dev/null)" "$_pgxn_ver"
done

# The defect that actually shipped. `git archive` rather than a filesystem test,
# because what matters is whether the DISTRIBUTION contains it -- an export-ignore
# rule could drop a file that is present in the tree.
_meta_file="$(python3 -c "
import json,sys
print(json.load(open(sys.argv[1]))['provides']['pgcolumnar']['file'])" "$_meta" 2>/dev/null)"
check "premise: META.json names a base install script" \
"$([ -n "$_meta_file" ] && echo yes || echo no)" "yes"
check "the script META.json names is in the published distribution" \
"$(cd "$SRCDIR" && git archive HEAD 2>/dev/null | tar -t 2>/dev/null | grep -cx "$_meta_file")" "1"

# AND THE SCRIPTS META.json CANNOT NAME. `provides.file` is ONE filename, so the arm
# above cannot notice an `export-ignore` that drops a DIFFERENT install script -- an
# upgrade path. That breaks `ALTER EXTENSION ... UPDATE` for anyone who installed
# from PGXN, and nothing else here would see it.
#
# IT IS HERE AND NOT ONLY IN THE PYTEST TWIN FOR A RELEASE REASON. The pytest guards
# run in CI; the five-major shell matrix is the release gate. An arm protecting the
# upgrade path for published installs belongs in the gate that runs before a tag.
# Raised in review of #1086, where it existed only on the python side.
_meta_ship="$(cd "$SRCDIR" && git archive HEAD 2>/dev/null | tar -t 2>/dev/null)"
_meta_ondisk="$(cd "$SRCDIR" && ls pgcolumnar--*.sql 2>/dev/null)"
check "premise: the tree has install scripts to check" \
"$([ -n "$_meta_ondisk" ] && echo yes || echo no)" "yes"
#
# NO PIPE INTO AN EARLY-EXIT READER (#486). The first version of this loop was
# `printf '%s\n' "$_meta_ship" | grep -qx "$_ms"`, which is exactly the shape
# selftest/080 refuses: a builtin writing a captured string into a reader that
# exits on its first match. `$_meta_ship` is the whole `git archive | tar -t`
# listing, 4999 bytes on this tree -- right at the pipe-buffer boundary where the
# shape works almost every time and then does not.
#
# The newline sentinels on both sides give the anchored match `grep -x` was
# providing, without a subprocess. Caught by 080 in review of #1086, which is the
# rule doing its job on arrival: the arm was fine in the pytest twin and only
# became subject to 080 when it entered the shell harness.
_meta_missing=""
for _ms in $_meta_ondisk; do
case $'\n'"$_meta_ship"$'\n' in
*$'\n'"$_ms"$'\n'*) ;;
*) _meta_missing="$_meta_missing $_ms" ;;
esac
done
# A SENTINEL, not an empty expectation: comparing against "" passes on anything
# empty, including a sweep that produced nothing at all.
check "every pgcolumnar--*.sql in the tree is in the published distribution" \
"$([ -z "$_meta_missing" ] && echo "all shipped" || printf '%s' "${_meta_missing# }")" \
"all shipped"


echo "checks run: $checks"
if [ "$fail" = 0 ]; then
Expand Down
49 changes: 49 additions & 0 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ behaviour, the source of that number is named.
- [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)
- [41. test_projections.py: a second copy of some columns, kept honest](#41-test_projectionspy-a-second-copy-of-some-columns-kept-honest)
- [42. test_compression_reaches_the_cascade.py: the codec setting decides encodings too](#42-test_compression_reaches_the_cascadepy-the-codec-setting-decides-encodings-too)
- [43. test_pgxn_metadata.py: the published distribution metadata, which nothing read](#43-test_pgxn_metadatapy-the-published-distribution-metadata-which-nothing-read)

## 1. How to read a test in here

Expand Down Expand Up @@ -4289,3 +4290,51 @@ Removal proof, run on both harnesses: deleting the early return and rebuilding m
`.so` from `c8e5c790dbac` to `6455728725e5` and reddens exactly the codec arms, while
every content invariant stays green. The mutant still writes correct rows, which is the
point of keeping the invariant in the file: this is a decision changing, not corruption.

## 43. test_pgxn_metadata.py: the published distribution metadata, which nothing read

`META.json` is what PGXN receives. It hardcodes the version twice and names the base
install script by filename, and **no suite, no Makefile rule and no CI step ever read
it**. It went stale for the whole alpha4 cycle:

```
version 1.0.0-alpha.3 while VERSION said 1.0-alpha4
provides.pgcolumnar.file pgcolumnar--1.0-alpha3.sql
```

**The filename is the half that matters.** `pgcolumnar--1.0-alpha3.sql` does not
exist. This repository opens each cycle by RENAMING the base script to the new
version, so the published metadata named a file the distribution does not contain.
The two version strings were only the visible symptom, and a reader comparing them
against `VERSION` would have called it a cosmetic lag.

| test | what it establishes |
| --- | --- |
| `test_the_version_derivation_maps_the_forms_this_project_uses` | the transform itself, before anything relies on it |
| `test_meta_json_states_the_version_the_VERSION_file_holds` | both version fields, against the source of truth |
| `test_the_script_meta_json_names_is_in_the_published_distribution` | the defect that actually shipped |
| `test_every_sql_file_meta_json_could_name_is_shipped` | the neighbouring failure: an upgrade script dropped from the archive |

**The first exists because a derivation is a claim too.** PGXN requires three-part
semver, so `1.0-alpha4` publishes as `1.0.0-alpha.4`, and the mapping is computed
rather than hardcoded so a future `1.0-beta1` is covered. If that transform were
wrong, every other arm would compare against a wrong expectation and could pass or
fail for reasons having nothing to do with `META.json`.

**The fourth covers what the third cannot see.** `provides.file` is ONE filename, so
an `export-ignore` that dropped a different install script -- an upgrade path --
would leave the third arm green while `ALTER EXTENSION ... UPDATE` broke for anyone
who installed from PGXN.

`git archive` rather than `os.path.exists` throughout, because the question is what
the DISTRIBUTION contains and not what the working tree holds. An `export-ignore`
attribute can drop a file that is plainly present on disk, and this repository has
been bitten by one before.

**Not a port and not a pair.** `docs_style.sh` asserts the same properties through
its own parse and its own `git archive`; neither file names the other.

Removal proof, run on both harnesses: restore `META.json` as it shipped and the two
substantive arms redden on each side while every premise stays green. The premises
hold because the file still parses and still names *a* script -- it names the wrong
one, which is exactly the distinction the arms draw.
15 changes: 14 additions & 1 deletion test/pytest/expected_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,20 @@
# 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
# 342 -> 346 when test_pgxn_metadata.py landed: four arms over META.json, the PGXN
# distribution metadata that NO suite, Makefile rule or CI step had ever read. It
# named `pgcolumnar--1.0-alpha3.sql`, a file that does not exist -- the cycle-open
# rename moved it -- so the published metadata pointed at something the
# distribution does not contain.
#
# In the GUARD half, correctly: it reads META.json, VERSION and `git archive`, and
# needs neither a cluster nor the driver. `NO_CLUSTER` decides that membership and
# is asserted against the corpus in both directions, so the declaration and the
# property had to agree before this collected here at all.
#
# Re-derived by collection on this tree, never by adding four: `346 tests
# collected`. `cluster_tests` was re-derived in the same run and did NOT move.
guard_tests 346

# 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
4 changes: 4 additions & 0 deletions test/pytest/test_harness_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@
# The parity tool is python and reads python. No cluster and no driver: its input
# is a source file and its output is a verdict about two source files.
"test_compare_to_bash.py",
# The PGXN distribution metadata. Reads META.json and VERSION and runs
# `git archive`; the public seam is the published distribution, so it needs
# neither a cluster nor the driver.
"test_pgxn_metadata.py",
]


Expand Down
Loading
Loading