public inbox for [email protected]
help / color / mirror / Atom feedRe: psql: Add role's membership options to the \du+ command
12+ messages / 5 participants
[nested] [flat]
* Re: psql: Add role's membership options to the \du+ command
@ 2023-04-05 07:42 ` Pavel Luzanov <[email protected]>
1 sibling, 0 replies; 12+ messages in thread
From: Pavel Luzanov @ 2023-04-05 07:42 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>
On 04.04.2023 22:02, David G. Johnston wrote:
> On Tue, Apr 4, 2023 at 10:37 AM Tom Lane <[email protected]> wrote:
>
> Robert Haas <[email protected]> writes:
> > On Tue, Apr 4, 2023 at 1:12 PM Tom Lane <[email protected]> wrote:
> >> I wonder if, while we're here, we should apply the idea of
> >> joining-with-newlines-not-commas to the attributes column too.
>
> > That would make the column narrower, which might be good, because it
> > seems to me that listing the memberships could require quite a
> lot of
> > space, both vertical and horizontal.
>
> Right, that's what I was thinking.
>
>
> So, by way of example:
>
> regress_du_role1 | cannot login | regress_du_role0 granted by
> regress_du_admin with admin, inherit, set | Description for
> regress_du_role1
Perhaps more closely to syntax?
regress_du_role0 with admin, inherit, set granted by regress_du_admin
instead of
regress_du_role0 granted by regress_du_admin with admin, inherit, set
> No translations, all words are either identical to syntax or identifiers.
>
> I'm on board with newlines in the attributes field.
+1
> The specific member of column changes are:
>
> from -> granted by
> ( ) -> "with"
> ais -> admin, inherit, set
>
> I'm good with any or all of those selections, either as-is or in the
> more verbose form.
From yesterday's discussion, I think two things are important:
- it is advisable to avoid translation,
- there is no sense in the abbreviation (a,i,s), if there are full names
in the 'attributes' column.
So I agree with such changes and plan to implement them.
And one more question. (I think it's better to have it explicitly
rejected than to keep silent.)
What if this long output will be available only for \du+, and for \du
just show distinct (without duplicates)
roles in the current array format? For those, who don't care about these
new membership options, nothing will change.
Those, who need details will use the + modifier.
?
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
@ 2023-04-05 13:58 ` Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
1 sibling, 1 reply; 12+ messages in thread
From: Tom Lane @ 2023-04-05 13:58 UTC (permalink / raw)
To: Pavel Luzanov <[email protected]>; +Cc: David G. Johnston <[email protected]>; Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>
Pavel Luzanov <[email protected]> writes:
> What if this long output will be available only for \du+, and for \du
> just show distinct (without duplicates)
> roles in the current array format? For those, who don't care about these
> new membership options, nothing will change.
> Those, who need details will use the + modifier.
> ?
I kind of like that. Would we change to newlines in the Attributes
field in both \du and \du+? (I'm +1 for that, but maybe others aren't.)
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
@ 2023-04-05 14:24 ` David G. Johnston <[email protected]>
2023-04-13 12:44 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: David G. Johnston @ 2023-04-05 14:24 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Pavel Luzanov <[email protected]>; Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>
On Wed, Apr 5, 2023 at 6:58 AM Tom Lane <[email protected]> wrote:
> Pavel Luzanov <[email protected]> writes:
> > What if this long output will be available only for \du+, and for \du
> > just show distinct (without duplicates)
> > roles in the current array format? For those, who don't care about these
> > new membership options, nothing will change.
> > Those, who need details will use the + modifier.
> > ?
>
> I kind of like that. Would we change to newlines in the Attributes
> field in both \du and \du+? (I'm +1 for that, but maybe others aren't.)
>
>
If we don't change the \du "Member of" column display (aside from removing
duplicates) I'm disinclined to change the Attributes column.
I too am partial to only exposing this detail on the extended (+) display.
David J.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
@ 2023-04-13 12:44 ` Pavel Luzanov <[email protected]>
2023-05-03 16:00 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Pavel Luzanov @ 2023-04-13 12:44 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>
After playing with the \du command, I found that we can't avoid translation.
All attributes are translatable. Also, two of nine attributes shows in
new line separated format (connection limit and password valid until).
$ LANGUAGE=fr psql -c "ALTER ROLE postgres CONNECTION LIMIT 3 VALID
UNTIL 'infinity'" -c '\du'
ALTER ROLE
Liste des rôles
Nom du rôle | Attributs | Membre de
-------------+---------------------------------------------------------------------------------+-----------
postgres | Superutilisateur, Créer un rôle, Créer une base,
Réplication, Contournement RLS+| {}
| 3 connexions +|
| Mot de passe valide jusqu'à
infinity |
So I decided to keep the format suggested by David, but without
abbreviations and only for extended mode.
$ psql -c '\duS+'
List of roles
Role name | Attributes
| Member of | Description
-----------------------------+-------------------------------+---------------------------------------------------+-------------
pg_checkpoint | Cannot login
| |
pg_create_subscription | Cannot login
| |
pg_database_owner | Cannot login
| |
pg_execute_server_program | Cannot login
| |
pg_maintain | Cannot login
| |
pg_monitor | Cannot login |
pg_read_all_settings from postgres (inherit, set)+|
| |
pg_read_all_stats from postgres (inherit, set) +|
| |
pg_stat_scan_tables from postgres (inherit, set) |
pg_read_all_data | Cannot login
| |
pg_read_all_settings | Cannot login
| |
pg_read_all_stats | Cannot login
| |
pg_read_server_files | Cannot login
| |
pg_signal_backend | Cannot login
| |
pg_stat_scan_tables | Cannot login
| |
pg_use_reserved_connections | Cannot login
| |
pg_write_all_data | Cannot login
| |
pg_write_server_files | Cannot login
| |
postgres | Superuser
+| |
| Create role
+| |
| Create DB
+| |
| Replication
+| |
| Bypass RLS
+| |
| 3 connections
+| |
| Password valid until infinity
| |
Please look at new version. I understand that this is a compromise choice.
I am ready to change it if a better solution is offered.
P.S. If no objections I plan to add this patch to Open Items for v16
https://wiki.postgresql.org/wiki/PostgreSQL_16_Open_Items
On 05.04.2023 17:24, David G. Johnston wrote:
> On Wed, Apr 5, 2023 at 6:58 AM Tom Lane <[email protected]> wrote:
>
> Pavel Luzanov <[email protected]> writes:
> > What if this long output will be available only for \du+, and
> for \du
> > just show distinct (without duplicates)
> > roles in the current array format? For those, who don't care
> about these
> > new membership options, nothing will change.
> > Those, who need details will use the + modifier.
> > ?
>
> I kind of like that. Would we change to newlines in the Attributes
> field in both \du and \du+? (I'm +1 for that, but maybe others
> aren't.)
>
>
> If we don't change the \du "Member of" column display (aside from
> removing duplicates) I'm disinclined to change the Attributes column.
>
> I too am partial to only exposing this detail on the extended (+) display.
>
> David J.
>
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
Attachments:
[text/x-patch] v7-0001-psql-show-membership-options-in-the-du-command.patch (16.2K, ../../[email protected]/3-v7-0001-psql-show-membership-options-in-the-du-command.patch)
download | inline diff:
From 1f9433696e41a8f37cfd4c0514e136fedd50939e Mon Sep 17 00:00:00 2001
From: Pavel Luzanov <[email protected]>
Date: Thu, 13 Apr 2023 15:09:57 +0300
Subject: [PATCH v7] psql: show membership options in the \du command
---
doc/src/sgml/ref/psql-ref.sgml | 40 +++++++++++--
src/bin/psql/describe.c | 92 ++++++++++++++++++++++--------
src/test/regress/expected/psql.out | 49 ++++++++++++++++
src/test/regress/sql/psql.sql | 30 ++++++++++
4 files changed, 182 insertions(+), 29 deletions(-)
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index dc422373d6..f174bddcb0 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1727,9 +1727,23 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
<literal>S</literal> modifier to include system roles.
If <replaceable class="parameter">pattern</replaceable> is specified,
only those roles whose names match the pattern are listed.
- If the form <literal>\dg+</literal> is used, additional information
- is shown about each role; currently this adds the comment for each
- role.
+ </para>
+ <para>
+ If the form <literal>\dg</literal> is used, a list of distinct roles
+ of which it is a member is shown for each role in array format.
+ </para>
+ <para>
+ If the form <literal>\dg+</literal> is used, the memberships granted
+ to the role shown within each row, in newline-separated format.
+ The presentation includes both the name of the grantor as well as
+ the membership permissions: <literal>admin</literal>,
+ <literal>inherit</literal>, <literal>set</literal>.
+ The word <literal>empty</literal> is printed in the case that none of those
+ permissions are granted.
+ See the <link linkend="sql-grant"><command>GRANT</command></link> command for their meaning.
+ </para>
+ <para>
+ Also, if the form <literal>\dg+</literal> is used, the comment attached to the role is shown.
</para>
</listitem>
</varlistentry>
@@ -1969,9 +1983,23 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind 'first value' 'second value' \g
<literal>S</literal> modifier to include system roles.
If <replaceable class="parameter">pattern</replaceable> is specified,
only those roles whose names match the pattern are listed.
- If the form <literal>\du+</literal> is used, additional information
- is shown about each role; currently this adds the comment for each
- role.
+ </para>
+ <para>
+ If the form <literal>\du</literal> is used, a list of distinct roles
+ of which it is a member is shown for each role in array format.
+ </para>
+ <para>
+ If the form <literal>\du+</literal> is used, the memberships granted
+ to the role shown within each row, in newline-separated format.
+ The presentation includes both the name of the grantor as well as
+ the membership permissions: <literal>admin</literal>,
+ <literal>inherit</literal>, <literal>set</literal>.
+ The word <literal>empty</literal> is printed in the case that none of those
+ permissions are granted.
+ See the <link linkend="sql-grant"><command>GRANT</command></link> command for their meaning.
+ </para>
+ <para>
+ Also, if the form <literal>\du+</literal> is used, the comment attached to the role is shown.
</para>
</listitem>
</varlistentry>
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 83a37ee601..afdffb6932 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -36,7 +36,9 @@ static bool describeOneTableDetails(const char *schemaname,
bool verbose);
static void add_tablespace_footer(printTableContent *const cont, char relkind,
Oid tablespace, const bool newline);
-static void add_role_attribute(PQExpBuffer buf, const char *const str);
+static void add_role_attribute(PQExpBuffer buf,
+ const char *const str,
+ bool verbose);
static bool listTSParsersVerbose(const char *pattern);
static bool describeOneTSParser(const char *oid, const char *nspname,
const char *prsname);
@@ -3631,24 +3633,56 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printfPQExpBuffer(&buf,
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
- " r.rolconnlimit, r.rolvaliduntil,\n"
- " ARRAY(SELECT b.rolname\n"
- " FROM pg_catalog.pg_auth_members m\n"
- " JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
- " WHERE m.member = r.oid) as memberof");
+ " r.rolconnlimit, r.rolvaliduntil, r.rolreplication,\n");
- if (verbose)
+ if (pset.sversion >= 160000)
{
- appendPQExpBufferStr(&buf, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
- ncols++;
+ if (verbose)
+ appendPQExpBuffer(&buf,
+ " (SELECT pg_catalog.string_agg(\n"
+ " pg_catalog.format('%%I from %%I (%%s)',\n"
+ " b.rolname, m.grantor::pg_catalog.regrole::pg_catalog.text,\n"
+ " pg_catalog.regexp_replace(\n"
+ " pg_catalog.concat_ws(', ',\n"
+ " CASE WHEN m.admin_option THEN '%s' END,\n"
+ " CASE WHEN m.inherit_option THEN '%s' END,\n"
+ " CASE WHEN m.set_option THEN '%s' END),\n"
+ " '^$', '%s')\n"
+ " ), E'\\n'\n"
+ " ORDER BY b.rolname, m.grantor::pg_catalog.regrole::pg_catalog.text)\n"
+ " FROM pg_catalog.pg_auth_members m\n"
+ " JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
+ " WHERE m.member = r.oid) as memberof",
+ gettext_noop("admin"),
+ gettext_noop("inherit"),
+ gettext_noop("set"),
+ gettext_noop("empty"));
+ else
+ appendPQExpBufferStr(&buf,
+ " ARRAY(SELECT DISTINCT b.rolname\n"
+ " FROM pg_catalog.pg_auth_members m\n"
+ " JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
+ " WHERE m.member = r.oid\n"
+ " ORDER BY 1) AS memberof");
}
- appendPQExpBufferStr(&buf, "\n, r.rolreplication");
+ else
+ appendPQExpBufferStr(&buf,
+ " ARRAY(SELECT b.rolname\n"
+ " FROM pg_catalog.pg_auth_members m\n"
+ " JOIN pg_catalog.pg_roles b ON (m.roleid = b.oid)\n"
+ " WHERE m.member = r.oid) as memberof");
if (pset.sversion >= 90500)
{
appendPQExpBufferStr(&buf, "\n, r.rolbypassrls");
}
+ if (verbose)
+ {
+ appendPQExpBufferStr(&buf, "\n, pg_catalog.shobj_description(r.oid, 'pg_authid') AS description");
+ ncols++;
+ }
+
appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_roles r\n");
if (!showSystem && !pattern)
@@ -3687,26 +3721,26 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
resetPQExpBuffer(&buf);
if (strcmp(PQgetvalue(res, i, 1), "t") == 0)
- add_role_attribute(&buf, _("Superuser"));
+ add_role_attribute(&buf, _("Superuser"), verbose);
if (strcmp(PQgetvalue(res, i, 2), "t") != 0)
- add_role_attribute(&buf, _("No inheritance"));
+ add_role_attribute(&buf, _("No inheritance"), verbose);
if (strcmp(PQgetvalue(res, i, 3), "t") == 0)
- add_role_attribute(&buf, _("Create role"));
+ add_role_attribute(&buf, _("Create role"), verbose);
if (strcmp(PQgetvalue(res, i, 4), "t") == 0)
- add_role_attribute(&buf, _("Create DB"));
+ add_role_attribute(&buf, _("Create DB"), verbose);
if (strcmp(PQgetvalue(res, i, 5), "t") != 0)
- add_role_attribute(&buf, _("Cannot login"));
+ add_role_attribute(&buf, _("Cannot login"), verbose);
- if (strcmp(PQgetvalue(res, i, (verbose ? 10 : 9)), "t") == 0)
- add_role_attribute(&buf, _("Replication"));
+ if (strcmp(PQgetvalue(res, i, 8), "t") == 0)
+ add_role_attribute(&buf, _("Replication"), verbose);
if (pset.sversion >= 90500)
- if (strcmp(PQgetvalue(res, i, (verbose ? 11 : 10)), "t") == 0)
- add_role_attribute(&buf, _("Bypass RLS"));
+ if (strcmp(PQgetvalue(res, i, 10), "t") == 0)
+ add_role_attribute(&buf, _("Bypass RLS"), verbose);
conns = atoi(PQgetvalue(res, i, 6));
if (conns >= 0)
@@ -3735,10 +3769,15 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
printTableAddCell(&cont, attr[i], false, false);
- printTableAddCell(&cont, PQgetvalue(res, i, 8), false, false);
+ printTableAddCell(&cont, PQgetvalue(res, i, 9), false, false);
if (verbose)
- printTableAddCell(&cont, PQgetvalue(res, i, 9), false, false);
+ {
+ if (pset.sversion >= 90500)
+ printTableAddCell(&cont, PQgetvalue(res, i, 11), false, false);
+ else
+ printTableAddCell(&cont, PQgetvalue(res, i, 10), false, false);
+ }
}
termPQExpBuffer(&buf);
@@ -3754,10 +3793,17 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
}
static void
-add_role_attribute(PQExpBuffer buf, const char *const str)
+add_role_attribute(PQExpBuffer buf,
+ const char *const str,
+ bool verbose)
{
if (buf->len > 0)
- appendPQExpBufferStr(buf, ", ");
+ {
+ if (pset.sversion >= 160000)
+ appendPQExpBufferStr(buf, verbose ? "\n" : ", ");
+ else
+ appendPQExpBufferStr(buf, ", ");
+ }
appendPQExpBufferStr(buf, str);
}
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 956e475447..396eaa8495 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -6607,3 +6607,52 @@ cross-database references are not implemented: "no.such.database"."no.such.schem
cross-database references are not implemented: "no.such.database"."no.such.schema"."no.such.data.type"
\dX "no.such.database"."no.such.schema"."no.such.extended.statistics"
cross-database references are not implemented: "no.such.database"."no.such.schema"."no.such.extended.statistics"
+-- check \du
+CREATE ROLE regress_du_role0;
+CREATE ROLE regress_du_role1;
+CREATE ROLE regress_du_role2 LOGIN CREATEROLE BYPASSRLS REPLICATION CREATEDB NOINHERIT CONNECTION LIMIT 3 VALID UNTIL 'infinity';
+CREATE ROLE regress_du_admin LOGIN CREATEROLE;
+COMMENT ON ROLE regress_du_role0 IS 'Description for regress_du_role0';
+COMMENT ON ROLE regress_du_role1 IS 'Description for regress_du_role1';
+COMMENT ON ROLE regress_du_role2 IS 'Description for regress_du_role2';
+COMMENT ON ROLE regress_du_admin IS 'Description for regress_du_admin';
+GRANT regress_du_role0 TO regress_du_admin WITH ADMIN TRUE;
+GRANT regress_du_role1 TO regress_du_admin WITH ADMIN TRUE;
+GRANT regress_du_role2 TO regress_du_admin WITH ADMIN TRUE;
+GRANT regress_du_role0 TO regress_du_role1 WITH ADMIN TRUE, INHERIT TRUE, SET TRUE GRANTED BY regress_du_admin;
+GRANT regress_du_role0 TO regress_du_role2 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE GRANTED BY regress_du_admin;
+GRANT regress_du_role1 TO regress_du_role2 WITH ADMIN TRUE , INHERIT FALSE, SET TRUE GRANTED BY regress_du_admin;
+GRANT regress_du_role0 TO regress_du_role1 WITH ADMIN FALSE, INHERIT TRUE, SET FALSE GRANTED BY regress_du_role1;
+GRANT regress_du_role0 TO regress_du_role2 WITH ADMIN FALSE, INHERIT TRUE , SET TRUE GRANTED BY regress_du_role1;
+GRANT regress_du_role0 TO regress_du_role1 WITH ADMIN FALSE, INHERIT FALSE, SET TRUE GRANTED BY regress_du_role2;
+GRANT regress_du_role0 TO regress_du_role2 WITH ADMIN FALSE, INHERIT FALSE, SET FALSE GRANTED BY regress_du_role2;
+\du regress_du_role*
+ List of roles
+ Role name | Attributes | Member of
+------------------+-----------------------------------------------------------------+-------------------------------------
+ regress_du_role0 | Cannot login | {}
+ regress_du_role1 | Cannot login | {regress_du_role0}
+ regress_du_role2 | No inheritance, Create role, Create DB, Replication, Bypass RLS+| {regress_du_role0,regress_du_role1}
+ | 3 connections +|
+ | Password valid until infinity |
+
+\du+ regress_du_role*
+ List of roles
+ Role name | Attributes | Member of | Description
+------------------+-------------------------------+--------------------------------------------------------------+----------------------------------
+ regress_du_role0 | Cannot login | | Description for regress_du_role0
+ regress_du_role1 | Cannot login | regress_du_role0 from regress_du_admin (admin, inherit, set)+| Description for regress_du_role1
+ | | regress_du_role0 from regress_du_role1 (inherit) +|
+ | | regress_du_role0 from regress_du_role2 (set) |
+ regress_du_role2 | No inheritance +| regress_du_role0 from regress_du_admin (admin) +| Description for regress_du_role2
+ | Create role +| regress_du_role0 from regress_du_role1 (inherit, set) +|
+ | Create DB +| regress_du_role0 from regress_du_role2 (empty) +|
+ | Replication +| regress_du_role1 from regress_du_admin (admin, set) |
+ | Bypass RLS +| |
+ | 3 connections +| |
+ | Password valid until infinity | |
+
+DROP ROLE regress_du_role0;
+DROP ROLE regress_du_role1;
+DROP ROLE regress_du_role2;
+DROP ROLE regress_du_admin;
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 630f638f02..5ef5e40e46 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -1815,3 +1815,33 @@ DROP FUNCTION psql_error;
\dP "no.such.database"."no.such.schema"."no.such.partitioned.relation"
\dT "no.such.database"."no.such.schema"."no.such.data.type"
\dX "no.such.database"."no.such.schema"."no.such.extended.statistics"
+
+-- check \du
+CREATE ROLE regress_du_role0;
+CREATE ROLE regress_du_role1;
+CREATE ROLE regress_du_role2 LOGIN CREATEROLE BYPASSRLS REPLICATION CREATEDB NOINHERIT CONNECTION LIMIT 3 VALID UNTIL 'infinity';
+CREATE ROLE regress_du_admin LOGIN CREATEROLE;
+COMMENT ON ROLE regress_du_role0 IS 'Description for regress_du_role0';
+COMMENT ON ROLE regress_du_role1 IS 'Description for regress_du_role1';
+COMMENT ON ROLE regress_du_role2 IS 'Description for regress_du_role2';
+COMMENT ON ROLE regress_du_admin IS 'Description for regress_du_admin';
+
+GRANT regress_du_role0 TO regress_du_admin WITH ADMIN TRUE;
+GRANT regress_du_role1 TO regress_du_admin WITH ADMIN TRUE;
+GRANT regress_du_role2 TO regress_du_admin WITH ADMIN TRUE;
+
+GRANT regress_du_role0 TO regress_du_role1 WITH ADMIN TRUE, INHERIT TRUE, SET TRUE GRANTED BY regress_du_admin;
+GRANT regress_du_role0 TO regress_du_role2 WITH ADMIN TRUE, INHERIT FALSE, SET FALSE GRANTED BY regress_du_admin;
+GRANT regress_du_role1 TO regress_du_role2 WITH ADMIN TRUE , INHERIT FALSE, SET TRUE GRANTED BY regress_du_admin;
+GRANT regress_du_role0 TO regress_du_role1 WITH ADMIN FALSE, INHERIT TRUE, SET FALSE GRANTED BY regress_du_role1;
+GRANT regress_du_role0 TO regress_du_role2 WITH ADMIN FALSE, INHERIT TRUE , SET TRUE GRANTED BY regress_du_role1;
+GRANT regress_du_role0 TO regress_du_role1 WITH ADMIN FALSE, INHERIT FALSE, SET TRUE GRANTED BY regress_du_role2;
+GRANT regress_du_role0 TO regress_du_role2 WITH ADMIN FALSE, INHERIT FALSE, SET FALSE GRANTED BY regress_du_role2;
+
+\du regress_du_role*
+\du+ regress_du_role*
+
+DROP ROLE regress_du_role0;
+DROP ROLE regress_du_role1;
+DROP ROLE regress_du_role2;
+DROP ROLE regress_du_admin;
--
2.34.1
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-04-13 12:44 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
@ 2023-05-03 16:00 ` Jonathan S. Katz <[email protected]>
2023-05-03 16:13 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan S. Katz @ 2023-05-03 16:00 UTC (permalink / raw)
To: Pavel Luzanov <[email protected]>; David G. Johnston <[email protected]>; Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>; [email protected]
On 4/13/23 8:44 AM, Pavel Luzanov wrote:
> P.S. If no objections I plan to add this patch to Open Items for v16
> https://wiki.postgresql.org/wiki/PostgreSQL_16_Open_Items
[RMT hat]
I don't see why this is an open item as this feature was not committed
for v16. Open items typically revolve around committed features.
Unless someone makes a convincing argument otherwise, I'll remove this
from the Open Items list[1] tomorrow.
Thanks,
Jonathan
[1] https://wiki.postgresql.org/wiki/PostgreSQL_16_Open_Items
Attachments:
[application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-04-13 12:44 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
2023-05-03 16:00 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
@ 2023-05-03 16:13 ` David G. Johnston <[email protected]>
2023-05-03 16:25 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: David G. Johnston @ 2023-05-03 16:13 UTC (permalink / raw)
To: Jonathan S. Katz <[email protected]>; +Cc: Pavel Luzanov <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>; [email protected]
On Wed, May 3, 2023 at 9:00 AM Jonathan S. Katz <[email protected]>
wrote:
> On 4/13/23 8:44 AM, Pavel Luzanov wrote:
>
> > P.S. If no objections I plan to add this patch to Open Items for v16
> > https://wiki.postgresql.org/wiki/PostgreSQL_16_Open_Items
>
> [RMT hat]
>
> I don't see why this is an open item as this feature was not committed
> for v16. Open items typically revolve around committed features.
>
> Unless someone makes a convincing argument otherwise, I'll remove this
> from the Open Items list[1] tomorrow.
>
> Thanks,
>
> Jonathan
>
> [1] https://wiki.postgresql.org/wiki/PostgreSQL_16_Open_Items
The argument is that updating the psql \d views to show the newly added
options is something that the patch to add those options should have done
before being committed. Or, at worse, we should decide now that we don't
want to do so and spare people the effort of trying to get this committed
later.
David J.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-04-13 12:44 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
2023-05-03 16:00 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
2023-05-03 16:13 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
@ 2023-05-03 16:25 ` Tom Lane <[email protected]>
2023-05-03 16:30 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Tom Lane @ 2023-05-03 16:25 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: Jonathan S. Katz <[email protected]>; Pavel Luzanov <[email protected]>; Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>; [email protected]
"David G. Johnston" <[email protected]> writes:
> On Wed, May 3, 2023 at 9:00 AM Jonathan S. Katz <[email protected]>
> wrote:
>> I don't see why this is an open item as this feature was not committed
>> for v16. Open items typically revolve around committed features.
> The argument is that updating the psql \d views to show the newly added
> options is something that the patch to add those options should have done
> before being committed.
Yeah, if there is not any convenient way to see that info in psql
then that seems like a missing part of the feature.
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-04-13 12:44 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
2023-05-03 16:00 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
2023-05-03 16:13 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-05-03 16:25 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
@ 2023-05-03 16:30 ` Jonathan S. Katz <[email protected]>
2023-05-05 16:51 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Jonathan S. Katz @ 2023-05-03 16:30 UTC (permalink / raw)
To: Tom Lane <[email protected]>; David G. Johnston <[email protected]>; +Cc: Pavel Luzanov <[email protected]>; Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>; [email protected]
On 5/3/23 12:25 PM, Tom Lane wrote:
> "David G. Johnston" <[email protected]> writes:
>> On Wed, May 3, 2023 at 9:00 AM Jonathan S. Katz <[email protected]>
>> wrote:
>>> I don't see why this is an open item as this feature was not committed
>>> for v16. Open items typically revolve around committed features.
>
>> The argument is that updating the psql \d views to show the newly added
>> options is something that the patch to add those options should have done
>> before being committed.
>
> Yeah, if there is not any convenient way to see that info in psql
> then that seems like a missing part of the feature.
[RMT hat]
OK -- I was rereading the thread again to see if I could glean that
insight. There was a comment buried in the thread with David's opinion
on that front, but no one had +1'd that.
However, if this is for feature completeness, I'll withdraw the closing
of the open item, but would strongly suggest we complete it in time for
Beta 1.
[Personal hat]
Looking at Pavel's latest patch, I do find the output easy to
understand, though do we need to explicitly list "empty" if there are no
membership permissions?
Thanks,
Jonathan
Attachments:
[application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-04-13 12:44 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
2023-05-03 16:00 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
2023-05-03 16:13 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-05-03 16:25 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-05-03 16:30 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
@ 2023-05-05 16:51 ` David G. Johnston <[email protected]>
2023-05-07 19:14 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: David G. Johnston @ 2023-05-05 16:51 UTC (permalink / raw)
To: Jonathan S. Katz <[email protected]>; +Cc: Tom Lane <[email protected]>; Pavel Luzanov <[email protected]>; Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>; [email protected]
On Wed, May 3, 2023 at 9:30 AM Jonathan S. Katz <[email protected]>
wrote:
> [Personal hat]
>
> Looking at Pavel's latest patch, I do find the output easy to
> understand, though do we need to explicitly list "empty" if there are no
> membership permissions?
>
>
Yes. I dislike having the equivalent of null embedded within the output
here. I would rather label it for what it is. As a membership without any
attributes has no real purpose I don't see how the choice matters and at
least empty both stands out visually and can be grepped.
The SQL language uses the words "by" and "from" in its syntax; I'm against
avoiding them in our presentation here without a clearly superior
alternative that doesn't require a majority of people to have to translate
the symbol " / " back into the word " by " in order to read the output.
But if it is really a blocker then maybe we should produce 3 separate
newline separated columns, one for the member of role, one for the list of
attributes, and one with the grantor. The column headers can be translated
more easily as single nouns. The readability quite probably would end up
being equivalent (maybe even better) in tabular form instead of sentence
form.
David J.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-04-13 12:44 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
2023-05-03 16:00 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
2023-05-03 16:13 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-05-03 16:25 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-05-03 16:30 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
2023-05-05 16:51 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
@ 2023-05-07 19:14 ` Pavel Luzanov <[email protected]>
2023-05-18 02:42 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Pavel Luzanov @ 2023-05-07 19:14 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; Jonathan S. Katz <[email protected]>; +Cc: Tom Lane <[email protected]>; Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>; [email protected]; [email protected]
On 05.05.2023 19:51, David G. Johnston wrote:
> But if it is really a blocker then maybe we should produce 3 separate
> newline separated columns, one for the member of role, one for the
> list of attributes, and one with the grantor. The column headers can
> be translated more easily as single nouns. The readability quite
> probably would end up being equivalent (maybe even better) in tabular
> form instead of sentence form.
Just to visualize this approach. Below are the output for the tabular
form and the sentence form from last patch version (sql script attached):
Tabular form rolname | memberof | options
| grantor
------------------+------------------+---------------------+------------------ postgres
| | | regress_du_admin |
regress_du_role0+| admin, inherit, set+| postgres
+ | regress_du_role1+| admin, inherit, set+|
postgres + | regress_du_role2 | admin, inherit,
set | postgres regress_du_role0 | |
| regress_du_role1 | regress_du_role0+| admin, inherit, set+|
regress_du_admin+ | regress_du_role0+|
inherit +| regress_du_role1+ |
regress_du_role0 | set |
regress_du_role2 regress_du_role2 | regress_du_role0+|
admin +| regress_du_admin+ |
regress_du_role0+| inherit, set +|
regress_du_role1+ | regress_du_role0+|
empty +| regress_du_role2+ |
regress_du_role1 | admin, set | regress_du_admin(5
rows)Sentence form from patch v7 rolname
| memberof
------------------+-------------------------------------------------------------- postgres
| regress_du_admin | regress_du_role0 from postgres (admin, inherit,
set) + | regress_du_role1 from postgres (admin,
inherit, set) + | regress_du_role2 from postgres
(admin, inherit, set) regress_du_role0 | regress_du_role1 |
regress_du_role0 from regress_du_admin (admin, inherit,
set)+ | regress_du_role0 from regress_du_role1
(inherit) + | regress_du_role0 from
regress_du_role2 (set) regress_du_role2 | regress_du_role0 from
regress_du_admin (admin) + |
regress_du_role0 from regress_du_role1 (inherit, set)
+ | regress_du_role0 from regress_du_role2
(empty) + | regress_du_role1 from
regress_du_admin (admin, set)(5 rows)
The tabular form solves the latest patch translation problems mentioned by Kyotaro.
But it requires mapping elements between 3 array-like columns.
To move forward, needs more opinions?
-----
Pavel Luzanov
Attachments:
[application/sql] t.sql (3.1K, ../../[email protected]/3-t.sql)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: psql: Add role's membership options to the \du+ command
2023-04-05 13:58 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-04-05 14:24 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-04-13 12:44 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
2023-05-03 16:00 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
2023-05-03 16:13 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-05-03 16:25 ` Re: psql: Add role's membership options to the \du+ command Tom Lane <[email protected]>
2023-05-03 16:30 ` Re: psql: Add role's membership options to the \du+ command Jonathan S. Katz <[email protected]>
2023-05-05 16:51 ` Re: psql: Add role's membership options to the \du+ command David G. Johnston <[email protected]>
2023-05-07 19:14 ` Re: psql: Add role's membership options to the \du+ command Pavel Luzanov <[email protected]>
@ 2023-05-18 02:42 ` Jonathan S. Katz <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Jonathan S. Katz @ 2023-05-18 02:42 UTC (permalink / raw)
To: Pavel Luzanov <[email protected]>; David G. Johnston <[email protected]>; +Cc: Tom Lane <[email protected]>; Robert Haas <[email protected]>; David Zhang <[email protected]>; [email protected] <[email protected]>; [email protected]; [email protected]
On 5/7/23 3:14 PM, Pavel Luzanov wrote:
> On 05.05.2023 19:51, David G. Johnston wrote:
>> But if it is really a blocker then maybe we should produce 3 separate
>> newline separated columns, one for the member of role, one for the
>> list of attributes, and one with the grantor. The column headers can
>> be translated more easily as single nouns. The readability quite
>> probably would end up being equivalent (maybe even better) in tabular
>> form instead of sentence form.
>
> Just to visualize this approach. Below are the output for the tabular
> form and the sentence form from last patch version (sql script attached):
>
> Tabular form rolname | memberof | options
> | grantor
> ------------------+------------------+---------------------+------------------ postgres | | | regress_du_admin | regress_du_role0+| admin, inherit, set+| postgres + | regress_du_role1+| admin, inherit, set+| postgres + | regress_du_role2 | admin, inherit, set | postgres regress_du_role0 | | | regress_du_role1 | regress_du_role0+| admin, inherit, set+| regress_du_admin+ | regress_du_role0+| inherit +| regress_du_role1+ | regress_du_role0 | set | regress_du_role2 regress_du_role2 | regress_du_role0+| admin +| regress_du_admin+ | regress_du_role0+| inherit, set +| regress_du_role1+ | regress_du_role0+| empty +| regress_du_role2+ | regress_du_role1 | admin, set | regress_du_admin(5 rows)Sentence form from patch v7 rolname | memberof ------------------+-------------------------------------------------------------- postgres | regress_du_admin | regress_du_role0 from postgres (admin, inherit, set) + | regress_du_role1 from postgres (admin, inherit, set) + | regress_du_role2 from postgres (admin, inherit, set) regress_du_role0 | regress_du_role1 | regress_du_role0 from regress_du_admin (admin, inherit, set)+ | regress_du_role0 from regress_du_role1 (inherit) + | regress_du_role0 from regress_du_role2 (set) regress_du_role2 | regress_du_role0 from regress_du_admin (admin) + | regress_du_role0 from regress_du_role1 (inherit, set) + | regress_du_role0 from regress_du_role2 (empty) + | regress_du_role1 from regress_du_admin (admin, set)(5 rows)
>
> The tabular form solves the latest patch translation problems mentioned by Kyotaro.
> But it requires mapping elements between 3 array-like columns.
>
> To move forward, needs more opinions?
[RMT Hat]
Nudging this along, as it's an open item. It'd be good to get this
resolved before Beta 1, but that may be tough at this point.
[Personal hat]
I'm probably not the target user for this feature, so I'm not sure how
much you should weigh my opinion (e.g. I still don't agree with
explicitly showing "empty", but as mentioned, I'm not the target user).
That said, from a readability standpoint, it was easier for me to follow
the tabular form vs. the sentence form.
Thanks,
Jonathan
Attachments:
[application/pgp-signature] OpenPGP_signature (840B, ../../[email protected]/2-OpenPGP_signature)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* [PATCH v3 4/4] Add invalidation-based retry loop for AlterPublication
@ 2026-07-03 14:46 Bertrand Drouvot <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Bertrand Drouvot @ 2026-07-03 14:46 UTC (permalink / raw)
Apply the same RangeVarGetRelidExtended() style retry loop to
AlterPublication()'s tables/schemas branch that was added for subscriptions
in commit XXXX.
Previously, this branch resolved the publication name and checked ownership
at the top of AlterPublication(), then locked and re-read by OID. This left a
window where concurrent DDL could have modified the ownership and/or the name
resolution
Now the tables/schemas branch has its own complete retry loop: name
resolution, ownership check, and lock acquisition all inside the loop.
Author: Bertrand Drouvot <[email protected]>
Reviewed-by:
Discussion: https://postgr.es/m/akZUpiDa1UfmzYxL%40bdtpg
---
src/backend/commands/publicationcmds.c | 105 +++++++++++++++++--------
1 file changed, 72 insertions(+), 33 deletions(-)
100.0% src/backend/commands/
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 440adb356ad..dfd707bc7d7 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -39,6 +39,7 @@
#include "parser/parse_relation.h"
#include "rewrite/rewriteHandler.h"
#include "storage/lmgr.h"
+#include "storage/sinval.h"
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/inval.h"
@@ -1662,54 +1663,92 @@ AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt)
rel = table_open(PublicationRelationId, RowExclusiveLock);
- tup = SearchSysCacheCopy1(PUBLICATIONNAME,
- CStringGetDatum(stmt->pubname));
+ if (stmt->options)
+ {
+ tup = SearchSysCacheCopy1(PUBLICATIONNAME,
+ CStringGetDatum(stmt->pubname));
- if (!HeapTupleIsValid(tup))
- ereport(ERROR,
- (errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("publication \"%s\" does not exist",
- stmt->pubname)));
+ if (!HeapTupleIsValid(tup))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("publication \"%s\" does not exist",
+ stmt->pubname)));
- pubform = (Form_pg_publication) GETSTRUCT(tup);
+ pubform = (Form_pg_publication) GETSTRUCT(tup);
- /* must be owner */
- if (!object_ownercheck(PublicationRelationId, pubform->oid, GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_PUBLICATION,
- stmt->pubname);
+ /* must be owner */
+ if (!object_ownercheck(PublicationRelationId, pubform->oid,
+ GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_PUBLICATION,
+ stmt->pubname);
- if (stmt->options)
AlterPublicationOptions(pstate, stmt, rel, tup);
+ }
else
{
List *relations = NIL;
List *exceptrelations = NIL;
List *schemaidlist = NIL;
- Oid pubid = pubform->oid;
+ Oid pubid;
- ObjectsInPublicationToOids(stmt->pubobjects, pstate, &relations,
- &exceptrelations, &schemaidlist);
+ /*
+ * Lock the publication so nobody else can do anything with it.
+ *
+ * Like RangeVarGetRelidExtended() does for relations, we resolve the
+ * name, check ownership, and lock inside a loop. If invalidation
+ * messages arrive (indicating concurrent DDL), we retry. We keep the
+ * lock held across retries and only release it if the name resolves
+ * to a different OID on the next iteration.
+ */
+ {
+ Oid oldPubId = InvalidOid;
+ bool retry = false;
- CheckAlterPublication(stmt, tup, relations, schemaidlist);
+ for (;;)
+ {
+ uint64 inval_count = SharedInvalidMessageCounter;
- heap_freetuple(tup);
+ tup = SearchSysCacheCopy1(PUBLICATIONNAME,
+ CStringGetDatum(stmt->pubname));
- /* Lock the publication so nobody else can do anything with it. */
- LockDatabaseObject(PublicationRelationId, pubid, 0,
- AccessExclusiveLock);
+ if (!HeapTupleIsValid(tup))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("publication \"%s\" does not exist",
+ stmt->pubname)));
- /*
- * It is possible that by the time we acquire the lock on publication,
- * concurrent DDL has removed it. We can test this by checking the
- * existence of publication. We get the tuple again to avoid the risk
- * of any publication option getting changed.
- */
- tup = SearchSysCacheCopy1(PUBLICATIONOID, ObjectIdGetDatum(pubid));
- if (!HeapTupleIsValid(tup))
- ereport(ERROR,
- errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("publication \"%s\" does not exist",
- stmt->pubname));
+ pubform = (Form_pg_publication) GETSTRUCT(tup);
+ pubid = pubform->oid;
+
+ if (!object_ownercheck(PublicationRelationId, pubid,
+ GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_PUBLICATION,
+ stmt->pubname);
+
+ if (retry)
+ {
+ if (pubid == oldPubId)
+ break;
+ UnlockDatabaseObject(PublicationRelationId, oldPubId, 0,
+ AccessExclusiveLock);
+ }
+
+ LockDatabaseObject(PublicationRelationId, pubid, 0,
+ AccessExclusiveLock);
+
+ if (inval_count == SharedInvalidMessageCounter)
+ break;
+
+ retry = true;
+ oldPubId = pubid;
+ heap_freetuple(tup);
+ }
+ }
+
+ ObjectsInPublicationToOids(stmt->pubobjects, pstate, &relations,
+ &exceptrelations, &schemaidlist);
+
+ CheckAlterPublication(stmt, tup, relations, schemaidlist);
relations = list_concat(relations, exceptrelations);
AlterPublicationTables(stmt, tup, relations, pstate->p_sourcetext,
--
2.34.1
--3eAVwo4vDNlHc8RC--
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2026-07-03 14:46 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-04-05 07:42 ` Pavel Luzanov <[email protected]>
2023-04-05 13:58 ` Tom Lane <[email protected]>
2023-04-05 14:24 ` David G. Johnston <[email protected]>
2023-04-13 12:44 ` Pavel Luzanov <[email protected]>
2023-05-03 16:00 ` Jonathan S. Katz <[email protected]>
2023-05-03 16:13 ` David G. Johnston <[email protected]>
2023-05-03 16:25 ` Tom Lane <[email protected]>
2023-05-03 16:30 ` Jonathan S. Katz <[email protected]>
2023-05-05 16:51 ` David G. Johnston <[email protected]>
2023-05-07 19:14 ` Pavel Luzanov <[email protected]>
2023-05-18 02:42 ` Jonathan S. Katz <[email protected]>
2026-07-03 14:46 [PATCH v3 4/4] Add invalidation-based retry loop for AlterPublication Bertrand Drouvot <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox