From 44d08e7f4aa50c602f1297572bd71ea9bb1d2dd5 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 6 Aug 2026 08:36:47 -0600 Subject: [PATCH 1/3] fix: re-fetch the ClickBench definition every run, and check what came back (#421) The owner decided the definition stays a run-time fetch rather than a vendored copy, so the benchmark tracks upstream. Two things stopped that from being true. The runner kept any file already present, so "fetched at run time" held once and never again: after the first run on a machine it was pinned to whatever was current that day. It now re-fetches every run and says when the bytes changed. PGC_CB_OFFLINE=1 runs against the existing copy for a host with no network, and announces itself, because a run against a stale definition is not comparable to one against the current definition. And curl -sSL without --fail treats HTTP 404 as success. GitHub answers a bad path with a 21-byte "404: Not Found" body and a 404 status, so curl exited 0, the page was written over the real definition, `[ -s ]` was satisfied because the page is not empty, and the run continued against an error page with a digest printed for it. Found with a deliberately bad URL while testing this change; the give-away was both files reporting the same digest. curl now gets --fail, the body is checked for the shape it should have, and neither replaces a good copy until both pass. Verified: with a bad URL and a good cache present, the run dies and the cache is byte-identical afterwards. Each run prints the SHA-256 of both files, because "fetched from main" does not identify anything -- main moves -- and a published number is only reproducible against a definition that can be named. PROVENANCE.md records both owner decisions of 2026-08-06: the run-time fetch, and that the CC BY-NC-SA NonCommercial term is considered acceptable for an open-source project. The second is written as the owner's determination and its reasoning rather than as settled law -- the term restricts use directed toward commercial advantage rather than products as such, and no legal opinion was sought -- so that it can be revisited rather than re-derived. docs/benchmarks.md states that the published table predates the digest and cannot cite one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01E9W9N2tvsvK7hndJgTmqJf --- PROVENANCE.md | 21 +++++++++++---- bench/run_clickbench.sh | 60 ++++++++++++++++++++++++++++++++++++----- docs/benchmarks.md | 22 +++++++++++++++ 3 files changed, 92 insertions(+), 11 deletions(-) diff --git a/PROVENANCE.md b/PROVENANCE.md index 3667acb..6adeeeb 100644 --- a/PROVENANCE.md +++ b/PROVENANCE.md @@ -551,11 +551,22 @@ oracle, so none changes query results. this repository. The runner fetches `postgresql/create.sql` and `postgresql/queries.sql` from upstream at run time, into the benchmark data directory, and feeds them to `psql` unmodified. The comparison oracle is the - heap arm of the same run, not any upstream expected output. **Proposed - 2026-08-05: keep the run-time fetch rather than take a durable in-tree copy. - Owner decision pending, and the NonCommercial term's effect on publishing - benchmark numbers is a separate question that a run-time fetch does not - address.** The measured numbers published in + heap arm of the same run, not any upstream expected output. **Decided + 2026-08-06 by the owner: keep the run-time fetch rather than take a durable + in-tree copy, so the benchmark tracks the current upstream definition.** The + runner therefore re-fetches on every run; a cached copy is used only under + `PGC_CB_OFFLINE=1`, which announces itself in the output, and each run prints + the SHA-256 of the two fetched files so a published number can be tied to the + definition it came from. + + On the **NonCommercial** term, also decided 2026-08-06 by the owner: pgColumnar + is an open-source project rather than a commercial product, so publishing + measurements taken with a CC BY-NC-SA benchmark is considered acceptable use. + Recorded as the owner's determination and its reasoning, not as settled law: + the term restricts use "primarily intended for or directed toward commercial + advantage" rather than products as such, and no legal opinion was sought. It is + written down here so it can be revisited rather than re-derived. The measured + numbers published in `docs/benchmarks.md` are our own, produced on our own hardware. The dataset (`hits.tsv.gz`) is downloaded for local measurement and is not redistributed; its own licensing is unestablished and it must not be added to the tree without diff --git a/bench/run_clickbench.sh b/bench/run_clickbench.sh index 71d5aec..fd6b656 100755 --- a/bench/run_clickbench.sh +++ b/bench/run_clickbench.sh @@ -161,7 +161,7 @@ PSQL="$BINDIR/psql -h /tmp -p $CB_PORT -U postgres -d clickbench -X -q" # 0. Preconditions, once, loudly # --------------------------------------------------------------------------- note "== preconditions" -for t in curl awk zcat "$BINDIR/psql" "$BINDIR/initdb" "$BINDIR/pg_ctl"; do +for t in curl awk zcat sha256sum cmp "$BINDIR/psql" "$BINDIR/initdb" "$BINDIR/pg_ctl"; do command -v "$t" >/dev/null 2>&1 || [ -x "$t" ] || die "missing tool: $t" done mkdir -p "$CB_DATA" || die "cannot write $CB_DATA" @@ -173,15 +173,63 @@ note " rows: $CB_ROWS tries: $CB_TRIES arms: $CB_ARMS" # 1. The definition, fetched rather than vendored # --------------------------------------------------------------------------- note "== fetching the ClickBench definition (not stored in this repository)" +# +# Re-fetched EVERY run, on purpose: the point of not vendoring the definition is +# that the benchmark tracks upstream, and a cached copy silently pins it to +# whatever was current the first time this ever ran on the machine. An earlier +# version kept the cache when the file was merely present, so "fetched at run +# time" was true once and false afterwards. +# +# PGC_CB_OFFLINE=1 keeps an existing copy without reaching the network, for a +# machine that has none. It says so in the output, because a run against a stale +# definition is not comparable to one against the current definition and the +# difference must not be invisible in the log. +# +# curl gets --fail, and the fetched bytes are checked for the shape they are +# supposed to have before they replace a good copy. Both matter, and the second +# is not redundant: +# +# -sSL without --fail treats HTTP 404 as SUCCESS. GitHub answers a bad path +# with a 21-byte "404: Not Found" body and a 404 status, so curl exited 0 and +# wrote that page over the real definition. `[ -s ]` was satisfied -- the page +# is not empty -- the digest was computed and printed, and the run continued +# against an error page. Measured while writing this, with a deliberately bad +# URL. The give-away was both files having the same digest. +# +# So: --fail rejects the status, the grep rejects a body that is not the file we +# asked for, and neither replaces the existing copy until both pass. for f in create.sql queries.sql; do - if [ ! -s "$CB_DATA/$f" ]; then - curl -sSL --retry 3 --max-time 120 -o "$CB_DATA/$f" "$CB_URL_BASE/$f" \ - || die "could not fetch $f" - note " fetched $f" + case "$f" in + create.sql) shape='CREATE TABLE' ;; + queries.sql) shape='SELECT' ;; + esac + if [ "${PGC_CB_OFFLINE:-0}" = 1 ]; then + [ -s "$CB_DATA/$f" ] || die "PGC_CB_OFFLINE=1 but $CB_DATA/$f is not there" + note " OFFLINE: using the existing $f, which may not be current" + elif curl -fsSL --retry 3 --max-time 120 -o "$CB_DATA/$f.new" "$CB_URL_BASE/$f" \ + && [ -s "$CB_DATA/$f.new" ] \ + && grep -qi "$shape" "$CB_DATA/$f.new"; then + if [ -s "$CB_DATA/$f" ] && ! cmp -s "$CB_DATA/$f" "$CB_DATA/$f.new"; then + note " fetched $f -- CHANGED since the last run on this machine" + else + note " fetched $f" + fi + mv "$CB_DATA/$f.new" "$CB_DATA/$f" else - note " have $f already" + rm -f "$CB_DATA/$f.new" + die "could not fetch $f (set PGC_CB_OFFLINE=1 to run against the copy already here)" fi done + +# The digest of what actually ran. A published number is only reproducible if the +# definition it came from can be identified, and "fetched from main" does not +# identify anything -- main moves. Cite these beside any result. +CB_SHA_CREATE=$(sha256sum "$CB_DATA/create.sql" | cut -c1-16) +CB_SHA_QUERIES=$(sha256sum "$CB_DATA/queries.sql" | cut -c1-16) +note " definition: create.sql $CB_SHA_CREATE queries.sql $CB_SHA_QUERIES" +require "the create.sql digest was computed" "${#CB_SHA_CREATE}" "16" || exit 1 +require "the queries.sql digest was computed" "${#CB_SHA_QUERIES}" "16" || exit 1 + NQUERIES=$(grep -c 'SELECT' "$CB_DATA/queries.sql") require "the query file holds 43 queries" "$NQUERIES" "43" || exit 1 # The column count is asserted against the CREATED TABLE further down, not diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 8b5c7ea..0e52574 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -739,6 +739,28 @@ project's MIT license does not carry. The harness fetches the definition from upstream at run time and copies nothing. Our comparison oracle is the heap arm of the same run. +The definition is re-fetched on every run, so the benchmark tracks the current +upstream rather than a copy taken once. That means upstream can change what is +measured between two runs, and a result is only comparable to another result +taken against the same definition. Each run therefore prints the SHA-256 of both +fetched files: + +``` + definition: create.sql 42d28575fd59fb4a queries.sql a7d6673357348ee9 +``` + +Those are the real digests of upstream `main` as of 2026-08-06, 43 queries. + +Cite those beside any number taken from a run. `PGC_CB_OFFLINE=1` runs against +the copy already on the machine, for a host with no network; it says so in the +output, because a run against a stale definition is not comparable to one against +the current definition. + +The table below predates the digest being recorded, so it cannot cite one. The +run was on 2026-08-05 and upstream carried the digests above a day later, but +that is an inference and not a measurement; the next run is the first that will +state it. + The numbers below are one run on 2026-08-05. The conditions were PostgreSQL 18.4 non-assert, 16 cores, 62 GB of memory, and 11,110,833 rows. That row count is every ninth row of the real 100 million row table. The reported time is the best From fe90046e10a561f9783e0c6e0ccd5e7cf06cf7bc Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 6 Aug 2026 08:40:04 -0600 Subject: [PATCH 2/3] docs: keep the ClickBench note inside the measurable STE rules (#421) Two sentences over the 25-word limit, caught by test/docs_style.sh. Split them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01E9W9N2tvsvK7hndJgTmqJf --- docs/benchmarks.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 0e52574..fe8b362 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -751,15 +751,16 @@ fetched files: Those are the real digests of upstream `main` as of 2026-08-06, 43 queries. -Cite those beside any number taken from a run. `PGC_CB_OFFLINE=1` runs against -the copy already on the machine, for a host with no network; it says so in the -output, because a run against a stale definition is not comparable to one against -the current definition. +Cite those beside any number taken from a run. + +`PGC_CB_OFFLINE=1` runs against the copy already on the machine, for a host with +no network. It says so in the output. A run against a stale definition is not +comparable to one against the current definition. The table below predates the digest being recorded, so it cannot cite one. The -run was on 2026-08-05 and upstream carried the digests above a day later, but -that is an inference and not a measurement; the next run is the first that will -state it. +run was on 2026-08-05, and upstream carried the digests above a day later. That +is an inference and not a measurement. The next run is the first that will state +it. The numbers below are one run on 2026-08-05. The conditions were PostgreSQL 18.4 non-assert, 16 cores, 62 GB of memory, and 11,110,833 rows. That row count is From b7296fc659ac6da7cd80d0cf7b62fadfb62aa6ae Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 6 Aug 2026 10:39:30 -0600 Subject: [PATCH 3/3] docs: drop the licence determination from this PR, #457 carries it (#421) CJD is right, and the criticism lands on something I had already flagged and then did anyway. Earlier today I noted that the NonCommercial term restricts use "primarily intended for or directed toward commercial advantage" rather than products as such, and that a project developed by a business is the gray part. Then I wrote the provenance record resting on exactly the footing I had warned against: "pgColumnar is an open-source project rather than a commercial product." That is a stronger claim than the owner made and a riskier one. It also omitted the four specific prohibitions, and the third is the one a well-meaning reader walks into: a benchmark table in documentation is fine, the same table in a sales deck is the prohibited use, and having produced the numbers ourselves does not change that because the definition they came from is the licensed material. A record that says "acceptable use" without saying which uses is the half that gets quoted. #457 records the determination with the owner's own reasoning and the full list, and puts the caution beside the numbers where someone about to lift them will see it. Both PRs touched this paragraph; this one drops it so #457 lands clean. The runner change in this PR is unaffected and was approved as merge-ready. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01E9W9N2tvsvK7hndJgTmqJf --- PROVENANCE.md | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/PROVENANCE.md b/PROVENANCE.md index 6adeeeb..3667acb 100644 --- a/PROVENANCE.md +++ b/PROVENANCE.md @@ -551,22 +551,11 @@ oracle, so none changes query results. this repository. The runner fetches `postgresql/create.sql` and `postgresql/queries.sql` from upstream at run time, into the benchmark data directory, and feeds them to `psql` unmodified. The comparison oracle is the - heap arm of the same run, not any upstream expected output. **Decided - 2026-08-06 by the owner: keep the run-time fetch rather than take a durable - in-tree copy, so the benchmark tracks the current upstream definition.** The - runner therefore re-fetches on every run; a cached copy is used only under - `PGC_CB_OFFLINE=1`, which announces itself in the output, and each run prints - the SHA-256 of the two fetched files so a published number can be tied to the - definition it came from. - - On the **NonCommercial** term, also decided 2026-08-06 by the owner: pgColumnar - is an open-source project rather than a commercial product, so publishing - measurements taken with a CC BY-NC-SA benchmark is considered acceptable use. - Recorded as the owner's determination and its reasoning, not as settled law: - the term restricts use "primarily intended for or directed toward commercial - advantage" rather than products as such, and no legal opinion was sought. It is - written down here so it can be revisited rather than re-derived. The measured - numbers published in + heap arm of the same run, not any upstream expected output. **Proposed + 2026-08-05: keep the run-time fetch rather than take a durable in-tree copy. + Owner decision pending, and the NonCommercial term's effect on publishing + benchmark numbers is a separate question that a run-time fetch does not + address.** The measured numbers published in `docs/benchmarks.md` are our own, produced on our own hardware. The dataset (`hits.tsv.gz`) is downloaded for local measurement and is not redistributed; its own licensing is unestablished and it must not be added to the tree without