Re: REPACK (CONCURRENTLY) doesn't check the table AM

On Thu, Aug 27, 2026 at 12:16 PM Nathan Bossart
<nathandbossart(at)gmail(dot)com> wrote:
>
> On Thu, Aug 27, 2026 at 11:59:10AM -0700, Bharath Rupireddy wrote:
> > On Thu, Aug 27, 2026 at 9:23 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
> >> REPACK (CONCURRENTLY) replays changes by decoding them from WAL, so it
> >> needs the table AM to be logically decodable, but
> >> check_concurrent_repack_requirements() doesn't check that. Should it be
> >> restricted to heap for v19?
> >
> > Logical decoding doesn't check the table AM type, but goes ahead and
> > decodes the WAL records (for that matter, none of CLUSTER, VACUUM
> > FULL, and REPACK have checks on table AM. They hand off at some point
> > to the table AM layer). I'm not sure if gating it just for concurrent
> > repack is the right choice. Is it that we want to have it just for
> > concurrent repack since it's new code with a new logical decoding
> > plugin and we want some field reports of needing it for other table
> > AMs?
>
> If the other commands are similarly broken, then perhaps this isn't a valid
> open item for v19. But IIUC concurrent repack is the only one of the
> commands you listed that depends on logical decoding, and if the table AM
> doesn't support that, then it won't work properly.

If the table AM doesn't support logical decoding, concurrent repack
would silently lose some table data as it misses the changes
happened during the rewrites. I think we should have the check for it.
One idea would be to have a new table AM callback returning true if
the table AM supports concurrent repack (i.e., its rmgr provides
rm_decode callback), but I think it's not the right time to introduce
a new table AM callback. So restricting it to heap for v19 seems fine
to me.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com