| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
| Cc: | Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Support for 8-byte TOAST values, round two |
| Date: | 2026-09-08 03:50:11 |
| Message-ID: | ap-F89TpWEq324ij@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Sep 07, 2026 at 04:54:35PM +0900, Michael Paquier wrote:
> Attached is v14, with all that addressed (should perhaps have moved
> the tests in 0008 to 0007, but it's late here and that does not change
> the stuff to review).
While doing a new round of self-review, that included some tests with
pg_upgrade, I have spotted a hole in the control file: the nextOid was
stored as an Oid instead of an Oid8. This would be bad when
upgrading, because we would conflict for oid8 TOAST values once the
counter gets higher than 2^32. This has also spotted a gap in
pg_resetwal and pg_control_checkpoint(), where we need to show the new
information as an oid8.
Thinking more about which part of the patch set makes the most sense
as an independent piece, I have decided to split the upgrade of
nextOid to Oid8 in the control file and the introduction of the new
GetNewObjectId8() as a patch of their own, labelled now at 0001. This
includes a battery of tests for pg_upgrade to check that the value is
correctly carried, resetwal and pg_control_checkpoint(). As a whole,
I'm feeling pretty much OK with how this part bundles.
Small note: while hacking on 0001 I have managed to break pg_resetwal
-o with negative values. Added a test for that on HEAD separately,
while on it..
There is still something that I am wondering about this part of the
patch, though, should we still publish in the output of
pg_control_checkpoint() and pg_controldata an Oid extracted from the
Oid8 value? I have not done that, and that does not make the control
file larger. Just wondering if that would be useful to keep because
we cannot really do an oid8->oid cast, and the oid information could
still be useful on its own?
The rest of the patch set is mostly following the same organization,
and I have spent a few hours working more on the last piece, making
cleaner the contract with the vartag handling (one could not the
changes in varatt.h, where I have changed to inline function for the
get/set Oid8 values in varatt_external_oid8).
0005~0007 in the set of patches cover gaps in the regression tests.
0005 is the same one set of SQLs as the previous patch set (mostly),
0006 and 0007 cover more cases with test_decoding, amcheck,
reloptions, the compression function, and the external toast pointer
comparison in toast_tuple_init(). They have been split as they can
still work once the reloption supports oid8, even if we don't make the
varatt_external added yet (last patch).
--
Michael
| Attachment | Content-Type | Size |
|---|---|---|
| v15-0001-Enlarge-OID-generation-to-8-bytes.patch | text/plain | 23.3 KB |
| v15-0002-Refactor-some-TOAST-value-ID-code-to-use-Oid8-in.patch | text/plain | 16.2 KB |
| v15-0003-Switch-pg_column_toast_chunk_id-return-value-fro.patch | text/plain | 5.7 KB |
| v15-0004-Add-support-for-TOAST-chunk_id-type-in-binary-up.patch | text/plain | 9.3 KB |
| v15-0005-Add-relation-option-toast_value_type.patch | text/plain | 6.4 KB |
| v15-0006-Add-support-for-oid8-TOAST-values.patch | text/plain | 17.4 KB |
| v15-0007-Add-tests-for-TOAST-relations-with-oid8-as-value.patch | text/plain | 21.1 KB |
| v15-0008-More-test-coverage-for-TOAST-relations-with-oid8.patch | text/plain | 13.4 KB |
| v15-0009-More-test-coverage-for-oid8-with-compression-and.patch | text/plain | 8.4 KB |
| v15-0010-Add-support-for-TOAST-pointers-as-oid8.patch | text/plain | 51.0 KB |
| v15-0011-Fix-toast_tuple_find_biggest_attribute-for-OID8-.patch | text/plain | 6.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-09-08 03:52:19 | Re: Crashes on a partition whose concurrent detach never finished |
| Previous Message | Rithvika Devisetti | 2026-09-08 03:49:38 | Re: WAIT FOR NO_THROW option could use some documentation |