Fix SHOW ALL command for non-superusers with replication connection

Since Postgres 10, SHOW commands can be triggered with replication
connections in a WAL sender context, however it missed that a
transaction context is needed for syscache lookups.  This commit makes
sure that the syscache lookups can happen correctly by setting a
transaction context when running SHOW commands in a WAL sender.

Superuser-only parameters can be displayed using SHOW commands not only
to superusers, but also to members of system role pg_read_all_settings,
which requires a syscache lookup to check if the connected role is a
member of this system role or not, or the instance crashes.  Superusers
do not need to check the syscache so it worked correctly in this case.

New tests are added to cover this issue.

Reported-by: Alexander Kukushkin
Author: Michael Paquier
Reviewed-by: Álvaro Herrera
Discussion: https://postgr.es/m/15734-2daa8761eeed8e20@postgresql.org
Backpatch-through: 10

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 9c349108678d976e07e02b481c2a9a134fd92617..c9ff4a2b3d38df55d16c6a680338d846b5bc2126 100644 (file)
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -1552,7 +1552,10 @@ exec_replication_command(const char *cmd_string)
                                DestReceiver *dest = CreateDestReceiver(DestRemoteSimple);
                                VariableShowStmt *n = (VariableShowStmt *) cmd_node;
 
+                               /* syscache access needs a transaction environment */
+                               StartTransactionCommand();
                                GetPGVariable(n->name, dest);
+                               CommitTransactionCommand();
                        }
                        break;
 

diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 8ad76b19c5bbfb99bb9c32a2ada270f3b47ab927..7047d88fb62d277977e8108ea463f3cd7b98ac02 100644 (file)
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -441,7 +441,8 @@ sub init
 
        TestLib::system_or_bail('initdb', '-D', $pgdata, '-A', 'trust', '-N',
                @{ $params{extra} });
-       TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
+       TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata,
+               @{ $params{auth_extra} });
 
        open my $conf, '>>', "$pgdata/postgresql.conf";
        print $conf "\n# Added by PostgresNode.pm\n";

diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index 8dff5fc7202f1287dec97fe2216318a2465b1b98..d2f11c36ba357c53e6f05a16b92e78a76ed424a0 100644 (file)
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -3,11 +3,14 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 26;
+use Test::More tests => 32;
 
 # Initialize master node
 my $node_master = get_new_node('master');
-$node_master->init(allows_streaming => 1);
+# A specific role is created to perform some tests related to replication,
+# and it needs proper authentication configuration.
+$node_master->init(allows_streaming => 1,
+       auth_extra => ['--create-role', 'repl_role']);
 $node_master->start;
 my $backup_name = 'my_backup';
 
@@ -117,6 +120,55 @@ test_target_session_attrs($node_master, $node_standby_1, $node_master, "any",
 test_target_session_attrs($node_standby_1, $node_master, $node_standby_1,
        "any", 0);
 
+# Test for SHOW commands using a WAL sender connection with a replication
+# role.
+note "testing SHOW commands for replication connection";
+
+$node_master->psql('postgres',"
+CREATE ROLE repl_role REPLICATION LOGIN;
+GRANT pg_read_all_settings TO repl_role;");
+my $master_host = $node_master->host;
+my $master_port = $node_master->port;
+my $connstr_common = "host=$master_host port=$master_port user=repl_role";
+my $connstr_rep = "$connstr_common replication=1";
+my $connstr_db = "$connstr_common replication=database dbname=postgres";
+
+# Test SHOW ALL
+my ($ret, $stdout, $stderr) =
+    $node_master->psql('postgres', 'SHOW ALL;',
+                      on_error_die => 1,
+                      extra_params => [ '-d', $connstr_rep ]);
+ok($ret == 0, "SHOW ALL with replication role and physical replication");
+($ret, $stdout, $stderr) =
+    $node_master->psql('postgres', 'SHOW ALL;',
+                      on_error_die => 1,
+                      extra_params => [ '-d', $connstr_db ]);
+ok($ret == 0, "SHOW ALL with replication role and logical replication");
+
+# Test SHOW with a user-settable parameter
+($ret, $stdout, $stderr) =
+    $node_master->psql('postgres', 'SHOW work_mem;',
+                      on_error_die => 1,
+                      extra_params => [ '-d', $connstr_rep ]);
+ok($ret == 0, "SHOW with user-settable parameter, replication role and physical replication");
+($ret, $stdout, $stderr) =
+    $node_master->psql('postgres', 'SHOW work_mem;',
+                      on_error_die => 1,
+                      extra_params => [ '-d', $connstr_db ]);
+ok($ret == 0, "SHOW with user-settable parameter, replication role and logical replication");
+
+# Test SHOW with a superuser-settable parameter
+($ret, $stdout, $stderr) =
+    $node_master->psql('postgres', 'SHOW data_directory;',
+                      on_error_die => 1,
+                      extra_params => [ '-d', $connstr_rep ]);
+ok($ret == 0, "SHOW with superuser-settable parameter, replication role and physical replication");
+($ret, $stdout, $stderr) =
+    $node_master->psql('postgres', 'SHOW data_directory;',
+                      on_error_die => 1,
+                      extra_params => [ '-d', $connstr_db ]);
+ok($ret == 0, "SHOW with superuser-settable parameter, replication role and logical replication");
+
 note "switching to physical replication slot";
 
 # Switch to using a physical replication slot. We can do this without a new