| From: | "Greg Burd" <greg(at)burd(dot)me> |
|---|---|
| To: | "Rui Zhao" <zhaorui126(at)gmail(dot)com>, "Michael Paquier" <michael(at)paquier(dot)xyz> |
| Cc: | "Bharath Rupireddy" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, "Yugo Nagata" <nagata(at)sraoss(dot)co(dot)jp>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Support for 8-byte TOAST values, round two |
| Date: | 2026-09-15 17:05:09 |
| Message-ID: | 8ba57b65-6054-4287-8e0e-0ebc4b99d3d0@app.fastmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Sep 15, 2026, at 12:20 PM, Rui Zhao wrote:
> Hi Michael,
>
> The tests of my last mail (standby promotion, pg_upgrade with the counter
> past 2^32, rewrites after ALTER TABLE SET (toast_value_type), logical
> replication) give the same results on master at fb60892f40, and
> check-world passes there with 0006 and 0007 applied.
>
>> One thing that neither you nor Greg pointed out is a disturbance I
>> have found in amcheck. The code relied on a vartag to decide the max
>> chunk size of a TOAST blob:
>
> 1. Once 0004 and 0005 are in, amcheck is the one consumer of oid8
> pointers left that no test runs with IDs past 2^32. 0006 adds a TAP
> test for it.
Hello Rui,
> If I cast the value ID that check_toasted_attribute() passes to
> toast_valueid_scankey_init() to Oid by hand, the test fails, with
> verify_heapam() reporting every value:
>
> toast value 4295067314 not found in toast table
+1, and thank you for showing it red as well as green. That is the right
follow-up to the thing Michael found in the same file, since the reason
nothing caught that was the same reason nothing would catch this: amcheck
never runs against a counter past 2^32.
> 2. The "compressed out-of-line values" of check_heap.sql are not out of
> line. 0007 makes them larger.
Good catch, and I can confirm the diagnosis. I can also tell you the fix is
not large enough, because the outcome depends on default_toast_compression
and your numbers are the pglz ones. Same two values, same
SET STORAGE EXTENDED, on one build:
method input stored placement
pglz repeat('xyzzy',20000) 1161 inline
pglz repeat('xyzzy',100000) 5735 out-of-line
lz4 repeat('xyzzy',20000) 415 inline
lz4 repeat('xyzzy',100000) 1983 inline
So 0007 fixes check_heap.sql on a pglz build and leaves it checking nothing
on an lz4 one, which is the default where lz4 is compiled in. Whatever the
size, a repeat() of a short string compresses far enough that the threshold
is a property of the compression method rather than of the test.
Something that resists compression avoids the question entirely. This goes
out of line under both methods:
(SELECT string_agg(md5(g::text), '') FROM generate_series(1, 2000) g)
pglz 64000 chars, stored 64000, out-of-line
lz4 64000 chars, stored 59738, out-of-line
Pinning default_toast_compression in the test would work too, though it costs
you the coverage of whichever method you did not pin.
> 3. That works with either of two behaviors for ALTER TABLE, and the
> current one is neither. I prefer a) and can write it.
> Today SET and RESET (toast_value_type) change pg_class.reloptions and
> nothing else, and no rewrite applies the new value [...] A dump and
> restore is the only thing that applies the reloption. Other heap
> reloptions are applied by a rewrite
This is a better diagnosis than the one I posted an hour ago. I treated
the surprise as a reporting problem and sent a warning for the reset case.
You have identified that the surprise is a consequence of this reloption
being the only one that no rewrite applies, which is the actual oddity.
The fillfactor page counts make that hard to argue with.
So please consider my patch withdrawn in favor of either of yours. Under b)
it is plainly unnecessary, since a rejected reset has nothing to warn about.
Under a) it is close to unnecessary too, because a user who resets then
rewrites has asked for the change and the two paths agree.
If it helps break the tie, I lean b), for the reason you give: the reloption
then always describes the type in use, whether set or absent. That is the
property that makes Bharath's 0001 unnecessary rather than merely
outvoted, and it is the one that removes the case I was worried about, where
a reset taken for an unrelated reason turns into a restore that will not
finish on a relation with more than four billion out-of-line values.
The cost you name is real, a rewrite behind ALTER TABLE ... SET where no
other reloption does that. I would take it, because SET ACCESS METHOD is
already precedent for exactly that shape, and because an expensive operation
the user asked for beats a cheap one whose consequence shows up later
somewhere else. a) is still a clear improvement on today and I would not
object to it.
Either way this is Michael's call to make, I'm anxious to hear his thoughts
on this.
best.
-greg
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Sami Imseih | 2026-09-15 17:10:54 | Re: Report index currently being vacuumed in pg_stat_progress_vacuum |
| Previous Message | Dean Rasheed | 2026-09-15 16:59:32 | Re: Global temporary tables |