Skip to content

feat: Hilbert curve clustering, and open the 1.0-alpha4 cycle (#889) - #899

Merged
jdatcmd merged 6 commits into
mainfrom
feat/889-hilbert-clustering
Sep 9, 2026
Merged

feat: Hilbert curve clustering, and open the 1.0-alpha4 cycle (#889)#899
jdatcmd merged 6 commits into
mainfrom
feat/889-hilbert-clustering

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Closes #889. Alpha4's first feature, and it opens the 1.0-alpha4 cycle.

pgcolumnar.cluster_hilbert(regclass, VARIADIC name[])    -> void
pgcolumnar.recluster_hilbert(regclass, VARIADIC name[])  -> bigint

Five commits, red-green-refactor-red-green. Each test commit precedes the code it holds, and the
red run is in each message.

The construction

Skilling's AxesToTranspose feeding the packer that already shipped, unchanged. Skilling's
observation is that the Hilbert index is the MSB-round-first interleave of the transposed
coordinates — bit for bit the loop that was already in cluster_zorder_key. So:

Z-order:  ordinals -> pack
Hilbert:  ordinals -> transpose -> pack

Same width (8*ncols), same bytea, same memcmp-equals-curve-order invariant, so the sort, the
augmented TupleDesc, byteaLt and the writer are untouched by the choice of curve. The curve is
threaded as one parameter through the key builder, the self-gate's strcmp and the recorded
extent, so those three cannot disagree.

Two suites, and why the property arms are not enough on their own

test/hilbert_curve.sh, 184 arms, pure C against stub headers — no cluster. It carries two
deliberately wrong encoders as controls, because unit-step adjacency alone certifies a
serpentine
and dyadic contiguity alone certifies Z-order, the curve that already shipped. The
battery is non-vacuous only as the pair, and each control is pinned to an exact count:

serpentine: C1 violations=0 C2 violations=0 C3 violations=621870 in 25 of 25 nested cases
Z-order:    C1 violations=0 C2 violations=2123558 in 32 of 32 multi-column cases, C3 violations=0

test/hilbert_cluster.sh, 181 arms, the SQL surface, the gate, vacuum_sorted and the daemon.

96 golden byte vectors are the only arms that pin which Hilbert curve this is. Skilling's
differs from Butz/Hamilton for ncols >= 3 and both are valid, so no property arm can separate
them and the bytes are an on-disk format commitment. They were derived from the published algorithm
and reproduced byte-for-byte by two reviewers before src/columnar_curve.c existed, which is
the only reason a golden table is worth anything.

Removal proof

Five mutants of the curve, each asserted applied (the patch raises if its anchor is missing, and a
mutation whose md5 matches the original is refused as a no-op), each restored, file verified back
at bf5f7e07105f:

mutant passed failed
unmutated 184 0
M1 transpose is identity — i.e. Z-order 133 51
M2 no Gray encode 133 51
M3 no final t fold 145 39
M4 invert/exchange swapped 122 62
M5 pack LSB-first in byte 69 115

M1 is the one that matters: neutering the transpose turns this back into the curve that already
ships, and 51 arms notice.

The owner ruling: the curve is sticky

sorted_kind is the table's declared intent. Plain recluster() on a Hilbert table with a matching
key is a no-op, not a conversion; switching curves requires naming the other verb;
vacuum_sorted skips a Hilbert table rather than re-sorting it lexicographically; the daemon
dispatches on the recorded kind.

Stated rather than hidden: the daemon's dispatch is not load-bearing against this suite.
Reverting it to the hard-coded recluster() leaves all 181 arms green, because the sticky plain
recluster() already preserves the curve. It is kept because the ruling is about the daemon and
should be readable at the call rather than inferred from a rule two modules away — but no arm
guards it, and one that did would have to read the curve out of the daemon's log line, which the
log now carries.

Why this opens 1.0-alpha4

The first implementation put both functions in pgcolumnar--1.0-alpha2--1.0-alpha3.sql.
v1.0-alpha3 is a published pre-release (2026-09-03), so that file is a shipped artifact:
editing it leaves a database that upgraded from the release and a fresh install both reporting
1.0-alpha3 with different function sets and no path between them.

VERSION and pgcolumnar.control          -> 1.0-alpha4
pgcolumnar--1.0-alpha3.sql              -> pgcolumnar--1.0-alpha4.sql
pgcolumnar--1.0-alpha3--1.0-alpha4.sql     new, carrying the two verbs
pgcolumnar--1.0-alpha2--1.0-alpha3.sql     reverted, byte-identical to the branch point

And the newly-previous version gained coverage it never had.
test/native_upgrade_converge.sh staged frozen base scripts for 1.0-alpha and 1.0-alpha2 only,
so a database at the version people actually have had no arm. The shipped alpha3 base script is
now frozen into test/fixtures/ taken from the tag (diff against git show v1.0-alpha3: is
empty, and it contains neither new verb). 8 checks -> 11.

My first edit there added it to the staging loop alone and the count stayed at 8 — a silent no-op,
caught by watching the number rather than the exit status. The arms come from a second loop.

Removal proof: renaming cluster_hilbert in the new upgrade script reddens 9 of 11 arms
including all three 1.0-alpha3 ones, with
function pgcolumnar.cluster_hilbert(regclass, name[]) does not exist.

Verification, on the rebased tree

COPT=-Werror, PG 18.4, build OK with 0 warnings, 0 FAIL lines:

hilbert_cluster  181   projection_rewrite  84   native_upgrade_converge  11
hilbert_curve    184   projections         73   harness_selftest        261
native_cluster    11   recluster_gate      13   entry_point_privilege    31
native_recluster  12   recluster_extent    17   autovacuum               11
sorted_pathkeys  113   vacuum_sorted_gate  34   docs_style           PASSED

projection_rewrite and projections are #892's, run here because this rebase composes two
changes to the maintenance paths and a PR describes its branch, not the composition.

What this does NOT deliver

No locality measurement. The release plan asserts Hilbert "gives better locality" and cites no
measurement of ours; nothing here supplies one, and #889 records why the obvious fixture cannot:
on a dense power-of-two-aligned grid the two curves produce the identical row-to-group
partition and the measured skip ratio is 1.0000. A pilot needs a sparse, skewed, non-dyadic
fixture and must assert the two curves partition differently before any ratio means anything. If it
cannot separate them, "not measurable on our fixtures" is the result to report rather than a
threshold to lower.

The ordinal layer also caps the benefit and is deliberately untouched: matched int4 x int4 gives
mean contiguous run 7.14 vs 12.67, but bool x int4 gives 8.07 vs 8.10 — no win at all.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

jdatcmd and others added 5 commits September 9, 2026 08:14
A standalone C battery over the key construction #889 will add. It needs no
cluster: it writes C, compiles it, runs it, and turns the program's output into
check() arms. It is deliberately NOT registered in run_all_versions.sh, because
a red suite in the matrix is everyone's problem; it joins the matrix in the
commit that adds src/columnar_curve.c.

Red now, and red for the right reason: 174 battery arms report "the code under
test is absent: src/columnar_curve.c src/columnar_curve.h". The 10 INSTRUMENT
arms pass, because they need nothing from src/ -- they are what proves the
battery can tell curves apart before there is a curve to test:

  serpentine: C1 violations=0 C2 violations=0 C3 violations=621870 in 25 of 25
  Z-order:    C1 violations=0 C2 violations=2123558 in 32 of 32, C3 violations=0

That pair is the point. Unit-step adjacency alone certifies a serpentine, which
is not a Hilbert curve. Dyadic contiguity alone certifies Z-order, which is the
curve that already ships. The battery is non-vacuous only as C2 AND C3, and each
control is pinned to an exact count rather than a bound, so a checker stuck at
pass and a checker stuck at fail both redden the suite.

The first version of this file was worse than it looked, which is why the arms
were audited before being trusted. Three adversarial passes compiled thirteen
wrong encoders against it, and TWO OF THEM PASSED 138 of 138 -- one differing
from the correct key on 95.3% of random 8-column inputs. The causes were
specific and are fixed:

  - C4b drew key bytes from the low byte of a 32-bit LCG, whose period there is
    512. It claimed 24,000 round trips per ncols and performed 8 to 64 distinct
    keys, the same 8 keys 375 times each at ncols=8. Now splitmix64, and the
    premise arm asserts the DISTINCT keys the program counted rather than the
    iteration count -- a coverage number the harness could not manufacture.
  - Four arms never called the code under test at all, and stayed green with
    both product functions destroyed. Each is now composed with the encoder
    rather than deleted.
  - C1 had no negative control, though the header claimed every property had
    one, so nothing in the suite could make the permutation checker report a
    violation. A truncating control now does.

The 96 golden byte vectors are the only arms that pin WHICH Hilbert curve this
is -- Skilling's differs from Butz/Hamilton for ncols >= 3 and both are valid,
so the property arms cannot distinguish them and the bytes are a format
commitment. They were derived from the published AxestoTranspose, independently
reproduced byte-for-byte by two auditors from their own transcriptions, and the
small cases hand-checked. The code under test does not exist, so they could not
have been generated from it.

What is NOT claimed: that the battery catches every wrong encoder. It now
catches the three blind spots that were measured, and the mutant matrix moved
from 138/0 to 181/3 on the worst of them. That number is the reviser's and I
have not reproduced it -- it needs a correct encoder to run against, which
arrives with the implementation, and I will check it then rather than assert it
now.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
src/columnar_curve.{c,h}: two pure functions over uint64 ordinals, neither
touching a PostgreSQL type beyond uint64, which is what lets the suite compile
this file against four-line stub headers and exercise the curve with no server
tree.

  cluster_hilbert_transpose  -- Skilling 2004 AxestoTranspose, b fixed at 64
  cluster_pack_interleave    -- the interleave loop, moved from cluster_zorder_key

Skilling's observation is that the Hilbert index IS the MSB-round-first
interleave of the transposed coordinates, bit for bit the packing Z-order
already used. So the curves differ by one in-place pass and share the packer,
the width and everything downstream:

  Z-order:  ordinals -> pack
  Hilbert:  ordinals -> transpose -> pack

test/hilbert_curve.sh: 184 passed, 0 failed, 0 unrunnable. It was 174 failed at
eb45776, where the arms were committed before this code existed.

REMOVAL PROOF, run here rather than inherited. Five mutants, each asserted to
have applied (the patch raises if its anchor is missing, and a mutation whose
md5 matches the original is refused as a no-op), each restored afterwards, with
the file verified back at bf5f7e07105f at the end:

  unmutated                    bf5f7e07105f   184 passed    0 failed
  M1 transpose is identity     916c58ad8bd9   133 passed   51 failed
  M2 no Gray encode            b9f72ca2c94d   133 passed   51 failed
  M3 no final t fold           9881723ebde0   145 passed   39 failed
  M4 invert/exchange swapped   a1deb7f2cef1   122 passed   62 failed
  M5 pack LSB-first in byte    57dacde80b1e    69 passed  115 failed

M1 is the one that matters: making the transpose a no-op turns this back into
Z-order, the curve that already ships, and 51 arms notice. That is the evidence
the battery distinguishes the new curve from the old one rather than merely
approving of it.

The 96 golden byte vectors passing is the strongest single signal, because they
could not have been generated from this file. They were derived from Skilling's
published algorithm and independently reproduced byte-for-byte by two reviewers
BEFORE src/columnar_curve.c existed. They are also the only arms that pin WHICH
Hilbert curve this is: Skilling's differs from Butz/Hamilton for ncols >= 3 and
both are valid, so the property arms cannot tell them apart and the bytes are an
on-disk format commitment.

The suite is now registered in run_all_versions.sh, which the red commit
deliberately did not do. harness_selftest: 261 passed, 0 failed, including "the
suite list is sorted in C order" -- note C order, not the locale's: plain `sort`
disagrees with the array and always has, so it is the wrong instrument for that
question.

Nothing calls the curve yet. Wiring it into the clustering paths, the gate and
the daemon is the next commit, per the owner ruling recorded on #889.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
Inert by construction and by measurement. No behaviour changes here; this is
the seam the Hilbert key needs.

  cluster_key_ordinals   extracted -- the type gate, the NULL rule and
                         cluster_type_ordinal now have one implementation
                         rather than one per curve
  cluster_zorder_key     keeps its name, its signature and its output, and
                         now calls cluster_pack_interleave for the interleave

The interleave moved to columnar_curve.c in 4557930 rather than being copied,
so there is no second copy to drift.

WHY THIS IS PROVED RATHER THAN ASSERTED. A refactor validated by "the tests
still pass" is validated against itself when the tests call the refactored code
on both sides. test/hilbert_curve.sh's 48 C7 arms hold cluster_pack_interleave
against hex literals captured from this loop BEFORE it moved, so the comparison
is new code against a frozen record. Those arms were committed at eb45776, when
columnar_curve.c did not exist and could not have generated them.

Measured with COPT=-Werror, PG18.4, build OK with 0 warnings:

  native_cluster       11 passed 0 failed      sorted_pathkeys      113 passed 0 failed
  native_recluster     12 passed 0 failed      sorted_mark_rename    46 passed 0 failed
  recluster_extent     17 passed 0 failed      vacuum_sorted_gate    34 passed 0 failed
  recluster_gate       13 passed 0 failed      hilbert_curve        184 passed 0 failed

Every count identical to the baseline taken on this branch before any change.

cluster_hilbert_key is deliberately NOT here. It was written, and removed again,
because a static function with no caller is -Werror=unused-function and the
build fails:

  src/columnar_vacuum.c:1104:1: error: 'cluster_hilbert_key' defined but not
  used [-Werror=unused-function]

So the key arrives in the same commit as the entry points that call it, rather
than sitting unreachable in the tree for a commit. That is a constraint worth
stating: on this project a new static helper cannot land ahead of its caller.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
The surface, the recorded kind, the self-gate, vacuum_sorted and the daemon.
181 checks: 98 passed, 74 failed, 9 unrunnable. Red, and red because
pgcolumnar.cluster_hilbert and pgcolumnar.recluster_hilbert do not exist -- 27
of the reds carry 42883 directly, and the rest are fixtures that could not be
built because of them.

WHAT THE ADVERSARIAL PASS CAUGHT, which is the reason this file is 1061 lines
and not 650. Three auditors built deliberately wrong implementations as SQL
shims and ran the suite against them. The flagship gate arm was green on a stub:

  shim C: recluster_hilbert is `BEGIN RETURN 0; END`, touching nothing
  -> S4(a) all three arms PASS, including "and the physical layout is
     byte-identical, SO THE GATE FIRED rather than the count being 0 for some
     other reason"

A stub returns 0 AND leaves the layout untouched, so physlayout was the same
observation twice rather than corroboration. The header stated the rule
correctly and the arm broke it. Fixed by making (a) carry its own positive
control: decay the table just gated, require the SAME call to return >0 and move
the layout, then require the gate to close again. That turns "nothing happened"
into "this verb can do work here and chose not to", which is what the gate
firing means.

  shim A: records 'hilbert', writes ZORDER bytes
  -> "(b) and the recorded kind became hilbert" PASS, beside two red siblings

A bare string assertion with no layout beside it. The kind arms are compound now.

THE 9 UNRUNNABLE ARE THE POINT, not a shortfall. Each is an arm that previously
passed on nothing and now says why it cannot be evaluated:

  UNRUN  (a) and the physical layout is byte-identical: UNMET_PRECONDITION: the
         call did not return 0 (returned []), so an unchanged layout would not
         be about the gate
  UNRUN  and the layout the daemon produced IS a Hilbert layout: [...] av_hi
         already matched the reference [...] so the comparison cannot be a change

That is #858's third state earning its keep in the first suite written after it
landed. Failure still outranks unrunnable, so the suite reports FAILED.

S7 drives the actual daemon rather than calling recluster() and reasoning about
it, per the ruling on #889. It asserts the launcher is running, that the daemon
was off beforehand and off again after, reads the daemon's own log line for
attribution, and freezes both hand-driven references before the window so the
comparison is a change rather than two untouched twins.

NOT REGISTERED, deliberately -- a red suite must not enter the matrix. The cost,
stated rather than discovered later: harness_selftest.sh fails its "every suite
is registered" arm while this file exists unregistered. Measured both ways: 260
passed + 1 failed with it present, 261 + 0 with it absent. This file is the sole
cause. It gets registered in the commit that lands the verbs.

WHAT NO ARM HERE CAN DO: reach the C encoder. Nothing in this suite proves the
bytes are Hilbert rather than merely "not the other two kinds" -- that is
test/hilbert_curve.sh's 184 arms, and the 96 golden vectors in particular. These
arms buy the surface, the gate, the kind and the daemon.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
Turns test/hilbert_cluster.sh from 98/74/9 to 181 passed, 0 failed, 0
unrunnable, without touching an arm. `git diff` over test/ carries no change to
either Hilbert suite (both byte-identical to 058b844) beyond the registration
line and the version-cycle work described below.

  pgcolumnar.cluster_hilbert(regclass, VARIADIC name[])    -> void
  pgcolumnar.recluster_hilbert(regclass, VARIADIC name[])  -> bigint

Separate verbs because PostgreSQL refuses to extend the existing signature in
either direction and an array-plus-kind overload breaks the documented
cluster('t','a','b') call style. Both measured on 18.4, recorded on #889.

THE CURVE IS STICKY, per the owner ruling. sorted_kind is the table's declared
intent, so plain recluster() on a Hilbert table with a matching key is a no-op
rather than a conversion, switching curves requires naming the other verb,
vacuum_sorted skips a Hilbert table rather than re-sorting it lexicographically,
and the daemon dispatches on the recorded kind.

The curve is threaded as one parameter through the key builder, the self-gate's
strcmp and the recorded extent, so the three cannot disagree. The eager worker
was renamed from _zorder to _curve: a function called _zorder that lays Hilbert
is the same silent-mislabel trap the kind literals were.

WHY THIS OPENS 1.0-alpha4 RATHER THAN ADDING TO ALPHA3. The first implementation
put both functions in pgcolumnar--1.0-alpha2--1.0-alpha3.sql. v1.0-alpha3 is a
PUBLISHED pre-release (2026-09-03), so that file is a shipped artifact: editing
it leaves a database that upgraded from the release and a fresh install both
reporting 1.0-alpha3 with different function sets and no path between them.
That is the failure extension versioning exists to prevent. So:

  VERSION and pgcolumnar.control       -> 1.0-alpha4
  pgcolumnar--1.0-alpha3.sql           -> pgcolumnar--1.0-alpha4.sql
  pgcolumnar--1.0-alpha3--1.0-alpha4.sql   new, carrying the two verbs
  pgcolumnar--1.0-alpha2--1.0-alpha3.sql   reverted, byte-identical to 058b844

The four documents that cite `VERSION` were updated with it, which docs_style
checks by comparing the quoted string against the file rather than trusting them
to be edited together.

AND THE NEWLY-PREVIOUS VERSION GAINS COVERAGE. test/native_upgrade_converge.sh
staged frozen base scripts for 1.0-alpha and 1.0-alpha2 only, so a database at
the version people actually have had no arm. The shipped alpha3 base script is
now frozen into test/fixtures, taken from the TAG rather than from this tree
(diff against `git show v1.0-alpha3:` is empty, and it contains neither new
verb), and both loops cover it. 8 checks -> 11.

The first edit added it to the staging loop alone and the count stayed at 8 --
a silent no-op, caught by watching the number rather than the exit status. The
arms come from a second loop.

REMOVAL PROOF on the upgrade path: renaming cluster_hilbert in the upgrade
script reddens 9 of 11 arms, including all three 1.0-alpha3 ones, with
"function pgcolumnar.cluster_hilbert(regclass, name[]) does not exist".
Restored and re-verified.

COPT=-Werror, PG18.4, build OK with 0 warnings:

  hilbert_cluster   181    sorted_pathkeys        113    harness_selftest  261
  hilbert_curve     184    sorted_mark_rename      46    entry_point_priv   31
  native_cluster     11    vacuum_sorted_gate      34    autovacuum         11
  native_recluster   12    native_upgrade_conv     11    docs_style      PASSED
  recluster_gate     13    recluster_extent        17
  all 0 failed, 0 unrunnable

KNOWN AND MEASURED, not inferred: the daemon's curve dispatch is NOT load-bearing
against this suite. Reverting it to the hard-coded recluster() leaves all 181
green, because the sticky plain recluster() already preserves the curve. It is
kept because the ruling is about the daemon and should be readable at the call
rather than inferred from a rule two modules away -- but no arm guards it, and
an arm that did would have to read the curve out of the daemon's log line, which
the log now carries.

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

@OffgridwithJD OffgridwithJD 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 at 4b665553. 12/12, and I read the breakdown rather than the colour: {"SUCCESS":12}, nothing neutral or skipped inside a zero-failure count.

I verified the load-bearing claims myself rather than reading the report, because this PR opens a release cycle and adds an external source category — the two places where "it passes" is the weakest evidence.

What I ran

The battery discriminates. This is the question that matters for a second curve: does it distinguish Hilbert from the Z-order that already ships, or would it approve whatever it is handed? I applied M1 myself — neuter cluster_hilbert_transpose to the identity, which turns the curve back into Z-order — and asserted the mutation applied rather than assuming:

baseline    184 passed +  0 failed
M1          133 passed + 51 failed
restored    byte-identical, mutation gone, tree clean

51 arms notice. That reproduces your number exactly and it is the strongest thing in the PR.

The four "the curve is sticky" claims in docs/sql-reference.md all hold, with the two controls that stop them being vacuous:

claim measured
plain cluster() on a Hilbert table maintains Hilbert hilberthilbert
plain recluster() likewise hilbert
cluster_hilbert on a Z-ordered table converts it zorderhilbert
vacuum_sorted leaves a Hilbert table alone hilbert
control: a Z-order table still reports zorder zorder — so sorted_kind is not a constant
control: the curves lay rows differently hilbert 15035,9021,12028,6014,18042 vs zorder 9021,15035,6014,18042,12028

That last one is the one I would keep. Same rows, different order, so the Hilbert path is not silently falling back.

The upgrade path. test/native_upgrade_converge.sh is modified in the same PR that adds a version, which is exactly where a vacuous convergence check would hide. It is right: 1.0-alpha3 is added to both the fixture list and the upgrade loop. The new fixture shows +0/-0, which is a rename, and its blob sha at your head is identical to pgcolumnar--1.0-alpha3.sql at the released v1.0-alpha3 tag (c7269cf1e242…). So the arm upgrades from what actually shipped, not from a working copy.

The provenance claim is accurate. columnar_curve.c says Skilling's AxestoTranspose is public domain. I checked the source rather than taking it: the listing carries an explicit PUBLIC-DOMAIN SOFTWARE notice. No licensing problem.

One request, procedural rather than blocking

PROVENANCE.md's implementer rules say to build only from the spec and the public PostgreSQL API, and its Log is where provenance decisions get recorded. This PR introduces a source category the Log has no precedent for — a published algorithm listing, transcribed with parameters fixed and the structure unchanged, by the file's own description.

The licensing is fine and I am not asking you to rewrite the function. I am asking for a Log entry, because the value of that document is that a future reader can tell why this was acceptable without re-deriving it. Right now that reasoning lives only in a source comment, and the file that exists to hold it is silent.

One question

av_maintain_one now reads sorted_kind by joining maintenance_due against sort_status in one query. sort_status is RETURNS record, not SETOF, so the join cannot go empty — I checked, since an empty right side would have made SPI_processed == 1 fail and silently skipped compaction too, not just the curve dispatch.

What remains is a coupling: the daemon's compaction now depends on sort_status succeeding, where before it depended only on maintenance_due. If there is a state where sort_status raises and maintenance_due would not, that table stops being maintained at all, quietly. Is there an arm for that, or is it unreachable for a relation the daemon iterates?

One observation that is NOT yours

The 1.0-alpha and 1.0-alpha2 fixtures do not match their released tags — v1.0-alpha shipped no pgcolumnar--1.0-alpha.sql at all, only pgcolumnar--1.0-dev.sql, and the 1.0-alpha2 fixture differs in both sha and size from the tag's. Both are byte-identical between main and your head, so this predates you and is not this PR's to answer. Say the word and I will file it; the convergence arms for those two versions may be upgrading from a reconstructed base rather than a released one.

On the daemon fix

a Hilbert table was reclustered by the Z-order verb and came back relabelled 'zorder' -- the daemon converting a layout the user chose, on a timer, with nothing in the log to say so

That is the best find in the PR and it is the kind that only shows up when you compose two features. Writing the dispatch out explicitly even though recluster() already preserves the curve is the right call for the reason you give: the ruling should be readable at the call it governs, not two modules away.

PROVENANCE.md's Log is where provenance decisions live, and #889 introduced a
source category it had no precedent for. The licensing reasoning existed only in
a comment in src/columnar_curve.c; the document that exists to hold it was
silent. (OffgridwithJD, #899 review.)

The entry records what was transcribed and from where, why the "build only from
the specification and the public PostgreSQL API" rule is not being bent -- that
rule exists to keep another COLUMNAR ENGINE's source out of this tree, which a
published public-domain algorithm from the literature is not, and the same
reasoning already covers pglz/lz4/zstd and the min/max skip list -- and what was
deliberately NOT done: no other Hilbert implementation was read, and the
correctness evidence is property-based rather than comparative.

The provenance itself was checked against Skilling's published listing by
OffgridwithJD during the #899 review. I have not obtained the paper and the
entry says so, rather than presenting their verification as a second one.

It also records the consequence a future reader needs: Hilbert curves are not
unique above two dimensions, this construction differs from Butz/Hamilton for
three or more columns, and both are valid -- so the key bytes are an on-disk
format commitment, a disagreement with another library is not a defect, and the
96 golden vectors are what pin which curve this is.

docs_style.sh: 9 checks, PASSED.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
@jdatcmd
jdatcmd merged commit 6da7c3f into main Sep 9, 2026
12 checks passed
@jdatcmd
jdatcmd deleted the feat/889-hilbert-clustering branch September 9, 2026 15:05
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 9, 2026
…essage

Four findings from @jdatcmd's review. The blocker first.

THE HARNESS REPORTED GREEN AGAINST SOURCE THAT CANNOT COMPILE. Demonstrated
with `#error THIS SOURCE IS BROKEN AND CANNOT BUILD` appended to
src/columnar_projection.c and nothing rebuilt: pytest said 25 passed, exit 0,
while test/native_projection.sh said FATAL and exited 1. I hit the same hole
independently from the other direction twenty minutes earlier -- a build from
another branch sat in the prefix, 43 passed, and the only failure was a
hardcoded version string noticing by accident.

My first fix compared the installed .control and .sql against the source. It
would not have caught the review's case at all, because appending #error to a
.c file leaves both artifacts byte-identical. It is deleted rather than kept.

The harness now builds and installs, and refuses to report if either fails. It
drives pgc_build_and_install, extracted UNCHANGED from pgc_setup in test/lib.sh,
so there is one implementation instead of two that drift -- and drift here would
be invisible in exactly the way the defect was. The extraction is inert: a
normal suite still builds, installs and passes 73 checks; the function still
refuses broken source with FATAL and non-zero; the unbroken control returns 0.

The pkglibdir race that made this harness skip installing is a reason to
serialise the install, not to skip it, so it runs once per session under a
flock. The marker is keyed on a SOURCE FINGERPRINT as well as the prefix:
keying on pg_config and major alone would skip the rebuild after a source edit,
reintroducing the staleness this guard exists to stop, through the optimisation
meant to make the guard cheap.

THE PORT BAND WAS FALSE IN BOTH HALVES, and it broke runs in this session before
the review arrived. ip_local_port_range is `32768 60999`, so PORT_BASE 54600 sat
INSIDE the ephemeral range; something holding a 546xx port produced 15
cluster-start errors. portlib.sh's bands are MAIN [10000, 29568) and AUX [29768,
31768). The harness now READS the floor and uses portlib.sh's own arithmetic,
drawing from AUX with a bind-test walk. The arm that asserted
`port == PORT_BASE + slot` now asserts the invariant the constant violated --
below the kernel's ephemeral floor -- instead of a fresh constant.

GUARDS THAT COULD BE DELETED WITH THE CORPUS STILL GREEN. Two causes needing one
remedy: guards no test drove at all, and guards a test drove while asserting only
the outcome, so a NEIGHBOURING guard's refusal satisfied the arm.
expect.refusal(result, name, *patterns) requires the message as well as the
failure, and refuses being called with no pattern so it cannot become the defect
it removes.

    census before   13 guards   3 HELD   10 UNHELD
    census after    13 guards  13 HELD    0 UNHELD

Three things writing those arms caught, all of them the point. refusal() refused
my own misuse when I passed a pattern into the name slot. hash("", "", ...)
never reaches the both-empty guard, because the guard above it is `got is want`
and CPython interns "" -- that arm would have passed while asserting nothing
about the guard it names. And the two error-sentinel guards stayed unheld until
the arms named which SIDE was refused, because the ordinary comparison failure
satisfied them otherwise.

The extension-version arm no longer hardcodes a version. It reads
pgcolumnar.control, which is durable across a release cycle -- commandprompt#899 moved the
tree to 1.0-alpha4 today -- and is the assertion the arm is named after: that
the loaded extension is the one this source describes.

50 passed, exit 0, serial and under -n 4, exit codes read without a pipe.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 9, 2026
…essage

Four findings from @jdatcmd's review. The blocker first.

THE HARNESS REPORTED GREEN AGAINST SOURCE THAT CANNOT COMPILE. Demonstrated
with `#error THIS SOURCE IS BROKEN AND CANNOT BUILD` appended to
src/columnar_projection.c and nothing rebuilt: pytest said 25 passed, exit 0,
while test/native_projection.sh said FATAL and exited 1. I hit the same hole
independently from the other direction twenty minutes earlier -- a build from
another branch sat in the prefix, 43 passed, and the only failure was a
hardcoded version string noticing by accident.

My first fix compared the installed .control and .sql against the source. It
would not have caught the review's case at all, because appending #error to a
.c file leaves both artifacts byte-identical. It is deleted rather than kept.

The harness now builds and installs, and refuses to report if either fails. It
drives pgc_build_and_install, extracted UNCHANGED from pgc_setup in test/lib.sh,
so there is one implementation instead of two that drift -- and drift here would
be invisible in exactly the way the defect was. The extraction is inert: a
normal suite still builds, installs and passes 73 checks; the function still
refuses broken source with FATAL and non-zero; the unbroken control returns 0.

The pkglibdir race that made this harness skip installing is a reason to
serialise the install, not to skip it, so it runs once per session under a
flock. The marker is keyed on a SOURCE FINGERPRINT as well as the prefix:
keying on pg_config and major alone would skip the rebuild after a source edit,
reintroducing the staleness this guard exists to stop, through the optimisation
meant to make the guard cheap.

THE PORT BAND WAS FALSE IN BOTH HALVES, and it broke runs in this session before
the review arrived. ip_local_port_range is `32768 60999`, so PORT_BASE 54600 sat
INSIDE the ephemeral range; something holding a 546xx port produced 15
cluster-start errors. portlib.sh's bands are MAIN [10000, 29568) and AUX [29768,
31768). The harness now READS the floor and uses portlib.sh's own arithmetic,
drawing from AUX with a bind-test walk. The arm that asserted
`port == PORT_BASE + slot` now asserts the invariant the constant violated --
below the kernel's ephemeral floor -- instead of a fresh constant.

GUARDS THAT COULD BE DELETED WITH THE CORPUS STILL GREEN. Two causes needing one
remedy: guards no test drove at all, and guards a test drove while asserting only
the outcome, so a NEIGHBOURING guard's refusal satisfied the arm.
expect.refusal(result, name, *patterns) requires the message as well as the
failure, and refuses being called with no pattern so it cannot become the defect
it removes.

    census before   13 guards   3 HELD   10 UNHELD
    census after    13 guards  13 HELD    0 UNHELD

Three things writing those arms caught, all of them the point. refusal() refused
my own misuse when I passed a pattern into the name slot. hash("", "", ...)
never reaches the both-empty guard, because the guard above it is `got is want`
and CPython interns "" -- that arm would have passed while asserting nothing
about the guard it names. And the two error-sentinel guards stayed unheld until
the arms named which SIDE was refused, because the ordinary comparison failure
satisfied them otherwise.

The extension-version arm no longer hardcodes a version. It reads
pgcolumnar.control, which is durable across a release cycle -- commandprompt#899 moved the
tree to 1.0-alpha4 today -- and is the assertion the arm is named after: that
the loaded extension is the one this source describes.

50 passed, exit 0, serial and under -n 4, exit codes read without a pipe.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 9, 2026
…essage

Four findings from @jdatcmd's review. The blocker first.

THE HARNESS REPORTED GREEN AGAINST SOURCE THAT CANNOT COMPILE. Demonstrated
with `#error THIS SOURCE IS BROKEN AND CANNOT BUILD` appended to
src/columnar_projection.c and nothing rebuilt: pytest said 25 passed, exit 0,
while test/native_projection.sh said FATAL and exited 1. I hit the same hole
independently from the other direction twenty minutes earlier -- a build from
another branch sat in the prefix, 43 passed, and the only failure was a
hardcoded version string noticing by accident.

My first fix compared the installed .control and .sql against the source. It
would not have caught the review's case at all, because appending #error to a
.c file leaves both artifacts byte-identical. It is deleted rather than kept.

The harness now builds and installs, and refuses to report if either fails. It
drives pgc_build_and_install, extracted UNCHANGED from pgc_setup in test/lib.sh,
so there is one implementation instead of two that drift -- and drift here would
be invisible in exactly the way the defect was. The extraction is inert: a
normal suite still builds, installs and passes 73 checks; the function still
refuses broken source with FATAL and non-zero; the unbroken control returns 0.

The pkglibdir race that made this harness skip installing is a reason to
serialise the install, not to skip it, so it runs once per session under a
flock. The marker is keyed on a SOURCE FINGERPRINT as well as the prefix:
keying on pg_config and major alone would skip the rebuild after a source edit,
reintroducing the staleness this guard exists to stop, through the optimisation
meant to make the guard cheap.

THE PORT BAND WAS FALSE IN BOTH HALVES, and it broke runs in this session before
the review arrived. ip_local_port_range is `32768 60999`, so PORT_BASE 54600 sat
INSIDE the ephemeral range; something holding a 546xx port produced 15
cluster-start errors. portlib.sh's bands are MAIN [10000, 29568) and AUX [29768,
31768). The harness now READS the floor and uses portlib.sh's own arithmetic,
drawing from AUX with a bind-test walk. The arm that asserted
`port == PORT_BASE + slot` now asserts the invariant the constant violated --
below the kernel's ephemeral floor -- instead of a fresh constant.

GUARDS THAT COULD BE DELETED WITH THE CORPUS STILL GREEN. Two causes needing one
remedy: guards no test drove at all, and guards a test drove while asserting only
the outcome, so a NEIGHBOURING guard's refusal satisfied the arm.
expect.refusal(result, name, *patterns) requires the message as well as the
failure, and refuses being called with no pattern so it cannot become the defect
it removes.

    census before   13 guards   3 HELD   10 UNHELD
    census after    13 guards  13 HELD    0 UNHELD

Three things writing those arms caught, all of them the point. refusal() refused
my own misuse when I passed a pattern into the name slot. hash("", "", ...)
never reaches the both-empty guard, because the guard above it is `got is want`
and CPython interns "" -- that arm would have passed while asserting nothing
about the guard it names. And the two error-sentinel guards stayed unheld until
the arms named which SIDE was refused, because the ordinary comparison failure
satisfied them otherwise.

The extension-version arm no longer hardcodes a version. It reads
pgcolumnar.control, which is durable across a release cycle -- commandprompt#899 moved the
tree to 1.0-alpha4 today -- and is the assertion the arm is named after: that
the loaded extension is the one this source describes.

50 passed, exit 0, serial and under -n 4, exit codes read without a pipe.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
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.

Hilbert curve clustering: a second key kind for cluster and recluster (1.0-alpha4)

2 participants