Re: [survey] New "Stable" QueryId based on normalized query text
> One problem with pg_stat_statement's normalized query is that it's not > stable, it's storing the normalized version of the first query string > passed when an entry is created. So you could have different strings > depending on whether the query was fully qualified or relying on > search path for instance. I think normalized query stored in pg_stat_statement it's not very important. it might look something like that ` query | calls | queryid | sql_id ---+---++ Select * from t | 4 | 762359559 | 680388963 select * from t | 7 | 3438533065 | 680388963 select * from test2.t | 1 | 230362373 | 680388963 ` we can cut schema name in sql normalization algorithm Efimkin Evgeny
Re: [survey] New "Stable" QueryId based on normalized query text
Hi!
What about adding new column in pg_stat_statements e.g. sql_id it's hash from
normalized query. Аnd add function which get that hash (using raw_parser,
raw_expression_tree_walker) for any query
`
postgres=# select get_queryid('select 1');
get_queryid
-
680388963
(1 row)
`
that function can be used on pg_stat_activity(query) for join
pg_stat_statements if it need.
12.08.2019, 14:51, "legrand legrand" :
> Hi Jim,
>
> Its never too later, as nothing has been concluded about that survey ;o)
>
> For information, I thought It would be possible to get a more stable
> QueryId,
> by hashing relation name or fully qualified names.
>
> With the support of Julien Rouhaud, I tested with this kind of code:
>
> case RTE_RELATION:
> if (pgss_queryid_oid)
> {
> APP_JUMB(rte->relid);
> }
> else
> {
> rel =
> RelationIdGetRelation(rte->relid);
>
> APP_JUMB_STRING(RelationGetRelationName(rel));
>
> APP_JUMB_STRING(get_namespace_name(get_rel_namespace(rte->relid)));
> RelationClose(rel);
> {
>
> thinking that 3 hash options would be interesting in pgss:
> 1- actual OID
> 2- relation names only (for databases WITHOUT distinct schemas contaning
> same tables)
> 3- fully qualified names schema.relname (for databases WITH distinct schemas
> contaning same tables)
>
> but performances where quite bad (it was a few month ago, but I remenber
> about a 1-5% decrease).
> I also remenber that's this was not portable between distinct pg versions
> 11/12
> and also not sure it was stable between windows / linux ports ...
>
> So I stopped here ... Maybe its time to test deeper this alternative
> (to get fully qualified names hashes in One call) knowing that such
> transformations
> will have to be done for all objects types (not only relations) ?
>
> I'm ready to continue testing as it seems the less impacting solution to
> keep actual pgss ...
>
> If this doesn't work, then trying with a normalized query text (associated
> with search_path) would be the
> other alternative, but impacts on actual pgss would be higher ...
>
> Regards
> PAscal
>
> --
> Sent from: https://www.postgresql-archive.org/PostgreSQL-hackers-f1928748.html
Efimkin Evgeny
Re: Special role for subscriptions
Hi! > These are basically that the truncate, insert, delete and insert > rights for the role creating the subscription. Why would we actually > need that? It's for security reasons. Because possible to attack target server. If publication have system tables for instance pg_authid > pg_subscription_users and these should be able to dump subscriptions, > so you have at least one problem. But in system_views.sql we give grant on subconninfo column and pg_dump required superuser privilege only for postgesql under 12 version. Old version pg_dump still works but require superuser for dump subscription. Efimkin Evgeny
Re: Special role for subscriptions
Hi! > Perhaps we would want something at database level different from GRANT > CREATE ON DATABASE, but only for subscriptions? How about 4 checks to create subscription for nonsuperuser? 1. Special role for create subscription 2. CREATE ON DATABASE privilege 3. INSERT, UPDATE, DELETE, TRUNCATE, REFERENCE privilege on target table 4. target table not in information_schema and pg_catalog Efimkin Evgeny
Re: Special role for subscriptions
Hi! > - If the user's permissions are later revoked, the subscription is unaffected. Now it work the same, if we revoke superuser, subscription is unaffected and replication still work Don't check grants in target database is very dangerous, i create publication with system tables(it's not difficult) select * from pg_publication_tables ; pubname | schemaname | tablename -++ pub | pg_catalog | pg_authid (1 row) After that i create subscription, in log i see that 2019-03-21 11:19:50.863 MSK [58599] LOG: logical replication table synchronization worker for subscription "sub_nosuper", table "pg_authid" has started 2019-03-21 11:19:51.039 MSK [58599] ERROR: null value in column "oid" violates not-null constraint 2019-03-21 11:19:51.039 MSK [58599] DETAIL: Failing row contains (null, pg_monitor, f, t, f, f, f, f, f, -1, null, null). 2019-03-21 11:19:51.039 MSK [58599] CONTEXT: COPY pg_authid, line 1: "pg_monitor f t f f f f f -1 \N \N" I think it's no problem use it to attack target server after some hack on publication side. Efimkin Evgeny
Re: Special role for subscriptions
Hi!
> Currently, user with pg_subscription_users can create subscription into any
> system table, can't they?
> We certainly need to change it to more secure way.
No, you can't add system tables to publication. In new patch i add privileges
checks on target table, non superuser can't create/refresh subscription if he
don't have INSERT, UPDATE, DELETE and TRUNCATE privileges.
Efimkin Evgeny
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 3f2f674a1a..c2c7241084 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -201,9 +201,10 @@
Subscriptions are dumped by pg_dump if the current user
- is a superuser. Otherwise a warning is written and subscriptions are
- skipped, because non-superusers cannot read all subscription information
- from the pg_subscription catalog.
+ is a superuser or member of role pg_subscription_users, other users should
+ use no-subscriptions because non-superusers cannot read
+ all subscription information from the pg_subscription
+ catalog.
@@ -522,7 +523,10 @@
- To create a subscription, the user must be a superuser.
+ To create a subscription, the user must be a superuser or be member of role
+ pg_subscription_users and have INSERT ,
+ UPDATE, DELETE, TRUNCATE
+ privileges on target table.
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 6106244d32..9c23a6280d 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -556,6 +556,10 @@ DROP ROLE doomed_role;
pg_read_all_stats and
pg_stat_scan_tables.
+
+ pg_subscription_users
+ Allow create/drop subscriptions and be owner of subscription. Read pg_subscription
+
@@ -579,6 +583,12 @@ DROP ROLE doomed_role;
other system information normally restricted to superusers.
+
+ The pg_subscription_users role are intended to allow
+ administrators trusted, but non-superuser, which are able to create/drop subscription.
+
+
+
Care should be taken when granting these roles to ensure they are only used where
needed and with the understanding that these roles grant access to privileged
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index afee2838cc..5483a65376 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -242,12 +242,16 @@ AddSubscriptionRelState(Oid subid, Oid relid, char state,
XLogRecPtr sublsn)
{
Relation rel;
+ Relation target_rel;
HeapTuple tup;
bool nulls[Natts_pg_subscription_rel];
Datum values[Natts_pg_subscription_rel];
+ AclResult aclresult;
+ AclMode aclmask;
LockSharedObject(SubscriptionRelationId, subid, 0, AccessShareLock);
+
rel = table_open(SubscriptionRelRelationId, RowExclusiveLock);
/* Try finding existing mapping. */
@@ -258,6 +262,14 @@ AddSubscriptionRelState(Oid subid, Oid relid, char state,
elog(ERROR, "subscription table %u in subscription %u already exists",
relid, subid);
+ /* Check permission on target table. */
+ aclmask = ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE;
+ target_rel = table_open(relid, NoLock);
+ aclresult = pg_class_aclcheck(RelationGetRelid(target_rel), GetUserId(), aclmask);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, get_relkind_objtype(target_rel->rd_rel->relkind),
+ RelationGetRelationName(target_rel));
+
/* Form the tuple. */
memset(values, 0, sizeof(values));
memset(nulls, false, sizeof(nulls));
@@ -278,6 +290,7 @@ AddSubscriptionRelState(Oid subid, Oid relid, char state,
/* Cleanup. */
table_close(rel, NoLock);
+ table_close(target_rel, NoLock);
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index d962648bc5..08e58295bd 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -939,9 +939,12 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
-- All columns of pg_subscription except subconninfo are readable.
REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+GRANT SELECT (tableoid, oid, subdbid, subname, subowner, subenabled,
+ subslotname, subsynccommit, subpublications)
ON pg_subscription TO public;
+GRANT SELECT (subconninfo)
+ON pg_subscription TO pg_subscription_users;
--
-- We have a few function definitions in here, too.
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index a60a15193a..e1555ed6eb 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -26,6 +26,7 @@
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/objectaddress.h"
+#include "catalog/pg_authid.h"
#include
Re: Special role for subscriptions
Hi! > I view that as the first step towards building a more granular privilege > system for subscription creation, and that was the second half of what I > was trying to say before- I do think there's value in having something > more granular than just "this role can create ANY subscription". As an > administrator, I might be fine with subscriptions to system X, but not > to system Y, for example. As long as we don't block off the ability to > build something finer grained in the future, then having the system role > to allow a given role to do create subscription seems fine to me. Do you mean something like `CREATE SERVER` with privileges for each server, which using in CREATE SUBSCRIPTION, very similar way used in foreign data wrapper? Efimkin Evgeny
Re: Special role for subscriptions
Hi!
Thanks for comments!
> Just letting the superuser decide who gets to create subscriptions
> seems good enough from here.
I've prepare patch with new system role, i'm not sure about name, called it
"pg_subscription_users".
In that patch we don't check permissions on target tables, i don't know, should
we check it?
Efimkin Evgeny
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 3f2f674a1a..0f7f60fcd0 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -201,9 +201,10 @@
Subscriptions are dumped by pg_dump if the current user
- is a superuser. Otherwise a warning is written and subscriptions are
- skipped, because non-superusers cannot read all subscription information
- from the pg_subscription catalog.
+ is a superuser or member of role pg_subscription_users, other users should
+ use no-subscriptions because non-superusers cannot read
+ all subscription information from the pg_subscription
+ catalog.
@@ -522,7 +523,8 @@
- To create a subscription, the user must be a superuser.
+ To create a subscription, the user must be a superuser or be member of role
+ pg_subscription_users.
diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
index 6106244d32..9c23a6280d 100644
--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -556,6 +556,10 @@ DROP ROLE doomed_role;
pg_read_all_stats and
pg_stat_scan_tables.
+
+ pg_subscription_users
+ Allow create/drop subscriptions and be owner of subscription. Read pg_subscription
+
@@ -579,6 +583,12 @@ DROP ROLE doomed_role;
other system information normally restricted to superusers.
+
+ The pg_subscription_users role are intended to allow
+ administrators trusted, but non-superuser, which are able to create/drop subscription.
+
+
+
Care should be taken when granting these roles to ensure they are only used where
needed and with the understanding that these roles grant access to privileged
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7723f01327..56bf04631e 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -939,9 +939,12 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
-- All columns of pg_subscription except subconninfo are readable.
REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+GRANT SELECT (tableoid, oid, subdbid, subname, subowner, subenabled,
+ subslotname, subsynccommit, subpublications)
ON pg_subscription TO public;
+GRANT SELECT (subconninfo)
+ON pg_subscription TO pg_subscription_users;
--
-- We have a few function definitions in here, too.
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index a60a15193a..e1555ed6eb 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -26,6 +26,7 @@
#include "catalog/namespace.h"
#include "catalog/objectaccess.h"
#include "catalog/objectaddress.h"
+#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "catalog/pg_subscription.h"
#include "catalog/pg_subscription_rel.h"
@@ -342,10 +343,10 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
if (create_slot)
PreventInTransactionBlock(isTopLevel, "CREATE SUBSCRIPTION ... WITH (create_slot = true)");
- if (!superuser())
+ if (!is_member_of_role(GetUserId(), DEFAULT_ROLE_SUBSCRIPTION_USERS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to create subscriptions";
+ (errmsg("must be member of role \"pg_subscription_users\" to create subscriptions";
rel = table_open(SubscriptionRelationId, RowExclusiveLock);
@@ -1035,12 +1036,12 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
NameStr(form->subname));
/* New owner must be a superuser */
- if (!superuser_arg(newOwnerId))
+ if (!is_member_of_role(newOwnerId, DEFAULT_ROLE_SUBSCRIPTION_USERS))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to change owner of subscription \"%s\"",
NameStr(form->subname)),
- errhint("The owner of a subscription must be a superuser.")));
+ errhint("The owner of a subscription must be member of role \"pg_subscription_users\".")));
form->subowner = newOwnerId;
CatalogTupleUpdate(rel, >t_self, tup);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4c98ae4d7f..8751801b89 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4111,7 +4111,7 @@ getSubscriptions(Archive *fout)
if (dopt->no_subscriptions || fout->remoteVersion < 10)
return;
- if
Re: Special role for subscriptions
Hi! I think it's good idea to able create subscription for database owner, but owner do not have permission on all tables in database. At the begging Stephen Frost said about table filter at subscription side. https://www.postgresql.org/message-id/20181106215244.GH18594%40tamriel.snowman.net I can add additional check in create subscription for database owner. Efimkin Evgeny
Re: Special role for subscriptions
Hi!
> * What are the most important use cases here? Are we just trying to
> avoid the unnecessary use of superuser, or is there a real use case for
> subscribing to a subset of a publication?
For instance in target database we do not have permission on some table used in
publication,
but we still CREATE SUBSCRIPTION for owned tables.
> * What are all the reasons CREATE SUBSCRIPTION currently requires
> superuser?
I'm not sure, but it seems like only superuser have rights on all tables. I
can't find any restrictions.
> * Is the original idea of a special role still viable?
yes, i wrote simple patch. Role create externally, but it can be system role.
Efimkin Evgeny
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9021463a4c..31b5b9af8c 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -322,6 +322,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
charoriginname[NAMEDATALEN];
boolcreate_slot;
List *publications;
+ Oid role;
/*
* Parse and check options.
@@ -341,11 +342,13 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
*/
if (create_slot)
PreventInTransactionBlock(isTopLevel, "CREATE SUBSCRIPTION ... WITH (create_slot = true)");
-
- if (!superuser())
+ role = get_role_oid("pg_subsciption_users", true);
+ if (!is_member_of_role(GetUserId(), role))
+ {
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-(errmsg("must be superuser to create subscriptions";
+(errmsg("must be pg_subsciption_users to create subscriptions";
+ }
rel = heap_open(SubscriptionRelationId, RowExclusiveLock);
@@ -1023,6 +1026,7 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel)
static void
AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
{
+ Oid role;
Form_pg_subscription form;
form = (Form_pg_subscription) GETSTRUCT(tup);
@@ -1034,13 +1038,16 @@ AlterSubscriptionOwner_internal(Relation rel, HeapTuple tup, Oid newOwnerId)
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION,
NameStr(form->subname));
+ role = get_role_oid("pg_subsciption_users", true);
/* New owner must be a superuser */
- if (!superuser_arg(newOwnerId))
+ if (!is_member_of_role(GetUserId(), role))
+ {
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied to change owner of subscription \"%s\"",
NameStr(form->subname)),
-errhint("The owner of a subscription must be a superuser.")));
+errhint("The owner of a subscription must be a pg_subsciption_users.")));
+ }
form->subowner = newOwnerId;
CatalogTupleUpdate(rel, >t_self, tup);
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 38ae1b9ab8..fa5d343993 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -754,6 +754,8 @@ copy_table(Relation rel)
CopyState cstate;
List *attnamelist;
ParseState *pstate;
+ AclResult aclresult;
+ AclMode aclmask;
/* Get the publisher relation info. */
fetch_remote_table_info(get_namespace_name(RelationGetNamespace(rel)),
@@ -770,6 +772,13 @@ copy_table(Relation rel)
initStringInfo();
appendStringInfo(, "COPY %s TO STDOUT",
quote_qualified_identifier(lrel.nspname, lrel.relname));
+ aclmask = ACL_INSERT | ACL_UPDATE | ACL_DELETE | ACL_TRUNCATE;
+ aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
+ aclmask);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
+ RelationGetRelationName(rel));
+
res = walrcv_exec(wrconn, cmd.data, 0, NULL);
pfree(cmd.data);
if (res->status != WALRCV_OK_COPY_OUT)
Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Hi! Thank you for comments, I’ll fix all inconsistencies in documentation. > I don't understand this requirement. There are a bunch of > password-less, still secured authentication that Postgres can use > depending on the situations. To name one: peer. So this concept > design looks rather broken to me. It does look a bit awkward, maybe — unless you account for details. For instance, you mention peer auth. Suppose we allow it, and there is nothing stopping the user from gaining superuser privileges by connecting locally. Also, same requirements are in effect for FDW. > Access to pg_subcription is still forbidden to non superusers, even > with the patch. Shouldn't a user who has CREATE rights be able to > dump his/her subscriptions? > > There is zero documentation about pg_user_subscriptions. > Why? Cannot you store in catalogs the tables which can be used with a > subscription, and then reuse the table list when connecting later. That was my first thought too, but that approach made the implementation far too confusing. For insance, it requred an introduction of a new status for sync worker and a new statement to enable sync on particular tables. > I find the concept behind this patch a bit confusing, and honestly I > don't think that adding an extra per-relation filtering on the target > server is a concept which compiles well with the existing logical > replication because it is already possible to assign a subset of > tables on the source, and linking a subscription to a publication means > that this subset of tables will be used. Something which is more > simple, and that we could consider is to lower the requirement for > subscription creation to database owners, still this means that we give > a mean to any database owner to trigger connections remote instances > and spawn extra processes. This deserves more discussion, and there > is the dump case which is not really covered. Efimkin Evgeny
Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Hi!
Thanks for comments!
I fixed build and return lines about subscription apply process in doc
Efimkin Evgeny
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 3f2f674a1a..5d211646bf 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -522,7 +522,8 @@
- To create a subscription, the user must be a superuser.
+ To add tables to a subscription, the user must have ownership rights on the
+ table.
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 6dfb2e4d3e..f0a368f90c 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -24,6 +24,9 @@ PostgreSQL documentation
ALTER SUBSCRIPTION name CONNECTION 'conninfo'
ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] [ WITH ( set_publication_option [= value] [, ... ] ) ]
ALTER SUBSCRIPTION name REFRESH PUBLICATION [ WITH ( refresh_option [= value] [, ... ] ) ]
+ALTER SUBSCRIPTION name ADD TABLE table_name [, ...]
+ALTER SUBSCRIPTION name SET TABLE table_name [, ...]
+ALTER SUBSCRIPTION name DROP TABLE table_name [, ...]
ALTER SUBSCRIPTION name ENABLE
ALTER SUBSCRIPTION name DISABLE
ALTER SUBSCRIPTION name SET ( subscription_parameter [= value] [, ... ] )
@@ -44,9 +47,7 @@ ALTER SUBSCRIPTION name RENAME TO <
You must own the subscription to use ALTER SUBSCRIPTION.
To alter the owner, you must also be a direct or indirect member of the
- new owning role. The new owner has to be a superuser.
- (Currently, all subscription owners must be superusers, so the owner checks
- will be bypassed in practice. But this might change in the future.)
+ new owning role.
@@ -137,6 +138,35 @@ ALTER SUBSCRIPTION name RENAME TO <
+
+ADD TABLE table_name
+
+
+ The ADD TABLE clauses will add new table in subscription, table must be
+ present in publication.
+
+
+
+
+
+SET TABLE table_name
+
+
+ The SET TABLE clause will replace the list of tables in
+ the publication with the specified one.
+
+
+
+
+
+DROP TABLE table_name
+
+
+ The DROP TABLE clauses will remove table from subscription.
+
+
+
+
ENABLE
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 1a90c244fb..511fec53a1 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -24,6 +24,7 @@ PostgreSQL documentation
CREATE SUBSCRIPTION subscription_name
CONNECTION 'conninfo'
PUBLICATION publication_name [, ...]
+[ FOR TABLE table_name [, ...]
[ WITH ( subscription_parameter [= value] [, ... ] ) ]
@@ -88,6 +89,16 @@ CREATE SUBSCRIPTION subscription_name
+
+FOR TABLE
+
+
+ Specifies a list of tables to add to the subscription. All tables listed in a clause
+ must be present in the publication.
+
+
+
+
WITH ( subscription_parameter [= value] [, ... ] )
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 3e229c693c..c23d6eb0bd 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -906,6 +906,27 @@ CREATE VIEW pg_stat_progress_vacuum AS
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
+CREATE VIEW pg_user_subscriptions AS
+SELECT
+S.oid,
+ S.subdbid,
+S.subnameAS subname,
+CASE WHEN S.subowner = 0 THEN
+'public'
+ELSE
+A.rolname
+END AS usename,
+ S.subenabled,
+CASE WHEN (S.subowner <> 0 AND A.rolname = current_user)
+OR (SELECT rolsuper FROM pg_authid WHERE rolname = current_user)
+THEN S.subconninfo
+ ELSE NULL END AS subconninfo,
+ S.subslotname,
+ S.subsynccommit,
+ S.subpublications
+FROM pg_subscription S
+LEFT JOIN pg_authid A ON (A.oid = S.subowner);
+
CREATE VIEW pg_user_mappings AS
SELECT
U.oid AS umid,
@@ -938,7 +959,8 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
-- All columns of pg_subscription except subconninfo are readable.
REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+GRANT SELECT (tableoid, oid, subdbid, subname,
+ subowner, subenabled, subslotname, subpublications, subsynccommit)
ON pg_subscription TO public;
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index a60a15193a..57eadf6f83 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -30,6 +30,7 @@
#include "catalog/pg_subscription.h"
#include
Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Hi! Thanks for comments
1. fixed
2. in non-superuser we have to use authorization, and FOR table clause, i don't
known how merge both files.
3. fixed
4. fixed and run pgindent
5. add some new cases in regression test
Efimkin Evgeny
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 3f2f674a1a..ff8a65a3e4 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -522,12 +522,8 @@
- To create a subscription, the user must be a superuser.
-
-
-
- The subscription apply process will run in the local database with the
- privileges of a superuser.
+ To add tables to a subscription, the user must have ownership rights on the
+ table.
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 6dfb2e4d3e..f0a368f90c 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -24,6 +24,9 @@ PostgreSQL documentation
ALTER SUBSCRIPTION name CONNECTION 'conninfo'
ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] [ WITH ( set_publication_option [= value] [, ... ] ) ]
ALTER SUBSCRIPTION name REFRESH PUBLICATION [ WITH ( refresh_option [= value] [, ... ] ) ]
+ALTER SUBSCRIPTION name ADD TABLE table_name [, ...]
+ALTER SUBSCRIPTION name SET TABLE table_name [, ...]
+ALTER SUBSCRIPTION name DROP TABLE table_name [, ...]
ALTER SUBSCRIPTION name ENABLE
ALTER SUBSCRIPTION name DISABLE
ALTER SUBSCRIPTION name SET ( subscription_parameter [= value] [, ... ] )
@@ -44,9 +47,7 @@ ALTER SUBSCRIPTION name RENAME TO <
You must own the subscription to use ALTER SUBSCRIPTION.
To alter the owner, you must also be a direct or indirect member of the
- new owning role. The new owner has to be a superuser.
- (Currently, all subscription owners must be superusers, so the owner checks
- will be bypassed in practice. But this might change in the future.)
+ new owning role.
@@ -137,6 +138,35 @@ ALTER SUBSCRIPTION name RENAME TO <
+
+ADD TABLE table_name
+
+
+ The ADD TABLE clauses will add new table in subscription, table must be
+ present in publication.
+
+
+
+
+
+SET TABLE table_name
+
+
+ The SET TABLE clause will replace the list of tables in
+ the publication with the specified one.
+
+
+
+
+
+DROP TABLE table_name
+
+
+ The DROP TABLE clauses will remove table from subscription.
+
+
+
+
ENABLE
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 1a90c244fb..511fec53a1 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -24,6 +24,7 @@ PostgreSQL documentation
CREATE SUBSCRIPTION subscription_name
CONNECTION 'conninfo'
PUBLICATION publication_name [, ...]
+[ FOR TABLE table_name [, ...]
[ WITH ( subscription_parameter [= value] [, ... ] ) ]
@@ -88,6 +89,16 @@ CREATE SUBSCRIPTION subscription_name
+
+FOR TABLE
+
+
+ Specifies a list of tables to add to the subscription. All tables listed in a clause
+ must be present in the publication.
+
+
+
+
WITH ( subscription_parameter [= value] [, ... ] )
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 3e229c693c..c23d6eb0bd 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -906,6 +906,27 @@ CREATE VIEW pg_stat_progress_vacuum AS
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
+CREATE VIEW pg_user_subscriptions AS
+SELECT
+S.oid,
+ S.subdbid,
+S.subnameAS subname,
+CASE WHEN S.subowner = 0 THEN
+'public'
+ELSE
+A.rolname
+END AS usename,
+ S.subenabled,
+CASE WHEN (S.subowner <> 0 AND A.rolname = current_user)
+OR (SELECT rolsuper FROM pg_authid WHERE rolname = current_user)
+THEN S.subconninfo
+ ELSE NULL END AS subconninfo,
+ S.subslotname,
+ S.subsynccommit,
+ S.subpublications
+FROM pg_subscription S
+LEFT JOIN pg_authid A ON (A.oid = S.subowner);
+
CREATE VIEW pg_user_mappings AS
SELECT
U.oid AS umid,
@@ -938,7 +959,8 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
-- All columns of pg_subscription except subconninfo are readable.
REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+GRANT SELECT (tableoid, oid, subdbid, subname,
+ subowner, subenabled, subslotname, subpublications, subsynccommit)
ON pg_subscription TO public;
diff --git a/src/backend/commands/subscriptioncmds.c
Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Hi!
1. done
2. rename to pg_user_subscriptions
3. by pg_dump, i checked upgrade from 10 to 12devel, it's work fine
4. done
5. done
6. I took it from AlterSubscription_refresh, in that function no any free()
7. done
Ефимкин Евгений
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 3f2f674a1a..ff8a65a3e4 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -522,12 +522,8 @@
- To create a subscription, the user must be a superuser.
-
-
-
- The subscription apply process will run in the local database with the
- privileges of a superuser.
+ To add tables to a subscription, the user must have ownership rights on the
+ table.
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 6dfb2e4d3e..f0a368f90c 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -24,6 +24,9 @@ PostgreSQL documentation
ALTER SUBSCRIPTION name CONNECTION 'conninfo'
ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] [ WITH ( set_publication_option [= value] [, ... ] ) ]
ALTER SUBSCRIPTION name REFRESH PUBLICATION [ WITH ( refresh_option [= value] [, ... ] ) ]
+ALTER SUBSCRIPTION name ADD TABLE table_name [, ...]
+ALTER SUBSCRIPTION name SET TABLE table_name [, ...]
+ALTER SUBSCRIPTION name DROP TABLE table_name [, ...]
ALTER SUBSCRIPTION name ENABLE
ALTER SUBSCRIPTION name DISABLE
ALTER SUBSCRIPTION name SET ( subscription_parameter [= value] [, ... ] )
@@ -44,9 +47,7 @@ ALTER SUBSCRIPTION name RENAME TO <
You must own the subscription to use ALTER SUBSCRIPTION.
To alter the owner, you must also be a direct or indirect member of the
- new owning role. The new owner has to be a superuser.
- (Currently, all subscription owners must be superusers, so the owner checks
- will be bypassed in practice. But this might change in the future.)
+ new owning role.
@@ -137,6 +138,35 @@ ALTER SUBSCRIPTION name RENAME TO <
+
+ADD TABLE table_name
+
+
+ The ADD TABLE clauses will add new table in subscription, table must be
+ present in publication.
+
+
+
+
+
+SET TABLE table_name
+
+
+ The SET TABLE clause will replace the list of tables in
+ the publication with the specified one.
+
+
+
+
+
+DROP TABLE table_name
+
+
+ The DROP TABLE clauses will remove table from subscription.
+
+
+
+
ENABLE
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 1a90c244fb..04af4e27c7 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -24,6 +24,7 @@ PostgreSQL documentation
CREATE SUBSCRIPTION subscription_name
CONNECTION 'conninfo'
PUBLICATION publication_name [, ...]
+[ FOR TABLE table_name [, ...]
[ WITH ( subscription_parameter [= value] [, ... ] ) ]
@@ -88,6 +89,16 @@ CREATE SUBSCRIPTION subscription_name
+
+FOR TABLE
+
+
+ Specifies a list of tables to add to the subscription. All tables listed in clause
+ must be present in publication.
+
+
+
+
WITH ( subscription_parameter [= value] [, ... ] )
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index f4d9e9daf7..6ec6b24eb1 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -904,6 +904,27 @@ CREATE VIEW pg_stat_progress_vacuum AS
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
+CREATE VIEW pg_user_subscriptions AS
+SELECT
+S.oid,
+ S.subdbid,
+S.subnameAS subname,
+CASE WHEN S.subowner = 0 THEN
+'public'
+ELSE
+A.rolname
+END AS usename,
+ S.subenabled,
+CASE WHEN (S.subowner <> 0 AND A.rolname = current_user)
+OR (SELECT rolsuper FROM pg_authid WHERE rolname = current_user)
+THEN S.subconninfo
+ ELSE NULL END AS subconninfo,
+ S.subslotname,
+ S.subsynccommit,
+ S.subpublications
+FROM pg_subscription S
+LEFT JOIN pg_authid A ON (A.oid = S.subowner);
+
CREATE VIEW pg_user_mappings AS
SELECT
U.oid AS umid,
@@ -936,7 +957,8 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
-- All columns of pg_subscription except subconninfo are readable.
REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+GRANT SELECT (tableoid, oid, subdbid, subname,
+ subowner, subenabled, subslotname, subpublications, subsynccommit)
ON pg_subscription TO public;
diff --git a/src/backend/commands/subscriptioncmds.c
Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Hello!
In latest patch i removed `FOR ALL TABLES` clause and `alltables` parameter,
now it's look more simple.
Add new system view pg_user_subscirption to allow non-superuser use pg_dump and
select addition column from pg_subscrption
Changes docs.
Thanks!
Ефимкин Евгений
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index 3f2f674a1a..ff8a65a3e4 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -522,12 +522,8 @@
- To create a subscription, the user must be a superuser.
-
-
-
- The subscription apply process will run in the local database with the
- privileges of a superuser.
+ To add tables to a subscription, the user must have ownership rights on the
+ table.
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index 6dfb2e4d3e..f0a368f90c 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -24,6 +24,9 @@ PostgreSQL documentation
ALTER SUBSCRIPTION name CONNECTION 'conninfo'
ALTER SUBSCRIPTION name SET PUBLICATION publication_name [, ...] [ WITH ( set_publication_option [= value] [, ... ] ) ]
ALTER SUBSCRIPTION name REFRESH PUBLICATION [ WITH ( refresh_option [= value] [, ... ] ) ]
+ALTER SUBSCRIPTION name ADD TABLE table_name [, ...]
+ALTER SUBSCRIPTION name SET TABLE table_name [, ...]
+ALTER SUBSCRIPTION name DROP TABLE table_name [, ...]
ALTER SUBSCRIPTION name ENABLE
ALTER SUBSCRIPTION name DISABLE
ALTER SUBSCRIPTION name SET ( subscription_parameter [= value] [, ... ] )
@@ -44,9 +47,7 @@ ALTER SUBSCRIPTION name RENAME TO <
You must own the subscription to use ALTER SUBSCRIPTION.
To alter the owner, you must also be a direct or indirect member of the
- new owning role. The new owner has to be a superuser.
- (Currently, all subscription owners must be superusers, so the owner checks
- will be bypassed in practice. But this might change in the future.)
+ new owning role.
@@ -137,6 +138,35 @@ ALTER SUBSCRIPTION name RENAME TO <
+
+ADD TABLE table_name
+
+
+ The ADD TABLE clauses will add new table in subscription, table must be
+ present in publication.
+
+
+
+
+
+SET TABLE table_name
+
+
+ The SET TABLE clause will replace the list of tables in
+ the publication with the specified one.
+
+
+
+
+
+DROP TABLE table_name
+
+
+ The DROP TABLE clauses will remove table from subscription.
+
+
+
+
ENABLE
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index 1a90c244fb..04af4e27c7 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -24,6 +24,7 @@ PostgreSQL documentation
CREATE SUBSCRIPTION subscription_name
CONNECTION 'conninfo'
PUBLICATION publication_name [, ...]
+[ FOR TABLE table_name [, ...]
[ WITH ( subscription_parameter [= value] [, ... ] ) ]
@@ -88,6 +89,16 @@ CREATE SUBSCRIPTION subscription_name
+
+FOR TABLE
+
+
+ Specifies a list of tables to add to the subscription. All tables listed in clause
+ must be present in publication.
+
+
+
+
WITH ( subscription_parameter [= value] [, ... ] )
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 5253837b54..a23b080cfe 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -904,6 +904,27 @@ CREATE VIEW pg_stat_progress_vacuum AS
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
+CREATE VIEW pg_user_subscription AS
+SELECT
+S.oid,
+ S.subdbid,
+S.subnameAS subname,
+CASE WHEN S.subowner = 0 THEN
+'public'
+ELSE
+A.rolname
+END AS usename,
+ S.subenabled,
+CASE WHEN (S.subowner <> 0 AND A.rolname = current_user)
+OR (SELECT rolsuper FROM pg_authid WHERE rolname = current_user)
+THEN S.subconninfo
+ ELSE NULL END AS subconninfo,
+ S.subslotname,
+ S.subsynccommit,
+ S.subpublications
+FROM pg_subscription S
+LEFT JOIN pg_authid A ON (A.oid = S.subowner);
+
CREATE VIEW pg_user_mappings AS
SELECT
U.oid AS umid,
@@ -936,7 +957,8 @@ REVOKE ALL ON pg_replication_origin_status FROM public;
-- All columns of pg_subscription except subconninfo are readable.
REVOKE ALL ON pg_subscription FROM public;
-GRANT SELECT (subdbid, subname, subowner, subenabled, subslotname, subpublications)
+GRANT SELECT (tableoid, oid, subdbid, subname,
+ subowner, subenabled, subslotname, subpublications, subsynccommit)
ON pg_subscription TO public;
diff --git
Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Hello!
Thank you for questions!
> I've got few questions:
> 1. How will the subscription work for inherited tables? Do we need tests for
> that?
For subscription created with `FOR TABLE` we can't support inherit tables
because subscriber don't know anything about inherit. In new patch i remove
`ONLY` for `FOR TABLE` in subscription related statements
> 2. ALTER PUBLICATION has ADD\DROP and SET. Should we add SET too? Or is there
> a reason not to do that?
Added it in new patch
> 3. Message "Must be superuser to create FOR ALL TABLES subscriptions" seems a
> bit strange to me. Also, this literal is embedded into translations. I do not
> know how we deal with it, how do we deal for example with "måste vara
> superanvändare för att skapa prenumerationer" or "для создания подписок нужно
> быть суперпользователем"? Where do we insert FOR ALL TABLES?
I add hint `Use CREATE SUBSCRIPTION ... FOR TABLE ...`
> 4. How does default behavior differ from FOR ALL TABLES?
The same with default implementation
> 5. Can we alter subscription FOR ALL TABLES? Drop some tables out of the
> subscription?
For subscriptions created with `FOR ALL TABLES` (default), you can't change
subscribed tables by `ALTER SUBSCRIPTION ADD/DROP` table, you should use `ALTER
SUBSCRIPTION REFRESH PUBLICATION`
And i don't know how do export for user created subscriptions, because now non
superuser can't select subconninfo column
03.12.2018, 09:06, "Andrey Borodin" :
> Hi, Evgeniy!
>
> Thanks for working on the feature.
>> 28 нояб. 2018 г., в 21:41, Evgeniy Efimkin
>> написал(а):
>>
>> Hello!
>> I wrote some tests(it's just 01_rep_changes.pl but for non superuser) and
>> fix `DROP TABLE` from subscription. Now old and new tests pass.
>>
>> 22.11.2018, 16:23, "Evgeniy Efimkin" :
>>> Hello!
>>> New draft attached with filtering table in subscription (ADD/DROP) and
>>> allow non-superusers use` CREATE SUBSCRIPTION` for own tables.
>
> I've looked into the patch. The code looks good and coherent to nearby code.
> There are no docs, obviously, there is WiP.
>
> I've got few questions:
> 1. How will the subscription work for inherited tables? Do we need tests for
> that?
> 2. ALTER PUBLICATION has ADD\DROP and SET. Should we add SET too? Or is there
> a reason not to do that?
> 3. Message "Must be superuser to create FOR ALL TABLES subscriptions" seems a
> bit strange to me. Also, this literal is embedded into translations. I do not
> know how we deal with it, how do we deal for example with "måste vara
> superanvändare för att skapa prenumerationer" or "для создания подписок нужно
> быть суперпользователем"? Where do we insert FOR ALL TABLES?
> 4. How does default behavior differ from FOR ALL TABLES?
> 5. Can we alter subscription FOR ALL TABLES? Drop some tables out of the
> subscription?
>
> Best regards, Andrey Borodin.
Ефимкин Евгений
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index e136aa6a0b..0782dd40f0 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -38,6 +38,7 @@
#include "utils/syscache.h"
+
static List *textarray_to_stringlist(ArrayType *textarray);
/*
@@ -70,6 +71,7 @@ GetSubscription(Oid subid, bool missing_ok)
sub->name = pstrdup(NameStr(subform->subname));
sub->owner = subform->subowner;
sub->enabled = subform->subenabled;
+ sub->alltables = subform->suballtables;
/* Get conninfo */
datum = SysCacheGetAttr(SUBSCRIPTIONOID,
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 9021463a4c..58f71a227c 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -30,6 +30,7 @@
#include "catalog/pg_subscription.h"
#include "catalog/pg_subscription_rel.h"
+#include "commands/dbcommands.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
#include "commands/subscriptioncmds.h"
@@ -322,6 +323,22 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
char originname[NAMEDATALEN];
bool create_slot;
List *publications;
+ AclResult aclresult;
+ bool alltables;
+
+ alltables = !stmt->tables;
+ /* FOR ALL TABLES requires superuser */
+ if (alltables && !superuser())
+ ereport(ERROR,
+(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("must be superuser to create subscriptions"),
+ errhint("Use CREATE SUBSCRIPTION ... FOR TABLE ...";
+
+ /* must have CREATE privilege on database */
+ aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(), ACL_CREATE);
+ if (
Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Hello! I wrote some tests(it's just 01_rep_changes.pl but for non superuser) and fix `DROP TABLE` from subscription. Now old and new tests pass. 22.11.2018, 16:23, "Evgeniy Efimkin" : > Hello! > New draft attached with filtering table in subscription (ADD/DROP) and allow > non-superusers use` CREATE SUBSCRIPTION` for own tables. > > 14.11.2018, 18:10, "Evgeniy Efimkin" : >> Hello! >> I started work on patch (draft attached). Draft has changes related only to >> `CREATE SUBSCRIPTION`. >> I also introduce a new status (DEFFERED) for tables in `FOR TABLE` clause >> (but not in publication). >> New column in pg_subscription (suballtables) will be used in `REFRESH` >> clause >> >> 09.11.2018, 15:24, "Evgeniy Efimkin" : >>> Hi! >>> In order to support create subscription from non-superuser, we need to >>> make it possible to choose tables on the subscriber side: >>> 1. add `FOR TABLE` clause in `CREATE SUBSCRIPTION`: >>> ``` >>> CREATE SUBSCRIPTION subscription_name >>> CONNECTION 'conninfo' >>> PUBLICATION publication_name [, ...] >>> [ FOR TABLE [ ONLY ] table_name [ * ] [, ...]| FOR ALL TABLES >>> ] >>> [ WITH ( subscription_parameter [= value] [, ... ] ) ] >>> ``` >>> ... where `FOR ALL TABLES` is only allowed for superuser. >>> and table list in `FOR TABLES` clause will be stored in >>> pg_subscription_rel table (maybe another place?) >>> >>> 2. Each subscription should have "all tables" attribute. >>> For example via a new column in pg_subscription "suballtables". >>> >>> 3. Add `ALTER SUBSCRIPTION (ADD TABLE | DROP TABLE)`: >>> ``` >>> ALTER SUBSCRIPTION subscription_name ADD TABLE [ ONLY ] >>> table_name [WITH copy_data]; >>> ALTER SUBSCRIPTION subscription_name DROP TABLE [ ONLY ] >>> table_name; >>> ``` >>> 4. On `ALTER SUBSCRIPTION REFRESH PUBLICATION` should check if >>> table owner equals subscription owner. The check is ommited if subscription >>> owner is superuser. >>> 5. If superuser calls `ALTER SUBSCRIPTION REFRESH PUBLICATION` on >>> subscription with table list and non-superuser owner, we should filter >>> tables which owner is not subscription's owner or maybe we need to raise >>> error? >>> >>> What do you think about it? Any objections? >>> >>> 07.11.2018, 00:52, "Stephen Frost" : >>>> Greetings, >>>> >>>> * Evgeniy Efimkin (efim...@yandex-team.ru) wrote: >>>>> As a first step I suggest we allow CREATE SUBSCRIPTION for table >>>>> owner only. >>>> >>>> That's a nice idea but seems like we would want to have a way to filter >>>> what tables a subscription follows then..? Just failing if the >>>> publication publishes tables that we don't have access to or are not the >>>> owner of seems like a poor solution.. >>>> >>>> Thanks! >>>> >>>> Stephen >>> >>> >>> Ефимкин Евгений >> >> >> Ефимкин Евгений > > > Ефимкин Евгений Ефимкин Евгений diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c index e136aa6a0b..5d7841f296 100644 --- a/src/backend/catalog/pg_subscription.c +++ b/src/backend/catalog/pg_subscription.c @@ -70,6 +70,7 @@ GetSubscription(Oid subid, bool missing_ok) sub->name = pstrdup(NameStr(subform->subname)); sub->owner = subform->subowner; sub->enabled = subform->subenabled; + sub->alltables = subform->suballtables; /* Get conninfo */ datum = SysCacheGetAttr(SUBSCRIPTIONOID, diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 9021463a4c..e7024d0804 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -30,6 +30,7 @@ #include "catalog/pg_subscription.h" #include "catalog/pg_subscription_rel.h" +#include "commands/dbcommands.h" #include "commands/defrem.h" #include "commands/event_trigger.h" #include "commands/subscriptioncmds.h" @@ -322,6 +323,21 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel) char originname[NAMEDATALEN]; bool create_slot; List *publicatio
Re: [WIP] CREATE SUBSCRIPTION with FOR TABLES clause (table filter)
Hello! New draft attached with filtering table in subscription (ADD/DROP) and allow non-superusers use` CREATE SUBSCRIPTION` for own tables. 14.11.2018, 18:10, "Evgeniy Efimkin" : > Hello! > I started work on patch (draft attached). Draft has changes related only to > `CREATE SUBSCRIPTION`. > I also introduce a new status (DEFFERED) for tables in `FOR TABLE` clause > (but not in publication). > New column in pg_subscription (suballtables) will be used in `REFRESH` clause > > 09.11.2018, 15:24, "Evgeniy Efimkin" : >> Hi! >> In order to support create subscription from non-superuser, we need to make >> it possible to choose tables on the subscriber side: >> 1. add `FOR TABLE` clause in `CREATE SUBSCRIPTION`: >> ``` >> CREATE SUBSCRIPTION subscription_name >> CONNECTION 'conninfo' >> PUBLICATION publication_name [, ...] >> [ FOR TABLE [ ONLY ] table_name [ * ] [, ...]| FOR ALL TABLES ] >> [ WITH ( subscription_parameter [= value] [, ... ] ) ] >> ``` >> ... where `FOR ALL TABLES` is only allowed for superuser. >> and table list in `FOR TABLES` clause will be stored in >> pg_subscription_rel table (maybe another place?) >> >> 2. Each subscription should have "all tables" attribute. >> For example via a new column in pg_subscription "suballtables". >> >> 3. Add `ALTER SUBSCRIPTION (ADD TABLE | DROP TABLE)`: >> ``` >> ALTER SUBSCRIPTION subscription_name ADD TABLE [ ONLY ] table_name >> [WITH copy_data]; >> ALTER SUBSCRIPTION subscription_name DROP TABLE [ ONLY ] table_name; >> ``` >> 4. On `ALTER SUBSCRIPTION REFRESH PUBLICATION` should check if >> table owner equals subscription owner. The check is ommited if subscription >> owner is superuser. >> 5. If superuser calls `ALTER SUBSCRIPTION REFRESH PUBLICATION` on >> subscription with table list and non-superuser owner, we should filter >> tables which owner is not subscription's owner or maybe we need to raise >> error? >> >> What do you think about it? Any objections? >> >> 07.11.2018, 00:52, "Stephen Frost" : >>> Greetings, >>> >>> * Evgeniy Efimkin (efim...@yandex-team.ru) wrote: >>>> As a first step I suggest we allow CREATE SUBSCRIPTION for table owner >>>> only. >>> >>> That's a nice idea but seems like we would want to have a way to filter >>> what tables a subscription follows then..? Just failing if the >>> publication publishes tables that we don't have access to or are not the >>> owner of seems like a poor solution.. >>> >>> Thanks! >>> >>> Stephen >> >> >> Ефимкин Евгений > > > Ефимкин Евгений Ефимкин Евгений diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c index e136aa6a0b..5d7841f296 100644 --- a/src/backend/catalog/pg_subscription.c +++ b/src/backend/catalog/pg_subscription.c @@ -70,6 +70,7 @@ GetSubscription(Oid subid, bool missing_ok) sub->name = pstrdup(NameStr(subform->subname)); sub->owner = subform->subowner; sub->enabled = subform->subenabled; + sub->alltables = subform->suballtables; /* Get conninfo */ datum = SysCacheGetAttr(SUBSCRIPTIONOID, diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 9021463a4c..4fe643c3bc 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -30,6 +30,7 @@ #include "catalog/pg_subscription.h" #include "catalog/pg_subscription_rel.h" +#include "commands/dbcommands.h" #include "commands/defrem.h" #include "commands/event_trigger.h" #include "commands/subscriptioncmds.h" @@ -322,6 +323,14 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel) char originname[NAMEDATALEN]; bool create_slot; List *publications; + AclResult aclresult; + bool alltables; + + /* must have CREATE privilege on database */ + aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(), ACL_CREATE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, OBJECT_DATABASE, + get_database_name(MyDatabaseId)); /* * Parse and check options. @@ -341,11 +350,13 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel) */ if (create_slot) PreventInTransactionBlock(isTopLevel, "CREATE SUBSCRIPTION ... WITH (create_slot = true)"); - - if (!superuser()) + alltables = !stmt->t
Re: [WIP] Special role for subscriptions
Hello!
I started work on patch (draft attached). Draft has changes related only to
`CREATE SUBSCRIPTION`.
I also introduce a new status (DEFFERED) for tables in `FOR TABLE` clause (but
not in publication).
New column in pg_subscription (suballtables) will be used in `REFRESH` clause
09.11.2018, 15:24, "Evgeniy Efimkin" :
> Hi!
> In order to support create subscription from non-superuser, we need to make
> it possible to choose tables on the subscriber side:
> 1. add `FOR TABLE` clause in `CREATE SUBSCRIPTION`:
> ```
> CREATE SUBSCRIPTION subscription_name
> CONNECTION 'conninfo'
> PUBLICATION publication_name [, ...]
> [ FOR TABLE [ ONLY ] table_name [ * ] [, ...]| FOR ALL TABLES ]
> [ WITH ( subscription_parameter [= value] [, ... ] ) ]
> ```
> ... where `FOR ALL TABLES` is only allowed for superuser.
> and table list in `FOR TABLES` clause will be stored in
> pg_subscription_rel table (maybe another place?)
>
> 2. Each subscription should have "all tables" attribute.
> For example via a new column in pg_subscription "suballtables".
>
> 3. Add `ALTER SUBSCRIPTION (ADD TABLE | DROP TABLE)`:
> ```
> ALTER SUBSCRIPTION subscription_name ADD TABLE [ ONLY ] table_name
> [WITH copy_data];
> ALTER SUBSCRIPTION subscription_name DROP TABLE [ ONLY ] table_name;
> ```
> 4. On `ALTER SUBSCRIPTION REFRESH PUBLICATION` should check if
> table owner equals subscription owner. The check is ommited if subscription
> owner is superuser.
> 5. If superuser calls `ALTER SUBSCRIPTION REFRESH PUBLICATION` on
> subscription with table list and non-superuser owner, we should filter tables
> which owner is not subscription's owner or maybe we need to raise error?
>
> What do you think about it? Any objections?
>
> 07.11.2018, 00:52, "Stephen Frost" :
>> Greetings,
>>
>> * Evgeniy Efimkin (efim...@yandex-team.ru) wrote:
>>> As a first step I suggest we allow CREATE SUBSCRIPTION for table owner
>>> only.
>>
>> That's a nice idea but seems like we would want to have a way to filter
>> what tables a subscription follows then..? Just failing if the
>> publication publishes tables that we don't have access to or are not the
>> owner of seems like a poor solution..
>>
>> Thanks!
>>
>> Stephen
>
>
> Ефимкин Евгений
Ефимкин Евгений
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index f138e61a8d..5452bd6a55 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -29,6 +29,7 @@
#include "catalog/pg_subscription.h"
#include "catalog/pg_subscription_rel.h"
+#include "commands/dbcommands.h"
#include "commands/defrem.h"
#include "commands/event_trigger.h"
#include "commands/subscriptioncmds.h"
@@ -321,6 +322,14 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
char originname[NAMEDATALEN];
bool create_slot;
List *publications;
+ AclResult aclresult;
+ bool alltables;
+
+ /* must have CREATE privilege on database */
+ aclresult = pg_database_aclcheck(MyDatabaseId, GetUserId(), ACL_CREATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, OBJECT_DATABASE,
+ get_database_name(MyDatabaseId));
/*
* Parse and check options.
@@ -340,11 +349,13 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
*/
if (create_slot)
PreventInTransactionBlock(isTopLevel, "CREATE SUBSCRIPTION ... WITH (create_slot = true)");
-
- if (!superuser())
+ alltables = !stmt->tables || !stmt->for_all_tables;
+ /* FOR ALL TABLES requires superuser */
+ if (alltables && !superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser to create subscriptions";
+ (errmsg("must be superuser to create FOR ALL TABLES publication";
+
rel = heap_open(SubscriptionRelationId, RowExclusiveLock);
@@ -384,6 +395,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
DirectFunctionCall1(namein, CStringGetDatum(stmt->subname));
values[Anum_pg_subscription_subowner - 1] = ObjectIdGetDatum(owner);
values[Anum_pg_subscription_subenabled - 1] = BoolGetDatum(enabled);
+ values[Anum_pg_subscription_suballtables - 1] = BoolGetDatum(alltables);
values[Anum_pg_subscription_subconninfo - 1] =
CStringGetTextDatum(conninfo);
if (slotname)
@@ -407,6 +419,27 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
snprintf(originname, sizeof(originname), "pg_%u", subid);
r
Re: Special role for subscriptions
Hi! In order to support create subscription from non-superuser, we need to make it possible to choose tables on the subscriber side: 1. add `FOR TABLE` clause in `CREATE SUBSCRIPTION`: ``` CREATE SUBSCRIPTION subscription_name CONNECTION 'conninfo' PUBLICATION publication_name [, ...] [ FOR TABLE [ ONLY ] table_name [ * ] [, ...]| FOR ALL TABLES ] [ WITH ( subscription_parameter [= value] [, ... ] ) ] ``` ... where `FOR ALL TABLES` is only allowed for superuser. and table list in `FOR TABLES` clause will be stored in pg_subscription_rel table (maybe another place?) 2. Each subscription should have "all tables" attribute. For example via a new column in pg_subscription "suballtables". 3. Add `ALTER SUBSCRIPTION (ADD TABLE | DROP TABLE)`: ``` ALTER SUBSCRIPTION subscription_name ADD TABLE [ ONLY ] table_name [WITH copy_data]; ALTER SUBSCRIPTION subscription_name DROP TABLE [ ONLY ] table_name; ``` 4. On `ALTER SUBSCRIPTION REFRESH PUBLICATION` should check if table owner equals subscription owner. The check is ommited if subscription owner is superuser. 5. If superuser calls `ALTER SUBSCRIPTION REFRESH PUBLICATION` on subscription with table list and non-superuser owner, we should filter tables which owner is not subscription's owner or maybe we need to raise error? What do you think about it? Any objections? 07.11.2018, 00:52, "Stephen Frost" : > Greetings, > > * Evgeniy Efimkin (efim...@yandex-team.ru) wrote: >> As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only. > > That's a nice idea but seems like we would want to have a way to filter > what tables a subscription follows then..? Just failing if the > publication publishes tables that we don't have access to or are not the > owner of seems like a poor solution.. > > Thanks! > > Stephen Ефимкин Евгений
Re: Special role for subscriptions
Hi! I think we can add FOR TABLES clause for create/refresh subscription, for example: CREATE SUBSCRIPTION my_sub CONNECTION ... PUBLICATION my_pub [WITH ...] [ FOR TABLES t1, t2 | ALL TABLES ]. ALL TABLES is avalibale only for superuser. FOR TABLES t1, t2 is available to owner of tables and superuser. 07.11.2018, 00:52, "Stephen Frost" : > Greetings, > > * Evgeniy Efimkin (efim...@yandex-team.ru) wrote: >> As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only. > > That's a nice idea but seems like we would want to have a way to filter > what tables a subscription follows then..? Just failing if the > publication publishes tables that we don't have access to or are not the > owner of seems like a poor solution.. > > Thanks! > > Stephen Ефимкин Евгений
Connection limit doesn't work for superuser
Connection limit doesn't work for superuser Hi hackers! It would be nice if ALTER USER ... WITH CONNECTION LIMIT will work for superuser. It would protect against connection leaks. e.g. we have two superusers, one of them reached connection limit but not max_connections, the other is still possible to connect to database and solve the problem. The current behaviour would be the same for the case with rolconnlimit = -1. Superuser can execute NOLOGIN to another superuser and it works. I found previos discussion about CONNECTION LIMIT for superuser but it ended about Slony. https://www.postgresql.org/message-id/1154351265.22367.210.camel%40coppola.muc.ecircle.de
Re: Special role for subscriptions
Hi! As a first step I suggest we allow CREATE SUBSCRIPTION for table owner only. 03.11.2018, 19:20, "Stephen Frost" : > Greetings, > > * Evgeniy Efimkin (efim...@yandex-team.ru) wrote: >> In postgresql 10 and 11 only superuser can create/alter subscriptions. >> If there was a special role (like pg_monitor), it would be more easy to >> grant control on subscriptions. >> I can make a patch if there are no objections against it. > > I think the short answer is 'yes, we should let non-superusers do that', > but the longer answer is: > > What level of access makes sense for managing subscriptions? Should > there be a way to say "user X is allowed to create a subscription for > remote system Y, but only for tables that exist in schema Q"? > > My general feeling is 'yes', though, of course, I don't want to say that > we have to have all of that before we move forward with allowing > non-superusers to create subscriptions, but I do think we want to make > sure that we have a well thought-out path for how to get from where we > are now to a system which has a lot more granularity, and to do our best > to try avoiding any paths that might paint us into a corner. > > Thanks! > > Stephen
Special role for subscriptions
Hi hackers! In postgresql 10 and 11 only superuser can create/alter subscriptions. If there was a special role (like pg_monitor), it would be more easy to grant control on subscriptions. I can make a patch if there are no objections against it.
