public inbox for [email protected]help / color / mirror / Atom feed
Re: psql: Add role's membership options to the \du+ command 19+ 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; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-04-05 13:58 Tom Lane <[email protected]> 1 sibling, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-04-05 14:24 David G. Johnston <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-04-13 12:44 Pavel Luzanov <[email protected]> parent: David G. Johnston <[email protected]> 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-05-03 16:00 Jonathan S. Katz <[email protected]> parent: Pavel Luzanov <[email protected]> 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-05-03 16:13 David G. Johnston <[email protected]> parent: Jonathan S. Katz <[email protected]> 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-05-03 16:25 Tom Lane <[email protected]> parent: David G. Johnston <[email protected]> 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-05-03 16:30 Jonathan S. Katz <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-05-05 16:51 David G. Johnston <[email protected]> parent: Jonathan S. Katz <[email protected]> 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-05-07 19:14 Pavel Luzanov <[email protected]> parent: David G. Johnston <[email protected]> 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* Re: psql: Add role's membership options to the \du+ command @ 2023-05-18 02:42 Jonathan S. Katz <[email protected]> parent: Pavel Luzanov <[email protected]> 0 siblings, 0 replies; 19+ 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] 19+ messages in thread
* [PATCH v3 3/4] Add invalidation-based retry loop for Alter/Drop Subscription @ 2026-07-03 14:03 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 19+ messages in thread From: Bertrand Drouvot @ 2026-07-03 14:03 UTC (permalink / raw) Following the approach of RangeVarGetRelidExtended() for relations, add a retry loop that includes name resolution, ownership check, and lock acquisition in AlterSubscription() and DropSubscription(). The loop records SharedInvalidMessageCounter, resolves the subscription name to an OID, checks ownership, then locks the subscription. If the invalidation counter changed (indicating concurrent DDL), we save the current OID and retry. On the next iteration, if the name still resolves to the same OID, we're done (already holding the correct lock). If it resolves to a different OID, we release the old lock and acquire the new one. This mirrors RangeVarGetRelidExtended()'s behavior: the lock is kept across retries to avoid a window where another session could have committed concurrent DDL modifying the ownership and/or the name resolution. Author: Bertrand Drouvot <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/akZUpiDa1UfmzYxL%40bdtpg --- src/backend/commands/subscriptioncmds.c | 167 +++++++++++++++++------- 1 file changed, 118 insertions(+), 49 deletions(-) 100.0% src/backend/commands/ diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index e23b366a87d..866341c2cfb 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -50,6 +50,7 @@ #include "replication/walsender.h" #include "replication/worker_internal.h" #include "storage/lmgr.h" +#include "storage/sinval.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/guc.h" @@ -1592,23 +1593,67 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, rel = table_open(SubscriptionRelationId, RowExclusiveLock); - /* Fetch the existing tuple. */ - tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, ObjectIdGetDatum(MyDatabaseId), - CStringGetDatum(stmt->subname)); + /* + * Lock the subscription 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 oldSubId = InvalidOid; + bool retry = false; - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); + for (;;) + { + uint64 inval_count = SharedInvalidMessageCounter; - form = (Form_pg_subscription) GETSTRUCT(tup); - subid = form->oid; + tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, + ObjectIdGetDatum(MyDatabaseId), + CStringGetDatum(stmt->subname)); - /* must be owner */ - if (!object_ownercheck(SubscriptionRelationId, subid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, - stmt->subname); + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("subscription \"%s\" does not exist", + stmt->subname))); + + form = (Form_pg_subscription) GETSTRUCT(tup); + subid = form->oid; + + if (!object_ownercheck(SubscriptionRelationId, subid, + GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, + stmt->subname); + + /* + * If upon retry we get the same OID, the invalidation messages + * did not change the final answer. So we're done. If we got a + * different OID, unlock the old one and lock the new one below. + */ + if (retry) + { + if (subid == oldSubId) + break; + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + } + + LockSharedObject(SubscriptionRelationId, subid, 0, + AccessExclusiveLock); + + /* If no invalidation messages, we're done. */ + if (inval_count == SharedInvalidMessageCounter) + break; + + /* Something may have changed, retry. */ + retry = true; + oldSubId = subid; + heap_freetuple(tup); + } + } /* parse and check options */ switch (stmt->kind) @@ -1686,25 +1731,6 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, orig_conninfo_needed = false; } - heap_freetuple(tup); - - /* Lock the subscription so nobody else can do anything with it. */ - LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock); - - /* - * Re-read the subscription tuple after acquiring the lock. A concurrent - * DROP or ALTER may have committed before we acquired the lock. - */ - tup = SearchSysCacheCopy1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); - - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); - - form = (Form_pg_subscription) GETSTRUCT(tup); - /* * Skip ACL checks on the subscription's foreign server, if any. If * changing the server (or replacing it with a raw connection), then the @@ -2570,11 +2596,6 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) form = (Form_pg_subscription) GETSTRUCT(tup); subid = form->oid; - /* must be owner */ - if (!object_ownercheck(SubscriptionRelationId, subid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, - stmt->subname); - /* DROP hook for the subscription being removed */ InvokeObjectDropHook(SubscriptionRelationId, subid, 0); @@ -2583,20 +2604,68 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) /* * Lock the subscription so nobody else can do anything with it (including * the replication workers). + * + * 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. */ - LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock); + { + Oid oldSubId = InvalidOid; + bool retry = false; - /* - * Re-read the subscription tuple after acquiring the lock. A concurrent - * ALTER or DROP may have committed before we acquired the lock. - */ - tup = SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); + for (;;) + { + uint64 inval_count = SharedInvalidMessageCounter; - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); + tup = SearchSysCache2(SUBSCRIPTIONNAME, + ObjectIdGetDatum(MyDatabaseId), + CStringGetDatum(stmt->subname)); + + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("subscription \"%s\" does not exist", + stmt->subname))); + + form = (Form_pg_subscription) GETSTRUCT(tup); + subid = form->oid; + + if (!object_ownercheck(SubscriptionRelationId, subid, + GetUserId())) + { + ReleaseSysCache(tup); + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, + stmt->subname); + } + + /* + * If upon retry we get the same OID, the invalidation messages + * did not change the final answer. So we're done. If we got a + * different OID, unlock the old one and lock the new one below. + */ + if (retry) + { + if (subid == oldSubId) + break; + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + } + + LockSharedObject(SubscriptionRelationId, subid, 0, + AccessExclusiveLock); + + /* If no invalidation messages, we're done. */ + if (inval_count == SharedInvalidMessageCounter) + break; + + /* Something may have changed, retry. */ + retry = true; + oldSubId = subid; + ReleaseSysCache(tup); + } + } form = (Form_pg_subscription) GETSTRUCT(tup); subowner = form->subowner; -- 2.34.1 --3eAVwo4vDNlHc8RC Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0004-Add-invalidation-based-retry-loop-for-AlterPublic.patch" ^ permalink raw reply [nested|flat] 19+ messages in thread
* [PATCH v3 3/4] Add invalidation-based retry loop for Alter/Drop Subscription @ 2026-07-03 14:03 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 19+ messages in thread From: Bertrand Drouvot @ 2026-07-03 14:03 UTC (permalink / raw) Following the approach of RangeVarGetRelidExtended() for relations, add a retry loop that includes name resolution, ownership check, and lock acquisition in AlterSubscription() and DropSubscription(). The loop records SharedInvalidMessageCounter, resolves the subscription name to an OID, checks ownership, then locks the subscription. If the invalidation counter changed (indicating concurrent DDL), we save the current OID and retry. On the next iteration, if the name still resolves to the same OID, we're done (already holding the correct lock). If it resolves to a different OID, we release the old lock and acquire the new one. This mirrors RangeVarGetRelidExtended()'s behavior: the lock is kept across retries to avoid a window where another session could have committed concurrent DDL modifying the ownership and/or the name resolution. Author: Bertrand Drouvot <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/akZUpiDa1UfmzYxL%40bdtpg --- src/backend/commands/subscriptioncmds.c | 167 +++++++++++++++++------- 1 file changed, 118 insertions(+), 49 deletions(-) 100.0% src/backend/commands/ diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index e23b366a87d..866341c2cfb 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -50,6 +50,7 @@ #include "replication/walsender.h" #include "replication/worker_internal.h" #include "storage/lmgr.h" +#include "storage/sinval.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/guc.h" @@ -1592,23 +1593,67 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, rel = table_open(SubscriptionRelationId, RowExclusiveLock); - /* Fetch the existing tuple. */ - tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, ObjectIdGetDatum(MyDatabaseId), - CStringGetDatum(stmt->subname)); + /* + * Lock the subscription 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 oldSubId = InvalidOid; + bool retry = false; - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); + for (;;) + { + uint64 inval_count = SharedInvalidMessageCounter; - form = (Form_pg_subscription) GETSTRUCT(tup); - subid = form->oid; + tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, + ObjectIdGetDatum(MyDatabaseId), + CStringGetDatum(stmt->subname)); - /* must be owner */ - if (!object_ownercheck(SubscriptionRelationId, subid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, - stmt->subname); + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("subscription \"%s\" does not exist", + stmt->subname))); + + form = (Form_pg_subscription) GETSTRUCT(tup); + subid = form->oid; + + if (!object_ownercheck(SubscriptionRelationId, subid, + GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, + stmt->subname); + + /* + * If upon retry we get the same OID, the invalidation messages + * did not change the final answer. So we're done. If we got a + * different OID, unlock the old one and lock the new one below. + */ + if (retry) + { + if (subid == oldSubId) + break; + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + } + + LockSharedObject(SubscriptionRelationId, subid, 0, + AccessExclusiveLock); + + /* If no invalidation messages, we're done. */ + if (inval_count == SharedInvalidMessageCounter) + break; + + /* Something may have changed, retry. */ + retry = true; + oldSubId = subid; + heap_freetuple(tup); + } + } /* parse and check options */ switch (stmt->kind) @@ -1686,25 +1731,6 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, orig_conninfo_needed = false; } - heap_freetuple(tup); - - /* Lock the subscription so nobody else can do anything with it. */ - LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock); - - /* - * Re-read the subscription tuple after acquiring the lock. A concurrent - * DROP or ALTER may have committed before we acquired the lock. - */ - tup = SearchSysCacheCopy1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); - - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); - - form = (Form_pg_subscription) GETSTRUCT(tup); - /* * Skip ACL checks on the subscription's foreign server, if any. If * changing the server (or replacing it with a raw connection), then the @@ -2570,11 +2596,6 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) form = (Form_pg_subscription) GETSTRUCT(tup); subid = form->oid; - /* must be owner */ - if (!object_ownercheck(SubscriptionRelationId, subid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, - stmt->subname); - /* DROP hook for the subscription being removed */ InvokeObjectDropHook(SubscriptionRelationId, subid, 0); @@ -2583,20 +2604,68 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) /* * Lock the subscription so nobody else can do anything with it (including * the replication workers). + * + * 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. */ - LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock); + { + Oid oldSubId = InvalidOid; + bool retry = false; - /* - * Re-read the subscription tuple after acquiring the lock. A concurrent - * ALTER or DROP may have committed before we acquired the lock. - */ - tup = SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); + for (;;) + { + uint64 inval_count = SharedInvalidMessageCounter; - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); + tup = SearchSysCache2(SUBSCRIPTIONNAME, + ObjectIdGetDatum(MyDatabaseId), + CStringGetDatum(stmt->subname)); + + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("subscription \"%s\" does not exist", + stmt->subname))); + + form = (Form_pg_subscription) GETSTRUCT(tup); + subid = form->oid; + + if (!object_ownercheck(SubscriptionRelationId, subid, + GetUserId())) + { + ReleaseSysCache(tup); + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, + stmt->subname); + } + + /* + * If upon retry we get the same OID, the invalidation messages + * did not change the final answer. So we're done. If we got a + * different OID, unlock the old one and lock the new one below. + */ + if (retry) + { + if (subid == oldSubId) + break; + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + } + + LockSharedObject(SubscriptionRelationId, subid, 0, + AccessExclusiveLock); + + /* If no invalidation messages, we're done. */ + if (inval_count == SharedInvalidMessageCounter) + break; + + /* Something may have changed, retry. */ + retry = true; + oldSubId = subid; + ReleaseSysCache(tup); + } + } form = (Form_pg_subscription) GETSTRUCT(tup); subowner = form->subowner; -- 2.34.1 --3eAVwo4vDNlHc8RC Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v3-0004-Add-invalidation-based-retry-loop-for-AlterPublic.patch" ^ permalink raw reply [nested|flat] 19+ messages in thread
* [PATCH v4 4/4] Add invalidation-based retry loop for AlterPublication @ 2026-07-03 14:46 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 19+ 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: Dilip Kumar <[email protected]> Reviewed-by: Hayato Kuroda (Fujitsu) <[email protected]> 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 --b+bwjVhifbzgesmw-- ^ permalink raw reply [nested|flat] 19+ messages in thread
* [PATCH v4 4/4] Add invalidation-based retry loop for AlterPublication @ 2026-07-03 14:46 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 19+ 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: Dilip Kumar <[email protected]> Reviewed-by: Hayato Kuroda (Fujitsu) <[email protected]> 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 --b+bwjVhifbzgesmw-- ^ permalink raw reply [nested|flat] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ messages in thread
* [PATCH v4 3/4] Add invalidation-based retry loop for Alter/Drop Subscription @ 2026-07-06 04:44 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 19+ messages in thread From: Bertrand Drouvot @ 2026-07-06 04:44 UTC (permalink / raw) Following the approach of RangeVarGetRelidExtended() for relations, add a retry loop that includes name resolution, ownership check, and lock acquisition in AlterSubscription() and DropSubscription(). The loop records SharedInvalidMessageCounter, resolves the subscription name to an OID, checks ownership, then locks the subscription. If the invalidation counter changed (indicating concurrent DDL), we save the current OID and retry. On the next iteration, if the name still resolves to the same OID, we're done (already holding the correct lock). If it resolves to a different OID, we release the old lock and acquire the new one. This mirrors RangeVarGetRelidExtended()'s behavior: the lock is kept across retries to avoid a window where another session could have committed concurrent DDL modifying the ownership and/or the name resolution. Author: Bertrand Drouvot <[email protected]> Reviewed-by: Dilip Kumar <[email protected]> Reviewed-by: Hayato Kuroda (Fujitsu) <[email protected]> Discussion: https://postgr.es/m/akZUpiDa1UfmzYxL%40bdtpg --- src/backend/commands/subscriptioncmds.c | 189 +++++++++++++++++------- 1 file changed, 134 insertions(+), 55 deletions(-) 100.0% src/backend/commands/ diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index c9e7fbdb47b..615c3921bc3 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -50,6 +50,7 @@ #include "replication/walsender.h" #include "replication/worker_internal.h" #include "storage/lmgr.h" +#include "storage/sinval.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/guc.h" @@ -1592,23 +1593,67 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, rel = table_open(SubscriptionRelationId, RowExclusiveLock); - /* Fetch the existing tuple. */ - tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, ObjectIdGetDatum(MyDatabaseId), - CStringGetDatum(stmt->subname)); + /* + * Lock the subscription 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 oldSubId = InvalidOid; + bool retry = false; - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); + for (;;) + { + uint64 inval_count = SharedInvalidMessageCounter; - form = (Form_pg_subscription) GETSTRUCT(tup); - subid = form->oid; + tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, + ObjectIdGetDatum(MyDatabaseId), + CStringGetDatum(stmt->subname)); - /* must be owner */ - if (!object_ownercheck(SubscriptionRelationId, subid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, - stmt->subname); + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("subscription \"%s\" does not exist", + stmt->subname))); + + form = (Form_pg_subscription) GETSTRUCT(tup); + subid = form->oid; + + if (!object_ownercheck(SubscriptionRelationId, subid, + GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, + stmt->subname); + + /* + * If upon retry we get the same OID, the invalidation messages + * did not change the final answer. So we're done. If we got a + * different OID, unlock the old one and lock the new one below. + */ + if (retry) + { + if (subid == oldSubId) + break; + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + } + + LockSharedObject(SubscriptionRelationId, subid, 0, + AccessExclusiveLock); + + /* If no invalidation messages, we're done. */ + if (inval_count == SharedInvalidMessageCounter) + break; + + /* Something may have changed, retry. */ + retry = true; + oldSubId = subid; + heap_freetuple(tup); + } + } /* parse and check options */ switch (stmt->kind) @@ -1686,25 +1731,6 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, orig_conninfo_needed = false; } - heap_freetuple(tup); - - /* Lock the subscription so nobody else can do anything with it. */ - LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock); - - /* - * Re-read the subscription tuple after acquiring the lock. A concurrent - * DROP or ALTER may have committed before we acquired the lock. - */ - tup = SearchSysCacheCopy1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); - - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); - - form = (Form_pg_subscription) GETSTRUCT(tup); - /* * Skip ACL checks on the subscription's foreign server, if any. If * changing the server (or replacing it with a raw connection), then the @@ -2567,38 +2593,91 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) return; } - form = (Form_pg_subscription) GETSTRUCT(tup); - subid = form->oid; - - /* must be owner */ - if (!object_ownercheck(SubscriptionRelationId, subid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, - stmt->subname); - ReleaseSysCache(tup); /* * Lock the subscription so nobody else can do anything with it (including * the replication workers). + * + * 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. */ - LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock); + { + Oid oldSubId = InvalidOid; + bool retry = false; - /* DROP hook for the subscription being removed */ - InvokeObjectDropHook(SubscriptionRelationId, subid, 0); + for (;;) + { + uint64 inval_count = SharedInvalidMessageCounter; - /* - * Re-read the subscription tuple after acquiring the lock. A concurrent - * ALTER or DROP may have committed before we acquired the lock. - */ - tup = SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); + tup = SearchSysCache2(SUBSCRIPTIONNAME, + ObjectIdGetDatum(MyDatabaseId), + CStringGetDatum(stmt->subname)); - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); + if (!HeapTupleIsValid(tup)) + { + if (retry) + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + table_close(rel, NoLock); + + if (!stmt->missing_ok) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("subscription \"%s\" does not exist", + stmt->subname))); + else + ereport(NOTICE, + (errmsg("subscription \"%s\" does not exist, skipping", + stmt->subname))); + + return; + } + + form = (Form_pg_subscription) GETSTRUCT(tup); + subid = form->oid; + + if (!object_ownercheck(SubscriptionRelationId, subid, + GetUserId())) + { + ReleaseSysCache(tup); + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, + stmt->subname); + } + + /* + * If upon retry we get the same OID, the invalidation messages + * did not change the final answer. So we're done. If we got a + * different OID, unlock the old one and lock the new one below. + */ + if (retry) + { + if (subid == oldSubId) + break; + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + } + + LockSharedObject(SubscriptionRelationId, subid, 0, + AccessExclusiveLock); + + /* If no invalidation messages, we're done. */ + if (inval_count == SharedInvalidMessageCounter) + break; + + /* Something may have changed, retry. */ + retry = true; + oldSubId = subid; + ReleaseSysCache(tup); + } + } + + /* DROP hook for the subscription being removed */ + InvokeObjectDropHook(SubscriptionRelationId, subid, 0); - form = (Form_pg_subscription) GETSTRUCT(tup); subowner = form->subowner; subserver = form->subserver; subconflictlogrelid = form->subconflictlogrelid; -- 2.34.1 --b+bwjVhifbzgesmw Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0004-Add-invalidation-based-retry-loop-for-AlterPublic.patch" ^ permalink raw reply [nested|flat] 19+ messages in thread
* [PATCH v4 3/4] Add invalidation-based retry loop for Alter/Drop Subscription @ 2026-07-06 04:44 Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 19+ messages in thread From: Bertrand Drouvot @ 2026-07-06 04:44 UTC (permalink / raw) Following the approach of RangeVarGetRelidExtended() for relations, add a retry loop that includes name resolution, ownership check, and lock acquisition in AlterSubscription() and DropSubscription(). The loop records SharedInvalidMessageCounter, resolves the subscription name to an OID, checks ownership, then locks the subscription. If the invalidation counter changed (indicating concurrent DDL), we save the current OID and retry. On the next iteration, if the name still resolves to the same OID, we're done (already holding the correct lock). If it resolves to a different OID, we release the old lock and acquire the new one. This mirrors RangeVarGetRelidExtended()'s behavior: the lock is kept across retries to avoid a window where another session could have committed concurrent DDL modifying the ownership and/or the name resolution. Author: Bertrand Drouvot <[email protected]> Reviewed-by: Dilip Kumar <[email protected]> Reviewed-by: Hayato Kuroda (Fujitsu) <[email protected]> Discussion: https://postgr.es/m/akZUpiDa1UfmzYxL%40bdtpg --- src/backend/commands/subscriptioncmds.c | 189 +++++++++++++++++------- 1 file changed, 134 insertions(+), 55 deletions(-) 100.0% src/backend/commands/ diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index c9e7fbdb47b..615c3921bc3 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -50,6 +50,7 @@ #include "replication/walsender.h" #include "replication/worker_internal.h" #include "storage/lmgr.h" +#include "storage/sinval.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/guc.h" @@ -1592,23 +1593,67 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, rel = table_open(SubscriptionRelationId, RowExclusiveLock); - /* Fetch the existing tuple. */ - tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, ObjectIdGetDatum(MyDatabaseId), - CStringGetDatum(stmt->subname)); + /* + * Lock the subscription 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 oldSubId = InvalidOid; + bool retry = false; - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); + for (;;) + { + uint64 inval_count = SharedInvalidMessageCounter; - form = (Form_pg_subscription) GETSTRUCT(tup); - subid = form->oid; + tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, + ObjectIdGetDatum(MyDatabaseId), + CStringGetDatum(stmt->subname)); - /* must be owner */ - if (!object_ownercheck(SubscriptionRelationId, subid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, - stmt->subname); + if (!HeapTupleIsValid(tup)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("subscription \"%s\" does not exist", + stmt->subname))); + + form = (Form_pg_subscription) GETSTRUCT(tup); + subid = form->oid; + + if (!object_ownercheck(SubscriptionRelationId, subid, + GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, + stmt->subname); + + /* + * If upon retry we get the same OID, the invalidation messages + * did not change the final answer. So we're done. If we got a + * different OID, unlock the old one and lock the new one below. + */ + if (retry) + { + if (subid == oldSubId) + break; + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + } + + LockSharedObject(SubscriptionRelationId, subid, 0, + AccessExclusiveLock); + + /* If no invalidation messages, we're done. */ + if (inval_count == SharedInvalidMessageCounter) + break; + + /* Something may have changed, retry. */ + retry = true; + oldSubId = subid; + heap_freetuple(tup); + } + } /* parse and check options */ switch (stmt->kind) @@ -1686,25 +1731,6 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, orig_conninfo_needed = false; } - heap_freetuple(tup); - - /* Lock the subscription so nobody else can do anything with it. */ - LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock); - - /* - * Re-read the subscription tuple after acquiring the lock. A concurrent - * DROP or ALTER may have committed before we acquired the lock. - */ - tup = SearchSysCacheCopy1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); - - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); - - form = (Form_pg_subscription) GETSTRUCT(tup); - /* * Skip ACL checks on the subscription's foreign server, if any. If * changing the server (or replacing it with a raw connection), then the @@ -2567,38 +2593,91 @@ DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel) return; } - form = (Form_pg_subscription) GETSTRUCT(tup); - subid = form->oid; - - /* must be owner */ - if (!object_ownercheck(SubscriptionRelationId, subid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, - stmt->subname); - ReleaseSysCache(tup); /* * Lock the subscription so nobody else can do anything with it (including * the replication workers). + * + * 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. */ - LockSharedObject(SubscriptionRelationId, subid, 0, AccessExclusiveLock); + { + Oid oldSubId = InvalidOid; + bool retry = false; - /* DROP hook for the subscription being removed */ - InvokeObjectDropHook(SubscriptionRelationId, subid, 0); + for (;;) + { + uint64 inval_count = SharedInvalidMessageCounter; - /* - * Re-read the subscription tuple after acquiring the lock. A concurrent - * ALTER or DROP may have committed before we acquired the lock. - */ - tup = SearchSysCache1(SUBSCRIPTIONOID, ObjectIdGetDatum(subid)); + tup = SearchSysCache2(SUBSCRIPTIONNAME, + ObjectIdGetDatum(MyDatabaseId), + CStringGetDatum(stmt->subname)); - if (!HeapTupleIsValid(tup)) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("subscription \"%s\" does not exist", - stmt->subname))); + if (!HeapTupleIsValid(tup)) + { + if (retry) + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + table_close(rel, NoLock); + + if (!stmt->missing_ok) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("subscription \"%s\" does not exist", + stmt->subname))); + else + ereport(NOTICE, + (errmsg("subscription \"%s\" does not exist, skipping", + stmt->subname))); + + return; + } + + form = (Form_pg_subscription) GETSTRUCT(tup); + subid = form->oid; + + if (!object_ownercheck(SubscriptionRelationId, subid, + GetUserId())) + { + ReleaseSysCache(tup); + aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_SUBSCRIPTION, + stmt->subname); + } + + /* + * If upon retry we get the same OID, the invalidation messages + * did not change the final answer. So we're done. If we got a + * different OID, unlock the old one and lock the new one below. + */ + if (retry) + { + if (subid == oldSubId) + break; + UnlockSharedObject(SubscriptionRelationId, oldSubId, 0, + AccessExclusiveLock); + } + + LockSharedObject(SubscriptionRelationId, subid, 0, + AccessExclusiveLock); + + /* If no invalidation messages, we're done. */ + if (inval_count == SharedInvalidMessageCounter) + break; + + /* Something may have changed, retry. */ + retry = true; + oldSubId = subid; + ReleaseSysCache(tup); + } + } + + /* DROP hook for the subscription being removed */ + InvokeObjectDropHook(SubscriptionRelationId, subid, 0); - form = (Form_pg_subscription) GETSTRUCT(tup); subowner = form->subowner; subserver = form->subserver; subconflictlogrelid = form->subconflictlogrelid; -- 2.34.1 --b+bwjVhifbzgesmw Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0004-Add-invalidation-based-retry-loop-for-AlterPublic.patch" ^ permalink raw reply [nested|flat] 19+ messages in thread
end of thread, other threads:[~2026-07-06 04:44 UTC | newest] Thread overview: 19+ 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:03 [PATCH v3 3/4] Add invalidation-based retry loop for Alter/Drop Subscription Bertrand Drouvot <[email protected]> 2026-07-03 14:03 [PATCH v3 3/4] Add invalidation-based retry loop for Alter/Drop Subscription Bertrand Drouvot <[email protected]> 2026-07-03 14:46 [PATCH v3 4/4] Add invalidation-based retry loop for AlterPublication Bertrand Drouvot <[email protected]> 2026-07-03 14:46 [PATCH v4 4/4] Add invalidation-based retry loop for AlterPublication Bertrand Drouvot <[email protected]> 2026-07-03 14:46 [PATCH v3 4/4] Add invalidation-based retry loop for AlterPublication Bertrand Drouvot <[email protected]> 2026-07-03 14:46 [PATCH v4 4/4] Add invalidation-based retry loop for AlterPublication Bertrand Drouvot <[email protected]> 2026-07-06 04:44 [PATCH v4 3/4] Add invalidation-based retry loop for Alter/Drop Subscription Bertrand Drouvot <[email protected]> 2026-07-06 04:44 [PATCH v4 3/4] Add invalidation-based retry loop for Alter/Drop Subscription 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