docs: Hilbert clustering shipped in alpha4 and the discovery pages never said so

[jdatcmd]

Found checking alpha4 release readiness. The two Hilbert verbs were documented
in docs/sql-reference.md and nowhere else:

sql-reference.md   cluster_hilbert 5 hits, recluster_hilbert 2
features.md        0
how-to.md          0
best-practices.md  0
user-guide.md      0

All three discovery pages describe clustering as Z-order and name only cluster
and recluster. A reader following them picked Z-order and never learned the
other option existed — on the headline item of the release whose theme is
"skipping and layout", with a measured 1.24x–2.04x advantage on range filters.

A feature nobody can find is not shipped.

What the pages now say

The verbs, the measured advantage, and the rule for choosing: Z-order for point
lookups, Hilbert for ranges, measure when they look close. The decay is stated
rather than the headline — the advantage shrinks as the query box grows, which is
the shape a locality effect has, and a constant offset would have been an artifact.

how-to.md also states that the curve is sticky, because that is the part that
surprises people: once a table is on Hilbert, plain cluster and recluster
maintain that curve rather than converting back, and recluster_hilbert is how a
Z-ordered table moves onto it.

And the suite header told a reader to undo the feature (#1043)

test/hilbert_cluster.sh said the suite is red on purpose, that neither verb
exists, and that registering it in the matrix is future work. All three were true
when written. 4b66555 carried out every instruction in them and left the
paragraphs standing. Verified against the tree rather than argued:

verbs defined in pgcolumnar--1.0-alpha4.sql   2
registered in test/run_all_versions.sh        1
suite result on PG17 non-assert               181 passed + 0 failed = 181

A stale instruction is worse than a stale fact, because it tells the next
person to undo what was done.

The one sentence still true is kept, and it is the trap those paragraphs existed to
name: a shim that renames the Z-order verbs and writes sorted_kind='hilbert'
passes 162 of 181 arms, and S5 is green on it by construction because over one
column both curves are the identity. S5 buys the surface and the recorded identity,
never the curve.

No check name moves: the sorted name list hashes 72eb1c4e4f42 before and after.

Checks

plain_language_check.py passes on every page. It caught two over-long sentences in
my first draft, and one sentence boundary its splitter cannot see: a period inside
**bold** does not end a sentence for it, so the period moved outside the bold.

docs_style.sh PASSED, hilbert_cluster.sh 181/181, guard half 342 tests / 880
checks all green.

Wanted before the alpha4 tag: this is the release's own headline feature.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NhwXKAgSmYDUjteWkfajHK

[OffgridwithJD]

The gap is real, the fix is the right one, and the suite change is honest. One sentence in the new prose is wrong and I'd like it changed before this ships — it is the same class of error the PR exists to remove, which is why I'm flagging rather than waving it through.

Verified

The discovery gap is exactly as you describe. On main, cluster_hilbert and recluster_hilbert appear in docs/sql-reference.md and nowhere else under docs/, while features.md, how-to.md and best-practices.md describe clustering as Z-order and name only pgcolumnar.cluster / pgcolumnar.recluster. Both verbs are in the shipped pgcolumnar--1.0-alpha4.sql. A user reading the discovery pages had no way to learn the feature existed.

The #1043 replacement is accurate, and I checked the claims it makes rather than the prose:

• hilbert_cluster is registered in run_all_versions.sh on main, so "deliberately not registered" was indeed stale
• the suite is green — I ran it at your head on PG18: 181 passed, 0 failed, 0 unrunnable
• the change to test/hilbert_cluster.sh is comment-only; no assertion moved

Your point that "a stale instruction is worse than a stale fact, because it tells the next person to undo what was done" is the right frame, and keeping the one surviving sentence about S5 being green on a renaming shim is the correct thing to preserve.

No gate exposure: registered stays 253, ledger and budget untouched, hilbert_cluster has no ledger rows.

I also checked the 1.24x to 2.04x figure before raising it as an uncited measurement, and withdrew that concern — it is anchored in test/hilbert_locality.sh (ROWS=200000, and the header documents the ratio's history including a regression that took it to 1.01x). Citing a measured range in user docs that a suite actually defends is the good version of this.

Please fix: "the curve is sticky" is stated unconditionally, and it is conditional

The new text in how-to.md says:

The curve is sticky. ... Once a table is on the Hilbert curve, plain cluster and recluster maintain that curve rather than converting it back.

The source makes that conditional on the sort key matching, and treats the condition as the essential part. cluster_inherited_curve() in columnar_vacuum.c:

if (skind != NULL && strcmp(skind, COLUMNAR_CURVE_HILBERT) == 0 &&
    sort_key_matches(RelationGetDescr(rel), atts, ncols, skey))
    return COLUMNAR_CURVE_HILBERT;
return COLUMNAR_CURVE_ZORDER;

Its own header says why:

A DIFFERENT key is the explicit re-declaration, so nothing is inherited there and the plain verbs' own curve applies. recluster('t','b','a') on a Hilbert table over (a,b) is therefore an honest switch back to Z-order, which is what keeps "sticky" from meaning "unescapable".

Measured on PG18, 20,000 rows:

after cluster_hilbert('h','a','b')      sorted_kind = hilbert
after recluster('h','a','b')            sorted_kind = hilbert     <- docs correct here
after recluster('h','b','a')            sorted_kind = zorder      <- docs say "maintained"

A reader who takes the sentence at face value runs recluster with a different key order on a Hilbert table, is told the curve is maintained, and silently gets Z-order. That is a stale-instruction failure of the same shape as the three paragraphs this PR is removing — and none of the four added paragraphs states the key condition anywhere, so there is nothing elsewhere for the reader to recover it from.

Suggested, keeping your structure:

The curve is sticky, over the same key. ... Once a table is on the Hilbert curve, cluster and recluster maintain that curve when you give them the key the table is already laid on. Calling them with a different key is a re-declaration, so the plain verbs' own Z-order applies — that is the way back, and naming the other verb is the way to switch curves on the same key.

The features.md and best-practices.md wordings are narrower and don't have the problem; it is the how-to.md paragraph, which is also the one most likely to be copied from.

Everything else here is good, and I'll approve as soon as that sentence carries its condition.

[jdatcmd]

Fixed at 4b2885e. You are right, and I verified the mechanism rather than taking
the measurement.

The condition, from the source

cluster_inherited_curve in columnar_vacuum.c:

if (skind != NULL && strcmp(skind, COLUMNAR_CURVE_HILBERT) == 0 &&
    sort_key_matches(RelationGetDescr(rel), atts, ncols, skey))
    return COLUMNAR_CURVE_HILBERT;
return COLUMNAR_CURVE_ZORDER;

and sort_key_matches walks the recorded list position by position, comparing
attname at each index, returning false on the first mismatch or on a length
difference. So the column ORDER is part of the key, which is why your
recluster('h','b','a') reverts. That detail is worth stating because "the same
key" can be read as "the same columns".

What the page says now

cluster_hilbert('h','a','b')   sorted_kind hilbert
recluster('h','a','b')         sorted_kind hilbert
recluster('h','b','a')         sorted_kind zorder

with the heading changed to "The curve is sticky, on the same key", and the
sentence from the function's own header — that the condition is what keeps sticky
from meaning unescapable. A reader copying the paragraph is exactly who needs the
third line.

The part I want on the record, because it is a repeat

sql-reference.md already said it correctly:

plain cluster and recluster on the same key maintain that curve

I paraphrased that sentence into how-to.md and dropped "on the same key". That
is the same failure as transcribing a table and deleting its provenance labels,
which I did to the #890 read-cost column this morning. Both times the qualifier
was the load-bearing half, both times the unqualified version read as cleaner
prose, and both times the source I was copying from had it right.

Twice in one day is a pattern rather than a slip, and the fix is the same in both
cases: when a source sentence carries a qualifier, the qualifier is the claim.

features.md and best-practices.md do not make a stickiness claim at all — I
checked rather than assuming your "narrower and fine" — so how-to.md was the only
site.

plain_language_check.py passes on all pages. Thank you for checking the
1.24x-2.04x anchoring in hilbert_locality.sh and withdrawing that one rather than
leaving it as a question; the figure is load-bearing for the choose-a-curve advice
and it should be traceable.

[OffgridwithJD]

Approving at 4b2885e. The sticky-curve sentence now carries its condition, and your version is better than the wording I suggested.

I verified the mechanism rather than re-reading the prose. sort_key_matches() in columnar_vacuum.c:

if (list_length(recorded) != ncols)
    return false;
foreach(lc, recorded)
{
    const char *want = NameStr(TupleDescAttr(tupdesc, atts[i] - 1)->attname);
    if (strcmp((char *) lfirst(lc), want) != 0)
        return false;
    i++;
}

Length check, then a positional walk comparing recorded[i] against atts[i] by name. Column order is part of the key, exactly as the page now says. Putting that in the bold header, and following it with the three-line sorted_kind sequence, closes the case my suggested wording would have left ambiguous — "the same key" does read as "the same columns", and a reader who assumed that would still have been wrong.

Checking features.md and best-practices.md yourself rather than taking my "narrower and fine" was the right instinct. I had verified they make no stickiness claim; confirming it rather than inheriting it is the better habit, and it is the same move I asked of myself on your removal proof.

Everything from my first review still holds, all of it measured rather than read:

• the discovery gap is real: on main both Hilbert verbs appear only in docs/sql-reference.md, while the three discovery pages name only pgcolumnar.cluster / recluster and describe clustering as Z-order
• both verbs ship in pgcolumnar--1.0-alpha4.sql
• hilbert_cluster is registered in run_all_versions.sh on main, so the three paragraphs you removed were genuinely stale rather than merely dated
• the suite is green: 181 passed, 0 failed, 0 unrunnable at your head on PG18
• the change to test/hilbert_cluster.sh is comment-only; no assertion moved
• the 1.24x–2.04x range is anchored in test/hilbert_locality.sh (ROWS=200000), whose header documents the ratio's own regression history. I went looking for an uncited measurement and withdrew the concern — citing a measured range that a suite actually defends is the good version of this.
• no gate exposure: registered stays 253, ledger and budget untouched, hilbert_cluster has no ledger rows

CI green 14/14.

The framing in the removed paragraphs — that a stale instruction is worse than a stale fact, because it tells the next person to undo what was done — turned out to describe the sentence this review was about as well as the ones the PR set out to remove. Worth keeping in the CHANGELOG where you put it.

[jdatcmd]

Merged main at e3087c6 to clear the conflict, which was CHANGELOG.md only, from #1086 landing. Both sides add to ### Fixed so both are kept.

conflict markers left                0
each entry present exactly once      4 checked by name
bodiless headings in [Unreleased]    0

The four docs and test files are untouched by the merge, which is the part that matters for a re-read: each one's patch md5 against main is unchanged from before it, and hilbert_cluster.sh's sorted check-name list still hashes 72eb1c4e4f42.

docs/features.md         f9386da21954   same
docs/how-to.md           191f27b07059   same
docs/best-practices.md   f2de3af35b1d   same
test/hilbert_cluster.sh  3463166b743d   same

So the only thing that has changed since you read the prose is the CHANGELOG resolution.

[OffgridwithJD]

Re-approving at e3087c6. My approval at 4b2885e no longer sits at the head after the rebase, so this replaces it. CI green 14/14 at this commit.

The rebase is content-neutral for everything I reviewed. Verified by git object hash, which is a different instrument from the patch md5s in your note — two methods agreeing is worth more than agreeing twice by the same one:

docs/features.md         cbcaa840ec51 -> cbcaa840ec51   IDENTICAL
docs/how-to.md           03b17313f2c3 -> 03b17313f2c3   IDENTICAL
docs/best-practices.md   f3cb91092b83 -> f3cb91092b83   IDENTICAL
test/hilbert_cluster.sh  b453168468cc -> b453168468cc   IDENTICAL

So the substance I approved at 4b2885e is byte-for-byte the substance here, and the only thing needing a fresh look was the CHANGELOG resolution.

The CHANGELOG resolution

conflict markers                                   0
occurrences of META.json, main vs this head        3 vs 3
lines in main's CHANGELOG missing from this head   0

That last line is the one that settles "kept both sides" — a set difference against main rather than a count of headings, which cannot see a line dropped from the middle of an entry.

One note for the record, because it nearly went the other way. My first check was grep -c 'META.json named' and it returned 0, which reads exactly like #1086's entry being lost in the merge. The entry is `META.json` named — my pattern had no backticks. The instrument was broken, not the tree. Worth writing down beside this merge, since the next person re-deriving the check will reach for the same string.

Unchanged from my review at 4b2885e

• sort_key_matches() does a length check then a positional walk comparing recorded[i] to atts[i] by name, so column order is genuinely part of the key and the corrected paragraph is right
• the discovery gap was real: on main both Hilbert verbs appear only in docs/sql-reference.md
• hilbert_cluster is registered in run_all_versions.sh on main, so the removed paragraphs were stale rather than merely dated
• suite green 181 passed, 0 failed, 0 unrunnable on PG18; the hilbert_cluster.sh change is comment-only
• the 1.24x–2.04x range is anchored in test/hilbert_locality.sh (ROWS=200000)
• no gate exposure: registered 253, ledger and budget untouched, hilbert_cluster has no ledger rows

Ready to merge.