Mitar, thanks for giving this your attention!
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?
>
I've made the requested changes to the docs and to the regression tests.
I think that specifically the standby regression tests do not get executed
by check-world - see section
https://www.postgresql.org/docs/current/regress-run.html#id-1.6.20.5.8. I'm
guessing this should be executed as part of the build verification pipeline
for patches, but I don't know anything about the PostgreSQL build
infrastructure.
Unfortunately I'm extremely tight for time at the moment and don't have
time to do the appropriate hot standby setup to test this... As the patch
is pretty straightforward, and since I'm hoping you guys execute the tests
on your end, I hope that's acceptable. If it's absolutely necessary for me
to test the patch locally, let me know and I'll do so.
From 8c816354e820bf3d0be69d55dbf0052b1d27feeb Mon Sep 17 00:00:00 2001
From: Shay Rojansky <r...@roji.org>
Date: Tue, 15 Jan 2019 18:49:40 +0100
Subject: [PATCH] Allow unlisten when in hot standby:wq
---
doc/src/sgml/high-availability.sgml | 16 ++++++++++------
src/backend/tcop/utility.c | 2 +-
src/test/regress/sql/hs_standby_allowed.sql | 4 ++++
src/test/regress/sql/hs_standby_disallowed.sql | 2 --
4 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml
index 4882b20828..bb1c86f73e 100644
--- a/doc/src/sgml/high-availability.sgml
+++ b/doc/src/sgml/high-availability.sgml
@@ -1767,6 +1767,11 @@ if (!triggered)
Plugins and extensions - <command>LOAD</command>
</para>
</listitem>
+ <listitem>
+ <para>
+ UNLISTEN
+ </para>
+ </listitem>
</itemizedlist>
</para>
@@ -1856,18 +1861,17 @@ if (!triggered)
</listitem>
<listitem>
<para>
- <command>LISTEN</command>, <command>UNLISTEN</command>, <command>NOTIFY</command>
+ <command>LISTEN</command>, <command>NOTIFY</command>
</para>
</listitem>
</itemizedlist>
</para>
<para>
- In normal operation, <quote>read-only</quote> transactions are allowed to
- use <command>LISTEN</command>, <command>UNLISTEN</command>, and
- <command>NOTIFY</command>, so Hot Standby sessions operate under slightly tighter
- restrictions than ordinary read-only sessions. It is possible that some
- of these restrictions might be loosened in a future release.
+ In normal operation, <quote>read-only</quote> transactions are allowed to use
+ <command>LISTEN</command>, and <command>NOTIFY</command>, so Hot Standby sessions
+ operate under slightly tighter restrictions than ordinary read-only sessions.
+ It is possible that some of these restrictions might be loosened in a future release.
</para>
<para>
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 27ae6be751..44136060d6 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -629,7 +629,7 @@ standard_ProcessUtility(PlannedStmt *pstmt,
{
UnlistenStmt *stmt = (UnlistenStmt *) parsetree;
- PreventCommandDuringRecovery("UNLISTEN");
+ /* allow UNLISTEN during recovery, which is a noop */
CheckRestrictedOperation("UNLISTEN");
if (stmt->conditionname)
Async_Unlisten(stmt->conditionname);
diff --git a/src/test/regress/sql/hs_standby_allowed.sql b/src/test/regress/sql/hs_standby_allowed.sql
index a33199dbbd..1f1cdf5a00 100644
--- a/src/test/regress/sql/hs_standby_allowed.sql
+++ b/src/test/regress/sql/hs_standby_allowed.sql
@@ -110,6 +110,10 @@ LOCK hs1 IN ROW SHARE MODE;
LOCK hs1 IN ROW EXCLUSIVE MODE;
COMMIT;
+-- UNLISTEN
+unlisten a;
+unlisten *;
+
-- LOAD
-- should work, easier if there is no test for that...
diff --git a/src/test/regress/sql/hs_standby_disallowed.sql b/src/test/regress/sql/hs_standby_disallowed.sql
index 21bbf526b7..a470600eec 100644
--- a/src/test/regress/sql/hs_standby_disallowed.sql
+++ b/src/test/regress/sql/hs_standby_disallowed.sql
@@ -88,8 +88,6 @@ COMMIT;
-- Listen
listen a;
notify a;
-unlisten a;
-unlisten *;
-- disallowed commands
--
2.19.1
