diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e22b82e..dfe8718f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/META.json b/META.json index 7b63c7d0..6c9d10ba 100644 --- a/META.json +++ b/META.json @@ -2,7 +2,7 @@ "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 " ], @@ -10,9 +10,9 @@ "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" } }, diff --git a/test/docs_style.sh b/test/docs_style.sh index b664a5ac..ac486d49 100755 --- a/test/docs_style.sh +++ b/test/docs_style.sh @@ -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 diff --git a/test/pytest/TESTS.md b/test/pytest/TESTS.md index d5457485..779a0735 100644 --- a/test/pytest/TESTS.md +++ b/test/pytest/TESTS.md @@ -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 @@ -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. diff --git a/test/pytest/expected_tests.txt b/test/pytest/expected_tests.txt index f788b820..0d9e1bd5 100644 --- a/test/pytest/expected_tests.txt +++ b/test/pytest/expected_tests.txt @@ -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 diff --git a/test/pytest/test_harness_deps.py b/test/pytest/test_harness_deps.py index bee165dc..5baeb154 100644 --- a/test/pytest/test_harness_deps.py +++ b/test/pytest/test_harness_deps.py @@ -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", ] diff --git a/test/pytest/test_pgxn_metadata.py b/test/pytest/test_pgxn_metadata.py new file mode 100644 index 00000000..e9b59a94 --- /dev/null +++ b/test/pytest/test_pgxn_metadata.py @@ -0,0 +1,157 @@ +"""`META.json` is the published distribution metadata and nothing ever read it. + +It hardcodes the version TWICE and names the base install script by filename. No +suite, no Makefile rule and no CI step consumed it, so 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. + +Public seam: the `META.json` file itself and `git archive`, which is what PGXN +receives. Read independently of `docs_style.sh` -- this parses the file and runs +`git archive` itself rather than sharing a helper with the shell arm. + +WHY `git archive` AND NOT `os.path.exists`. The question is whether the +DISTRIBUTION contains the file, not whether the working tree does. An +`export-ignore` attribute can drop a file that is plainly present on disk, and +this repository has been bitten by an export-ignore rule before. +""" + +import json +import subprocess +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +META = ROOT / "META.json" +VERSION = ROOT / "VERSION" + + +def _pgxn_form(version_text): + """`1.0-alpha4` -> `1.0.0-alpha.4`, PGXN's three-part semver. + + DERIVED, not hardcoded, so a future `1.0-beta1` or `1.0` is covered without + editing this file. The numeric part is padded to three components and the + suffix gets a dot before its trailing digits. + """ + head, _, suffix = version_text.partition("-") + parts = head.split(".") + while len(parts) < 3: + parts.append("0") + head = ".".join(parts) + if not suffix: + return head + # Split the suffix by hand rather than with a backreference. The regex form + # was written three times and escaped wrongly twice, because it passes through + # a shell heredoc on the way into this file; a loop cannot be mis-escaped. + digits = len(suffix) + while digits > 0 and suffix[digits - 1].isdigit(): + digits -= 1 + if digits == len(suffix): + return f"{head}-{suffix}" + return f"{head}-{suffix[:digits]}.{suffix[digits:]}" + + +def _archive_members(): + """What `git archive HEAD` would ship, which is what PGXN receives.""" + out = subprocess.run( + ["git", "archive", "HEAD"], cwd=ROOT, capture_output=True, check=True + ).stdout + listing = subprocess.run( + ["tar", "-t"], input=out, capture_output=True, check=True + ).stdout.decode("utf-8", "replace") + return [line for line in listing.splitlines() if line] + + +def test_the_version_derivation_maps_the_forms_this_project_uses(expect): + """The transform, before anything relies on it. + + A derivation is a claim too. If `_pgxn_form` were wrong, every arm below would + compare against a wrong expectation and could pass or fail for reasons that + have nothing to do with `META.json`. + """ + for given, want in ( + ("1.0-alpha4", "1.0.0-alpha.4"), + ("1.0-beta1", "1.0.0-beta.1"), + ("1.0", "1.0.0"), + ("2.1-rc2", "2.1.0-rc.2"), + ): + expect.text(_pgxn_form(given), want, f"{given} maps to PGXN {want}") + + +def test_meta_json_states_the_version_the_VERSION_file_holds(expect): + """Both version fields, against the file that is the source of truth. + + The two forms differ by convention and that is not a bug: PGXN requires + three-part semver, so `1.0-alpha4` publishes as `1.0.0-alpha.4`. + """ + version_text = VERSION.read_text(encoding="utf-8").strip() + expect.text( + "present" if version_text else "empty", + "present", + "premise: the VERSION file has a version to compare against", + ) + want = _pgxn_form(version_text) + + meta = json.loads(META.read_text(encoding="utf-8")) + expect.text(meta["version"], want, "META.json version is VERSION in PGXN form") + expect.text( + meta["provides"]["pgcolumnar"]["version"], + want, + "META.json provides.pgcolumnar.version is VERSION in PGXN form", + ) + + +def test_the_script_meta_json_names_is_in_the_published_distribution(expect): + """The defect that actually shipped. + + `provides.pgcolumnar.file` named a script that had been renamed away at + cycle-open, so the metadata pointed at a file PGXN would not receive. + """ + meta = json.loads(META.read_text(encoding="utf-8")) + named = meta["provides"]["pgcolumnar"]["file"] + expect.text( + "named" if named else "absent", + "named", + "premise: META.json names a base install script", + ) + + members = _archive_members() + expect.text( + "non-empty" if len(members) > 50 else f"only {len(members)}", + "non-empty", + "premise: git archive produced a distribution to look in", + ) + expect.num( + members.count(named), 1, f"the distribution contains {named} exactly once" + ) + + +def test_every_sql_file_meta_json_could_name_is_shipped(expect): + """The neighbouring failure: a script present in the tree and not in the archive. + + `provides.file` is one filename, so the arm above cannot see an `export-ignore` + that drops a DIFFERENT install script -- the upgrade paths. An upgrade script + missing from the distribution breaks `ALTER EXTENSION ... UPDATE` for a user + who installed from PGXN, and nothing else here would notice. + """ + on_disk = sorted(p.name for p in ROOT.glob("pgcolumnar--*.sql")) + expect.text( + "found" if on_disk else "none", + "found", + "premise: the tree has install scripts to check", + ) + members = set(_archive_members()) + missing = [n for n in on_disk if n not in members] + # A SENTINEL RATHER THAN AN EMPTY EXPECTATION. `expect.text(x, "")` is refused + # as vacuous, and rightly: anything empty satisfies it, including a comparison + # whose left side failed to produce anything at all. + expect.text( + "all shipped" if not missing else ", ".join(missing), + "all shipped", + "every pgcolumnar--*.sql in the tree is in the archive", + )