Fix the ALTER SUBSCRIPTION to reflect the change in run_as_owner option.

Reported-by: Jeff Davis
Author: Hou Zhijie
Reviewed-by: Amit Kapila
Backpatch-through: 16
Discussion: http://postgr.es/m/17b62714fd115bd1899afd922954540a5c6a0467[email protected]

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 34d881fd94f1cc68dc5310b6b1306ab177898b5d..6fe111e98d3afbc71961e522e195d66f8fa44c9f 100644 (file)
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -1204,6 +1204,13 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
                        = true;
                }
 
+               if (IsSet(opts.specified_opts, SUBOPT_RUN_AS_OWNER))
+               {
+                   values[Anum_pg_subscription_subrunasowner - 1] =
+                       BoolGetDatum(opts.runasowner);
+                   replaces[Anum_pg_subscription_subrunasowner - 1] = true;
+               }
+
                if (IsSet(opts.specified_opts, SUBOPT_ORIGIN))
                {
                    values[Anum_pg_subscription_suborigin - 1] =

diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index 3c1a0869eca3a398d38c8171cc6514f71eab2293..b15eddbff3c86f24f38f66fba815c6810b70fb3c 100644 (file)
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -155,14 +155,16 @@ ALTER SUBSCRIPTION regress_testsub SET PUBLICATION testpub2, testpub3 WITH (refr
 ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist2';
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'newname');
 ALTER SUBSCRIPTION regress_testsub SET (password_required = false);
+ALTER SUBSCRIPTION regress_testsub SET (run_as_owner = true);
 \dRs+
                                                                                                                List of subscriptions
       Name       |           Owner           | Enabled |     Publication     | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Synchronous commit |           Conninfo           | Skip LSN 
 -----------------+---------------------------+---------+---------------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+--------------------+------------------------------+----------
- regress_testsub | regress_subscription_user | f       | {testpub2,testpub3} | f      | off       | d                | f                | any    | f                 | f             | off                | dbname=regress_doesnotexist2 | 0/0
+ regress_testsub | regress_subscription_user | f       | {testpub2,testpub3} | f      | off       | d                | f                | any    | f                 | t             | off                | dbname=regress_doesnotexist2 | 0/0
 (1 row)
 
 ALTER SUBSCRIPTION regress_testsub SET (password_required = true);
+ALTER SUBSCRIPTION regress_testsub SET (run_as_owner = false);
 -- fail
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = '');
 ERROR:  replication slot name "" is too short

diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 55d7dbc9ab90b0fa6346b9d5625c0c2e7a14a969..444e563ff3f43222b5a552e33d0b22fac1e994e3 100644 (file)
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -94,9 +94,11 @@ ALTER SUBSCRIPTION regress_testsub SET PUBLICATION testpub2, testpub3 WITH (refr
 ALTER SUBSCRIPTION regress_testsub CONNECTION 'dbname=regress_doesnotexist2';
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = 'newname');
 ALTER SUBSCRIPTION regress_testsub SET (password_required = false);
+ALTER SUBSCRIPTION regress_testsub SET (run_as_owner = true);
 \dRs+
 
 ALTER SUBSCRIPTION regress_testsub SET (password_required = true);
+ALTER SUBSCRIPTION regress_testsub SET (run_as_owner = false);
 
 -- fail
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = '');

diff --git a/src/test/subscription/t/033_run_as_table_owner.pl b/src/test/subscription/t/033_run_as_table_owner.pl
index 9de3c04a0c214f8b8f0cee13ccef7c4ae22d5ea3..f4083202e5313e9e31999397dbe16f7dd874f34e 100644 (file)
--- a/src/test/subscription/t/033_run_as_table_owner.pl
+++ b/src/test/subscription/t/033_run_as_table_owner.pl
@@ -193,6 +193,37 @@ GRANT regress_alice TO regress_admin WITH INHERIT TRUE, SET FALSE;
 expect_replication("alice.unpartitioned", 3, 7, 13,
    "with INHERIT but not SET ROLE can replicate");
 
+# Similar to the previous test, remove all privileges again and instead,
+# give the ability to SET ROLE to regress_alice.
+$node_subscriber->safe_psql(
+   'postgres', qq(
+SET SESSION AUTHORIZATION regress_alice;
+REVOKE ALL PRIVILEGES ON alice.unpartitioned FROM regress_admin;
+RESET SESSION AUTHORIZATION;
+GRANT regress_alice TO regress_admin WITH INHERIT FALSE, SET TRUE;
+));
+
+# Because replication is running as the subscription owner in this test,
+# the above grant doesn't help.
+publish_insert("alice.unpartitioned", 14);
+expect_failure(
+   "alice.unpartitioned",
+   3,
+   7,
+   13,
+   qr/ERROR: ( [A-Z0-9]+:)? permission denied for table unpartitioned/msi,
+   "with no privileges cannot replicate");
+
+# Allow the replication to run as table owner and check that things start
+# working.
+$node_subscriber->safe_psql(
+   'postgres', qq(
+ALTER SUBSCRIPTION admin_sub SET (run_as_owner = false);
+));
+
+expect_replication("alice.unpartitioned", 4, 7, 14,
+   "can replicate after setting run_as_owner to false");
+
 # Remove the subscrition and truncate the table for the initial data sync
 # tests.
 $node_subscriber->safe_psql(
@@ -222,7 +253,7 @@ ALTER SUBSCRIPTION admin_sub ENABLE;
 # Because the initial data sync is working as the table owner, all
 # data should be copied.
 $node_subscriber->wait_for_subscription_sync($node_publisher, 'admin_sub');
-expect_replication("alice.unpartitioned", 3, 7, 13,
+expect_replication("alice.unpartitioned", 4, 7, 14,
    "table owner can do the initial data copy");
 
 done_testing();