Find dangling membership roles in pg_dumpall
Hello,
pg_dumpall.c has a function dumpRoleMembership() which dumps all
membership roles. This function includes a piece of code which checks
if the membership tree has an open end which can't be resolved.
However that code is never used.
The variable prev_remaining is initially set to 0, and then never changed.
Which in turn never executes this code:
if (remaining == prev_remaining)
because the loop is only entered while "remaining > 0".
The attached patch fixes this problem, and updates prev_remaining inside
the loop.
Co-Author: Artur Zakirov <zaartur@gmail.com>
who reviewed the patch.
Regards
--
Andreas 'ads' Scherbaum
German PostgreSQL User Group
European PostgreSQL User Group - Board of Directors
Volunteer Regional Contact, Germany - PostgreSQL Project
Attachments:
pg_dumpall.patchtext/x-patch; charset=UTF-8; name=pg_dumpall.patchDownload
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index 71a1319865..1e2a32c39a 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1044,6 +1044,9 @@ dumpRoleMembership(PGconn *conn)
* graph whose vertices are grants and whose edges point from
* grantors to members should be connected and acyclic. If we fail
* to make progress, either we or the server have messed up.
+ * Initially $prev_remaining is 0, while initially $remaining can't
+ * be 0. If a subsequent loop produces no new results the loop
+ * aborts here.
*/
if (remaining == prev_remaining)
{
@@ -1052,6 +1055,7 @@ dumpRoleMembership(PGconn *conn)
PQfinish(conn);
exit_nicely(1);
}
+ prev_remaining = remaining;
/* Make one pass over the grants for this role. */
for (i = start; i < end; ++i)
On 26 Apr 2023, at 12:18, Andreas 'ads' Scherbaum <ads@pgug.de> wrote:
Hello,
pg_dumpall.c has a function dumpRoleMembership() which dumps all
membership roles. This function includes a piece of code which checks
if the membership tree has an open end which can't be resolved.
However that code is never used.The variable prev_remaining is initially set to 0, and then never changed.
Which in turn never executes this code:if (remaining == prev_remaining)
because the loop is only entered while "remaining > 0".
The attached patch fixes this problem, and updates prev_remaining inside
the loop.
Nice catch, that indeed seems like a proper fix. This was introduced in
ce6b672e44 and so doesn't need a backpatch.
--
Daniel Gustafsson
On 26 Apr 2023, at 13:02, Daniel Gustafsson <daniel@yesql.se> wrote:
On 26 Apr 2023, at 12:18, Andreas 'ads' Scherbaum <ads@pgug.de> wrote:
The attached patch fixes this problem, and updates prev_remaining inside
the loop.Nice catch, that indeed seems like a proper fix. This was introduced in
ce6b672e44 and so doesn't need a backpatch.
Pushed, but without the comment extension as I felt the existing comment aptly
discussed the codepath. Thanks!
--
Daniel Gustafsson
On Wed, Apr 26, 2023 at 8:29 AM Daniel Gustafsson <daniel@yesql.se> wrote:
Pushed, but without the comment extension as I felt the existing comment aptly
discussed the codepath. Thanks!
Woopsie. Thanks to both of you.
--
Robert Haas
EDB: http://www.enterprisedb.com