From ffdb8bac10ecf40c40a3e8c441e299423ee07cef Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Thu, 16 Mar 2023 11:46:08 -0700 Subject: [PATCH] pg_dump: skip lock for extension tables without policies [backport to 11] If a user without SELECT permissions on an internal extension table tries to dump the extension, the dump will fail while trying to lock the table with ACCESS SHARE, even though the user doesn't want or need to dump the table in question. (The lock is taken to allow later pg_get_expr() calls on pg_policy to remain consistent in the face of concurrent schema changes.) It'd be ideal not to require SELECT permissions on a table to be able to dump its policies, but I don't have a great idea for how to implement that without races. As a workaround, skip the policy queries entirely if we determine that no policies exist for a table at the time of getTables(). --- src/bin/pg_dump/pg_dump.c | 41 ++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 56d5571366..125c6038c6 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -5982,6 +5982,7 @@ getTables(Archive *fout, int *numTables) int i_is_identity_sequence; int i_changed_acl; int i_ispartition; + int i_has_policies; /* * Find all the tables and table-like objects. @@ -6081,7 +6082,8 @@ getTables(Archive *fout, int *numTables) "OR %s IS NOT NULL" "))" "AS changed_acl, " - "%s AS ispartition " + "%s AS ispartition, " + "EXISTS (SELECT 1 FROM pg_policy WHERE polrelid = c.oid) AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6152,7 +6154,8 @@ getTables(Archive *fout, int *numTables) "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, " "tc.reloptions AS toast_reloptions, " "NULL AS changed_acl, " - "false AS ispartition " + "false AS ispartition, " + "EXISTS (SELECT 1 FROM pg_policy WHERE polrelid = c.oid) AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6199,7 +6202,8 @@ getTables(Archive *fout, int *numTables) "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, " "tc.reloptions AS toast_reloptions, " "NULL AS changed_acl, " - "false AS ispartition " + "false AS ispartition, " + "false AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6246,7 +6250,8 @@ getTables(Archive *fout, int *numTables) "WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, " "tc.reloptions AS toast_reloptions, " "NULL AS changed_acl, " - "false AS ispartition " + "false AS ispartition, " + "false AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6291,7 +6296,8 @@ getTables(Archive *fout, int *numTables) "c.reloptions AS reloptions, " "tc.reloptions AS toast_reloptions, " "NULL AS changed_acl, " - "false AS ispartition " + "false AS ispartition, " + "false AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6336,7 +6342,8 @@ getTables(Archive *fout, int *numTables) "c.reloptions AS reloptions, " "tc.reloptions AS toast_reloptions, " "NULL AS changed_acl, " - "false AS ispartition " + "false AS ispartition, " + "false AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6380,7 +6387,8 @@ getTables(Archive *fout, int *numTables) "c.reloptions AS reloptions, " "tc.reloptions AS toast_reloptions, " "NULL AS changed_acl, " - "false AS ispartition " + "false AS ispartition, " + "false AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6424,7 +6432,8 @@ getTables(Archive *fout, int *numTables) "c.reloptions AS reloptions, " "NULL AS toast_reloptions, " "NULL AS changed_acl, " - "false AS ispartition " + "false AS ispartition, " + "false AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6467,7 +6476,8 @@ getTables(Archive *fout, int *numTables) "NULL AS reloptions, " "NULL AS toast_reloptions, " "NULL AS changed_acl, " - "false AS ispartition " + "false AS ispartition, " + "false AS has_policies " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -6535,6 +6545,7 @@ getTables(Archive *fout, int *numTables) i_is_identity_sequence = PQfnumber(res, "is_identity_sequence"); i_changed_acl = PQfnumber(res, "changed_acl"); i_ispartition = PQfnumber(res, "ispartition"); + i_has_policies = PQfnumber(res, "has_policies"); if (dopt->lockWaitTimeout) { @@ -6627,6 +6638,18 @@ getTables(Archive *fout, int *numTables) strcmp(PQgetvalue(res, i, i_changed_acl), "f") == 0) tblinfo[i].dobj.dump &= ~DUMP_COMPONENT_ACL; + /* + * Similarly, if the table has no policies, we don't need to worry about + * those either. + * + * For tables internal to an extension, this may mean we don't need to + * take an ACCESS SHARE lock, which in turn allows less privileged users + * to successfully perform a dump if they don't have SELECT access to + * those tables (which they weren't trying to dump in the first place). + */ + if (strcmp(PQgetvalue(res, i, i_has_policies), "f") == 0) + tblinfo[i].dobj.dump &= ~DUMP_COMPONENT_POLICY; + tblinfo[i].interesting = tblinfo[i].dobj.dump ? true : false; tblinfo[i].dummy_view = false; /* might get set during sort */ tblinfo[i].postponed_def = false; /* might get set during sort */ -- 2.25.1