Hey Michael, Bharath, all, First, I've been following this on the earlier thread and this one and I greatly appreciate the work you're doing Michael. I think it is valuable and necessary. I've been out most of the Summer on vacation (I finally took one!) and so haven't had the time until yesterday to dive into this.
So, I applied and tested the v15 set and then the adjusted v16-0001 on top of current master. All good, tests passing. Beyond running the suite I poked at the feature directly. pg_resetwal -o 5000000000 followed by an insert into an oid8 table gives a chunk_id of 5000000000 and reads back correctly, while the oid table next to it wraps to 705032705. pg_column_toast_chunk_id() reports oid8 and the full value. I read the test body and found that your pg_upgrade tests do assert the 2^32+100000 counter carries over, thanks for the attention to detail. I spent most of my time trying to put numbers on the objections already raised, since several of them were estimates. I think that Matthias is right about the index, and I've reproduced it on real TOAST indexes, not a standalone proxy table. Same data on both sides, 40000 chunks: 917504 bytes with oid against 1277952 with oid8, so +39.3%. With denser 2-chunk values it's 1810432 against 2539520, +40.3%. I see that Bharath's independent run at 4GB of TOAST landed on +40.3% as well, so I'd treat the 40% as settled. However, I'd add the other half of that ratio, because I think it changes how much weight the number should carry. The TOAST heap was byte-identical in both of my runs, and the index is only 1.1% to 1.6% of TOAST storage there, 1.35% in Bharath's. So the extra bytes come to under 0.7% of the total TOAST footprint. Robert's last concern on the previous thread lingered and so I wanted to check because it seems untested so far in practice. The thought was that some tuples storable today would not be storable at all with the wider pointer. I tried, but couldn't reproduce the concern in practice. I created a table of N text columns, all set STORAGE EXTERNAL, 3000 bytes per value: cols oid oid8 360 OK OK 370 OK row is too big: size 8824, maximum size 8160 450 OK too big 460 too big too big So the measured ceiling on out-of-line attributes per row goes from about 453 to about 366, call it 19%. Between roughly 370 and 455 such columns you get a row that stores fine as oid and cannot be stored at all as oid8, and there is nothing the TOAST code can do about it. Wide all-text tables like might exist, but how common are they? This is a hard wall not a slowdown, so maybe a sentence in the docs next to the reloption would help? On the main heap, the cost is alignment dependent and sometimes free. One toasted column came out byte-identical, 52.22 bytes per row either way, because MAXALIGN swallows the extra 4 bytes. Two toasted columns showed it: 68.40 against 76.60 bytes per row, +12%. Bharath measured +15.4% on his shape. Worth knowing that the answer is "it depends on the row layout" rather than a flat 4 bytes per value. Which brings me to the v16-0001 doc change. I think this sentence is stronger than the code supports: The counter is 8 bytes wide and never wraps around during the life of a cluster. It can wrap, using the option that paragraph documents. pg_resetwal happily accepts -o 18446744073709551615, and it rejects 2^64 with a clean error, so the boundary itself is handled. But set the counter to 18446744073709550000, start up, insert 3000 toasted rows into an oid8 table, and the counter rolls over: chunk_ids run from 16384 to 18446744073709551615, 2778 of the 3000 rows land on low ids, and next_oid after a checkpoint reads 24576. So it is possible to induce that corner case, however unlikely in practice. The good news is the wrap is handled the way the design says: it resumes at FirstNormalObjectId rather than 0, all 6000 rows I ended up with detoast correctly, all chunk_ids were distinct, and verify_heapam() reports zero corruption rows on both the toast relation and the main table. So this isn't a data integrity problem. It does mean the invariant the oid8 path leans on is administrator-breakable, though. Since there's no entry recheck for oid8, monotonicity is what guarantees uniqueness, and after a wrap you no longer have it. Your own example is the sharp version of this: reset to a past value and the next insert fails hard on the toast index unique constraint. I reproduced that too and it fails cleanly so the pre-existing row's data was intact. It is a hard error and not silent corruption. Two smaller things. The duplicate key error a user hits in that state names an internal toast index and gives no hint about the cause, so if you want one place to spend a few lines it might be an errhint there. And the reloption docs currently describe what toast_value_type sets but none of the tradeoffs above, nor that dump and restore is the only way to move an existing table. Bharath's point 4 established that pg_dump does emit the changed reloption, so the migration path works, it just isn't written down. Nit, there's also a stray double space in "creating a toast relation". On the bigger question of per-relation sequences, I agree with Robert that bottlenecking this work behind that change is probably the wrong trade. This fixes is a live defect that users hit today. So: +1 from me on the direction and on this implementation. The v16-0001 counter work reads fine to me apart from that one doc sentence. I'll keep going on 0006 and 0010, which are the two I've read least carefully so far. Reproducers for the row-size and wraparound cases are trivial to reconstruct from the numbers above, but happy to post scripts if useful. best. -greg
