Hi Zsolt, I read through the thread, very nice points were raised. I looked at v5. It applies cleanly on master and builds without warnings.
One concern with the new USING INDEX branch in get_tables_to_repack_partitioned(): parent_oid = IndexGetRelation(child_oid, true); ... partoids = find_inheritance_children(parent_oid, NoLock); For a partitioned table below the first level, parent_oid isn't locked, so it can be dropped concurrently (with root -> p1 -> p11 -> ..., DROP TABLE p11 only locks p1). find_inheritance_children() goes through has_subclass(), which raises "cache lookup failed for relation" when the pg_class row is gone. The surrounding code already handles a vanished relation (IndexGetRelation(..., true) and the NULL check in warn_foreign_partition()), but this call doesn't. This is the same race as in your find_all_inheritors(NoLock) thread [1]. As I read v2 there, the missing_ok handling is internal to find_all_inheritors(), so it wouldn't cover this new call. Maybe expose the missing_ok variant and use it here, or go back to the v4 shape (find_all_inheritors() on the table), since you saw no measurable difference between them and v4 would get that fix for free. Minor comment, database-wide REPACK still skips foreign tables silently, which matches database-wide VACUUM, so I think that's right. It might be worth saying so in the commit message, given Bharath's point about keeping all the forms consistent. Otherwise the permission ordering and the tests look good to me. [1] https://www.postgresql.org/message-id/flat/CAN4CZFMp2oRXb_AC6%2B6%2BNxdjPHE2DJHFz_5WTpm99ovjKD9xTA%40mail.gmail.com Regards, Surya Poondla
