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

On Mon, Aug 31, 2026 at 8:46 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Thu, Aug 27, 2026 at 3:51 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > > On Thu, Aug 27, 2026 at 12:57 PM Bharath Rupireddy
> > > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > > >
> > > > > 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.
>
> Please find the attached v3 patch with a test added similar to other
> restricted error cases. Sorry for the noise.

I've looked at the patch and have one comment:

+ if (rel->rd_rel->relam != HEAP_TABLE_AM_OID)

Probably it's better to use GetHeapamTableAmRoutine() instead of using
heap AM oid. It would allow the following use case the patch adds in
the regression tests:

+CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler;
+CREATE TABLE repack_conc_heap2 (i int) USING heap2;
+REPACK (CONCURRENTLY) repack_conc_heap2;

REPACK'ing on such a table technically works fine. It's unlikely to
drop the check so I think we can add the check without regression
tests.

I attached the updated patch. Please review it.

Regards,

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