Hi ChangAo,

Thank you for adding the test case.
I tested v2 locally and looks good.

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().

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

Regards,
Surya Poondla

Reply via email to