Hi Surya, > A few suggestions: > In vacuum.c: > 1) The header comment on vacuum_is_permitted_for_relation() still describes > only the permission-check path. > Now that the function also signals "relation no longer exists", the header > should mention both cases. > 2) A one-line comment at the pg_class_aclcheck_ext() call explaining why the > _ext variant is needed (concurrent drop in the whole-database > list-construction path) would help future readers, and guard against a > cleanup patch that silently reverts to pg_class_aclcheck().
Fixed. > In vacuum_concurrent_drop.spec: > 1) The spec sets "client_min_messages = ERROR", which suppresses the very > WARNING the patch emits. > Let's have the client_min_messages to WARNING, so we see the warning in the > .out file. Fixed. We need to grant pg_maintain to regress_vacuum to avoid "permission denied ..." warnings. For versions (< 17) which don't have pg_maintain role, we may still want to use "client_min_messages = ERROR". > 2) Minor comment, maybe adding the ANALYZE permutation (or a sibling spec) > would test that code path too. (Not a mandatory thing but a good to have) Fixed. -- Regards, ChangAo Chen
v3-0001-Handle-concurrent-drop-when-doing-whole-database-.patch
Description: Binary data
v3-0002-Add-test-case-for-vacuum-with-a-concurrent-drop.patch
Description: Binary data
