From: Antonin Houska <ah@cybertec.at>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Mihail Nikalayeu <mihailnikalayeu@gmail.com>,
    Srinath Reddy Sadipiralla <srinath2133@gmail.com>,
    Matthias van de Meent <boekewurm+postgres@gmail.com>,
    Pg Hackers <pgsql-hackers@lists.postgresql.org>,
    Robert Treat <rob@xzilla.net>
Subject: Re: Adding REPACK [concurrently]
In-reply-to: <23099.1774429249@localhost>
References: <202603242222.5i7awkn7jpdr@alvherre.pgsql>
 <23099.1774429249@localhost>
Comments: In-reply-to Antonin Houska <ah@cybertec.at>
   message dated "Wed, 25 Mar 2026 10:00:49 +0100."
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <50824.1774457576.1@localhost>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 25 Mar 2026 17:52:56 +0100
Message-ID: <50825.1774457576@localhost>
Archived-At: 
 <https://www.postgresql.org/message-id/50825.1774457576%40localhost>
Precedence: bulk

Antonin Houska <ah@cybertec.at> wrote:

> Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> =

> > - 0008 to 0010 are as posted by Antonin; they are unchanged, except fo=
r
> >   fixes for the problems pointed out by Mihail.  Antonin, I would
> >   appreciate it if you want to change the "reform" bit in 0007 as
> >   discussed.
> =

> I've taken a look, but not sure if the tuple slots help here. In
> heapam_relation_copy_for_cluster(), both table_scan_getnextslot() and
> index_getnext_slot() call ExecStoreBufferHeapTuple() ->
> tts_buffer_heap_store_tuple(), which AFAICS do not deform the tuple. The=
n
> ExecFetchSlotHeapTuple() is used to retrieve the tuple, but again, the
> underlying slot (TTSOpsBufferHeapTuple) handles it by copying rather tha=
n
> deforming / forming. Thus I think the explicit "reforming" currently doe=
s not
> add any performance overhead.

Well, the deform / form steps do add some overhead of course, but these ar=
e
necessary to get rid of the values of the dropped columns. I wanted to say
that it wouldn't be cheaper with slots, because then we'd have to enforce =
the
deform / form steps too, although the coding would be different:

> Of course, we can still use the slots, and do the following: 1) enforce =
tuple
> deforming (by calling slot_getallattrs()), 2) set the dropped attributes=
 to
> NULL, 3) use ExecStoreVirtualTuple() to store the tuple into another slo=
t and
> 4) get the heap tuple from the other slot. Should I do that? I'm asking
> because I wasn't sure if you're concerned about performance or coding (o=
r
> both).

> Whatever approach we take, I see two more opportunities for better
> performance:
> =

> 1. Do the "reforming" only if there are some dropped columns. (AFAICS ev=
en the
> old CLUSTER / VACUUM FULL did not check this.)

I think this would need more work because CLUSTER / VACCUM FULL / REPACK d=
o
not remove the dropped attributes from the tuple descriptor. So the
optimization would only work until the first column is dropped. All the
following runs would then do the reforming even if no other colmns were dr=
oped
since the previous run.

Perhaps we can teach REPACK to remove dropped columns from the tuple
descriptor in the future.

-- =

Antonin Houska
Web: https://www.cybertec-postgresql.com