Re: [PATCH] Allow UNLISTEN during recovery

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, and it 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 DISCARD allows 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 commands not allowed and moved to the list of those which are allowed [1]. Moreover, src/test/regress/sql/hs_standby_allowed.sql and src/test/regress/sql/hs_standby_disallowed.sql tests should be updated based on these changes in the patch. What is surprising to me is that make check-world does not fail with this change, but there is an explicit check for UNLISTEN *. So does this mean those tests are not run or does it mean that this patch does not 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