docs: GIN and BRIN cannot be chosen, not merely have not been seen (#1143) - #1206
Conversation
…ommandprompt#1143) The limitations page left GIN as an open question and offered the fixture size as an explanation: "those settings are cost penalties rather than prohibitions". That is true in general and it is not what happens here. Both are bitmap-only access methods. A GIN index has no amgettuple at all, so the only path either can produce is a bitmap index scan feeding a bitmap heap scan, and the columnar table access method implements no bitmap-scan callback. The planner generates no such path, so cost never enters into it. MEASURED WITH ONE VARIABLE. Same 200,000 rows, same two indexes, same query, same settings, and the table access method as the only difference: heap GIN -> Bitmap Heap Scan + Bitmap Index Scan columnar GIN -> Seq Scan heap BRIN -> Bitmap Heap Scan + Bitmap Index Scan columnar BRIN -> Seq Scan same answers on both storages On PostgreSQL 18 the columnar plan is reported "Disabled: true" under enable_seqscan=off. The planner used a node it had been told not to use, because the pathlist held no alternative -- which is a stronger statement than the seq scan winning on cost, and one no row count can change. BRIN is settled by the same measurement; commandprompt#1143 records it as never having been probed past the build. THE CODE COMMENT IS NOT DECORATION. The callback set differs by major -- 15 to 17 declare scan_bitmap_next_block and scan_bitmap_next_tuple, 18 removed the former in the read-stream rework -- so "implement the two callbacks" is wrong advice on two of the five majors. And table_scan_bitmap_* guards only against logical decoding before calling through the pointer, so a NULL member is a null function-pointer call in the executor rather than an ereport. A half-implementation crashes. Both read from each installed server own tableam.h by @jdatcmd and verified here against all five. The features.md link moves with the heading. The bullet index_am_support.sh reads out of that page is untouched, and the suite still passes 20 + 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP
…commandprompt#1143) A columnar insert touches very few buffers, so any index maintenance is a large multiple of it. Measured on 100,000 rows, shared buffer hits on the INSERT: columnar no index 202 with BRIN 37,484 heap no index 101,468 with BRIN 118,923 The index is the same size on both storages -- 24,576 bytes for BRIN and 5,726,208 for GIN -- so identical bytes on disk buy a usable index on one access method and an unusable one on the other. Measured independently by @jdatcmd, whose columnar delta of 36,968 agrees with the 37,282 here to within 0.8% on a different box. Their heap delta was 6,031 against 17,455 here, so the heap side is not stable between boxes and no RATIO between the two is stated; only the columnar figures, which reproduce. TWO CLAIMS DELIBERATELY LEFT OUT. A multiplier expressed as "N times heap", because the heap term does not reproduce. And "brin_summarize_new_values errors on a columnar table" -- reported by @jdatcmd, and on this box the same call returns 0 rather than raising, with the index created before the rows so the unsummarized state exists. One of the two conditions differs and it is not reconciled, so it is not in a user-facing page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP
…eachable (commandprompt#1143) Two readings that looked contradictory and are not. `brin_summarize_new_values` returns 0 on a columnar table where heap returns a count. `brin_summarize_range` on a range that has work raises: SELECT brin_summarize_range(cr_brin, 2); ERROR: columnar: partial-range index build is not supported A 0 is not evidence that summarization works. It means BRIN found no range to summarize and never called into the access method, so it is silence rather than success. The error is what the path does when it is actually reached. @jdatcmd found the guard in index_build_range_scan and reported the ERROR; I could not reproduce it and reported the 0. Neither of us was wrong and neither reading settled it. The condition is reproduced here by asking for a range that HAS work: ranges 0, 1 and 5 return 0 on the same index and the same table, range 2 raises. That is the fourth property of a BRIN index on a columnar table, after building, never being chosen, and costing real work on every insert. The index stays at its initial size because nothing ever summarizes into it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP
…ss (commandprompt#1143) A range with work refuses ONCE. The next call on the same range returns 0 and the index never grows. Sweeping ranges 0 to 6 three times on one index: pass 1 0 0 E 0 0 0 0 index 24,576 bytes pass 2 0 0 0 0 0 0 0 index 24,576 bytes pass 3 0 0 0 0 0 0 0 index 24,576 bytes So a user who runs the documented maintenance function, sees an error, runs it again and sees 0 is told it was repaired. Nothing was summarized and the index is still empty. The page now says "only once" and shows the sweep, because the previous wording -- "the error appears only for a range that has work" -- is true and would let exactly that reader draw the wrong conclusion. IT ALSO EXPLAINS WHY NEITHER SESSION COULD REPRODUCE THE OTHER. @jdatcmd saw the ERROR and then could not get it again; I saw 0 and could not get the ERROR. Neither box changed and neither fixture was wrong. We were at different points in the same consumption sequence, and re-running the same call is precisely the thing that cannot distinguish them. Found by @jdatcmd sweeping a 526-page table; reproduced here on a 3-page one, which is why their pass showed five refusals and this one shows a single E. The mechanism is NOT claimed. The behaviour is consistent with the placeholder BRIN writes before the range scan marking the range, but neither of us has read that code or instrumented it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP
jdatcmd
left a comment
There was a problem hiding this comment.
Approving f42b05e. You will not approve your own, and this needs a non-author approval to move, so here it is with what I checked.
I nearly opened this review with a catastrophic false finding
My first look diffed origin/main against the branch two dots, and it showed #1206 deleting the entire parallel covering-projection block and reverting projScale to scale -- #1127's whole change, backed out.
It does no such thing. The branch's merge base is fa576236, which predates #1127, so a two-dot diff renders the other lane's additions as this branch's deletions. Three dots against the merge base:
54 0 CHANGELOG.md
1 1 docs/features.md
61 17 docs/limitations.md
17 0 src/columnar_tableam.c
src/columnar_customscan.c is not touched at all. I am recording it because the false reading was completely convincing and the correct instrument is one character different -- and because this repository already has a note about a scope argument feeding the analysis rather than the report.
The page
The structural claim is right and it is stated as a structural claim, which is what the issue needed: no bitmap-scan callback, therefore no bitmap path, therefore not a cost the row count can change. Disabled: true is the sharpest evidence in the whole issue and it is in the page.
The BRIN discriminator is the sentence I most wanted and it is exact:
Ranges with nothing to summarize return 0, so the error appears only for a range that has work. It also appears only once. ... Do not read that 0 as a repair.
Without "only once" a reader checks twice, gets 0, and concludes it was fixed.
The table's flat claim is very slightly stronger than my own measurement
The index is also the same size on both storages, at 24,576 bytes for BRIN and 5,726,208 for GIN.
BRIN was 24,576 on both for me too. GIN was not exactly equal on my box:
cx_gin_v 11,091,968 hx_gin_v 11,149,312 57,344 bytes apart, 0.5%
Different fixture, different absolute size, and the point survives entirely -- the work is real and it is the same work. But "the same size on both storages" reads as an identity, and two independent measurements give an identity on one box and 0.5% on another. "Essentially the same size" or a note that the fixture differs would hold on both.
Not blocking, and your number is right for your fixture.
The comment in columnar_tableam.c
Accurate on both readings and it is the right place for them: the callback set differs across majors (15-17 declare two, 18 removed scan_bitmap_next_block), and a half-implementation is a null function-pointer call rather than an ERROR. Someone who takes "builds but is never chosen" as an invitation needs both before they start.
What I did not re-measure
Your write-cost numbers (202 / 37,484 columnar, 101,468 / 118,923 heap) are yours, on your box, and mine differ on the base. The deltas agreed to within 1% when we compared, which is why the page's proportion argument is the one that carries -- and the page makes that argument rather than the ratio one, which is correct, since we established the heap side is not stable between boxes.
15 of 15, CLEAN.
…ndprompt#1143) The page and the changelog both said the index is "the same size on both storages" and gave 24,576 bytes for BRIN and 5,726,208 for GIN. That is what this fixture measured, and the flat identity is stronger than one fixture supports. @jdatcmd ran it at a different row count and got BRIN equal on both storages again, but GIN 11,091,968 against 11,149,312 -- about 0.5% apart. The point the paragraph makes survives completely: the storage does not change what the index costs. What does not survive is "the same size" read as byte-for-byte. Both places now say "essentially the same size", keep the measured numbers, and name them as this fixture's rather than as a property. Fixed in the changelog as well as the page, because the claim appeared twice and correcting only the one I was shown would leave the other to be rediscovered. docs_style.sh: 55 checks, PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP
jdatcmd
left a comment
There was a problem hiding this comment.
The correction is right and it introduces one wrong word, in the clause that explains the discrepancy. Holding for that single edit; everything else on 2e29436 is an improvement on what I approved.
The wrong variable
Both versions now say the second measurement came from a different row count:
about 0.5% apart on a second run at a different row count
A second run at a different row count put the two GIN indexes about 0.5% apart
Both measurements were at 100,000 rows. Yours, from the page itself:
Measured on 100,000 rows, shared buffer hits on the
INSERTitself
Mine, from the script I ran:
INSERT INTO cx_gin SELECT g, g % 1000,
ARRAY[md5(g::text), md5((g/7)::text), md5((g/13)::text)]
FROM generate_series(1,100000) g;Same row count. What differs is the fixture: three md5s in a text[] against whatever yours indexes, which is also why the absolute sizes are 11 MB against 5.7 MB.
It matters because the clause exists to tell a reader why two measurements of the same property disagree, and it points them at the one variable that was held constant. A reader who trusts it concludes GIN size varies with row count between storages -- which may be true and is not what either of us measured.
"a second run on a different fixture" is the fact, and it is the same number of words.
Otherwise this is better than the head I approved
You fixed it in both places. I only showed you the page; the CHANGELOG carried the same flat identity and I had not looked. Correcting the instance I pointed at and leaving the other to be rediscovered is the shape that cost an afternoon on #1146 this morning.
And the replacement is stronger than the phrase I suggested. I proposed "essentially the same size"; you wrote
Read that as "the storage does not change what the index costs", not as a byte-for-byte identity.
which says what the measurement supports instead of hedging the word that did not. The numbers stay, and they are now labelled as this fixture's.
Unchanged from my earlier review
The structural claim, Disabled: true, the BRIN "only once" discriminator, and the columnar_tableam.c comment on the per-major callback set and the null-pointer crash. All still right, and the three-dot diff still says columnar_customscan.c is untouched.
Re-approving on the next head with that one clause fixed.
…ndprompt#1143) My previous commit explained the 0.5% discrepancy as "a second run at a different row count". Both runs were at 100,000 rows. What differs is the fixture: @jdatcmd indexed a text[] of three md5s per row, which is also why the absolute sizes are 11 MB against 5.7 MB rather than close. The clause exists to tell a reader WHY two measurements of the same property disagree, and it was pointing at the one variable that was held constant. Same number of words, and now a fact. Corrected in both places again, for the same reason as last time. docs_style.sh: 55 checks, PASSED. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP
jdatcmd
left a comment
There was a problem hiding this comment.
Approving 33901ea. The clause is fixed in both places and I checked the fix rather than the diff.
- second run at a different row count.
+ second run on a different fixture.
Both instances. And saying nothing about row counts in the replacement is better than my suggestion would have been -- you had no second row count to name, and naming one would have been the same error in the other direction.
My own grep then lied to me while I was confirming it. grep -c "different row count" on the CHANGELOG came back 1 after the fix, which read as a missed instance. It is an unrelated entry from an earlier change, about 0.92 (rep) and 0.92 (mix) being shape names from a three-shape run "with a different row count and wall-clock rather than instruction counts". Caught by looking instead of believing the count -- which is the same discipline this whole paragraph is about, arriving one level down for the third time today.
What I re-checked on this head
| the structural claim | unchanged: no bitmap callback, therefore no bitmap path, therefore not a cost the row count can change |
Disabled: true |
still there, and still the sharpest evidence in #1143 |
| the BRIN discriminator | "It also appears only once" and "Do not read that 0 as a repair" both intact |
the columnar_tableam.c comment |
per-major callback set and the null-pointer crash, both accurate |
| three-dot diff against the merge base | 54/1/61/17 across four files; columnar_customscan.c still untouched |
15 of 15, CLEAN. Merging on this head.
Closes #1143.
The limitations page left GIN as an open question and offered the fixture as the explanation — "those settings are cost penalties rather than prohibitions". That is true in general and it is not what happens here.
Neither index can ever be chosen
Both are bitmap-only access methods. A GIN index has no
amgettupleat all, so the only path either can produce is a bitmap index scan feeding a bitmap heap scan. The columnar table access method implements no bitmap-scan callback, so the planner generates no such path and cost never enters into it.Measured with one variable
Same 200,000 rows, same two indexes, same query, same settings (
enable_seqscan,enable_indexscan,enable_indexonlyscanall off). The table access method is the only difference:Both storages return the same rows, so correctness is not the variable.
On PostgreSQL 18 the columnar plan is reported with
Disabled: true. That is the planner saying it used a node it had been told not to use, because the pathlist held no alternative — a stronger statement than the sequential scan winning on cost, and one no row count can change. It is also what settles the issue's own hypothesis that a 20,000-row fixture was too small.BRIN is settled by the same measurement. #1143 records it as never having been probed past the build.
The code comment is not decoration
Two things make "just implement the two callbacks" wrong advice, both found by @jdatcmd and verified here against all five installed servers:
The callback set differs by major. Read from each server's own
tableam.h:scan_bitmap_next_blockwas removed in 18's read-stream rework, so a sentence naming two callbacks is wrong on two of five majors. The comment says "every callback the major defines" instead.A half-implementation crashes rather than errors.
table_scan_bitmap_next_tupleguards only against logical decoding and then calls straight through the pointer:A NULL member is a null function-pointer call in the executor. So the planner generating no bitmap path is not merely why the index is unused — it is what stands between the current state and a segfault, and anyone implementing this halfway will find out by crashing.
Scope
features.mdlink moves with the heading. The bulletindex_am_support.shreads out of that page is untouched, and the suite still passes20 passed + 0 failed.docs_style.sh55 checks, PASSED.One sentence I did not write: that such an index is dead weight on write. It follows, I have not measured it, and an unmeasured sentence in a user-facing page is worse than a missing one.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP