Hi, On Fri, Sep 4, 2026 at 2:05 PM Masahiko Sawada <[email protected]> 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
