public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Srinath Reddy Sadipiralla <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: Imran Zaheer <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: PostgreSQL mailing lists <[email protected]>
Cc: Konstantin Knizhnik <[email protected]>
Subject: Re: BUG #19519: REPACK can fail due to missing chunk for toast value
Date: Wed, 8 Jul 2026 10:10:11 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAFC+b6rLNvDzzYxrfLdq7CWbbouHcuOxi-mjEOW1r3qy2uAPMQ@mail.gmail.com>
	<CA+UBfan1QukfNDPvb2YpPG-ME4vUduBfbbe5uWptXDryZbtwVA@mail.gmail.com>
	<[email protected]>
	<CAEze2WhCRX86ZY2QN7vuGc3wsAyOJbArFSc0i5KtMVnsguA7Kw@mail.gmail.com>
	<[email protected]>
	<CAEze2WgBjhf6KTKuJ-UqHHGj9Z66f62grmJgZ1EazvMH9HU+XA@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAFC+b6rOdx6rMi_numCrK8cM4z_HWQKrAcBY0dfrRcRWTRpsBQ@mail.gmail.com>
	<[email protected]>

On Wed, Jul 08, 2026 at 07:15:16AM +0900, Michael Paquier wrote:
> One thing that I still don't like much in the patch as written is my
> use of a missing_ok argument, which feels super confusing as it
> applies only to the underlying external TOAST, if the relation has
> any.  I'd be tempted to rewrite this portion of the patch with a
> uint32 flags.  Even if we assign one value for now (say MISSING_TOAST,
> MISSING_TOAST_OK or whatever), it would allow more flexibility on ABI
> grounds if we want more like states in the future across this portion
> of the stack.  Again, I strongly doubt that we will be able to
> backpatch any of that.

A couple of extra notes while I do not forget about that stuff..  The
patch may be better split into two if we go with this approach, as the
index build and rewrite paths require different solutions:
- One for the rewrite path.  It makes little sense to do any kind of
aggressive early detoasting because it may be wasteful due to the
tuple rewrites that update their data with only copies by reference
(main relation tuple is rewritten, reuses the same external TOAST
tuple).  For workloads where UPDATEs do not touch the TOASTed
attributes, that would be a waste.
- One for the index build path, which is actually too aggressive with
its early detoasting, now that I think about it.  There should be no
need to perform a detoast for anything else than the attributes that
are used in the index definition or the attributes that are used in
index expressions.  So as written this patch would lead to a
regression.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (21+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: BUG #19519: REPACK can fail due to missing chunk for toast value
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox