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

Hi,

On Fri, Sep 4, 2026 at 2:05 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> I've looked at the patch and have one comment:

Thanks for reviewing it.

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

That works for me. If a table AM that just copies the heap AM handler
as-is works with repack, that is fine. If a table AM that copies the
heap AM handler but overwrites one or more functions errors out with
repack, that still serves the purpose. Also, I am fine without a test
for this because we don't have another table AM in the core and all
tests rely on reusing the same heap AM handler.

> I attached the updated patch. Please review it.

The v4 patch looks good to me. pgindent and tests are happy.

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