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

Hi,

On Thu, Aug 27, 2026 at 12:49 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> 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.

Yes, that's correct. I came to the same conclusion.

> 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.

Agreed. Adding a new table AM callback needs consensus and is time
consuming. Perhaps we can try for HEAD based on field reports that
require the concurrent repack feature for non-heap table AMs.

> So restricting it to heap for v19 seems fine
> to me.

+1. I will send a patch soon.

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com