The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: not tested
Spec compliant: not tested
Documentation: tested, failed
Hi!
I read through the discussion. I agree with the idea here. I think if DISCARD ALL is allowed during hot standby mode,
andit includes UNLISTEN *, only UNLISTEN * should also be allowed. It seems this patch fixes this, but I could not test
it(I do not know how to force this state). I would go further though, and I would also allow UNLISTEN a. Because also
DISCARDallows discarding only part of resources.
So patch looks good to me, but documentation changes are missing from it. UNLISTEN should be removed from the list of
commandsnot allowed and moved to the list of those which are allowed [1]. Moreover,
src/test/regress/sql/hs_standby_allowed.sqland src/test/regress/sql/hs_standby_disallowed.sql tests should be updated
basedon these changes in the patch. What is surprising to me is that make check-world does not fail with this change,
butthere is an explicit check for UNLISTEN *. So does this mean those tests are not run or does it mean that this patch
doesnot fix the problem?
[1] https://www.postgresql.org/docs/current/hot-standby.html#HOT-STANDBY-USERS
Mitar
The new status of this patch is: Waiting on Author
