public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hüseyin Demir <[email protected]>
To: Laurenz Albe <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Greg Sabino Mullane <[email protected]>
Cc: [email protected]
Subject: Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table
Date: Fri, 10 Jul 2026 19:37:26 +0200
Message-ID: <CAB5wL7aTaOyijT3qJF9jAQNrv1K74sQkEyu+zkb54rYDc_NBdg@mail.gmail.com> (raw)
In-Reply-To: <CAB5wL7Y5WPpyfFQ0aLQhtNqDQucOMw+TvCC-noG4u=o=mnqtGQ@mail.gmail.com>
References: <[email protected]>
<CAKAnmmLYXcBSV8i5MVuiwu8tkX-JDGmZtJ38_YbdUTwoDW7xKg@mail.gmail.com>
<[email protected]>
<CAB5wL7bo-7okBCJ24fdVn4UhnuPLPG22eQjaLJZ9GJtv0FEWMg@mail.gmail.com>
<CAB5wL7bunPCi93Bwi7zN_1gFAZhKLwS=Fsub91SW417xujGtNA@mail.gmail.com>
<CAB5wL7aKGFFEF1SM54GxvaQPH=7Bs=HuUCqi7F4Ve025+Z950A@mail.gmail.com>
<CAB5wL7aK6UaukMSegzEG68y0VrKOg7wFnPNRaDaHreBf2iVWag@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAB5wL7ZuFwu2Sd-xmn-w5-BbrKk_Mu_1ubYow_qaqFGhJOiMXw@mail.gmail.com>
<CAB5wL7ai8AiaFV6B8J=w6vK1Q6Z9Hmp_y3PFL2Zzpx6Y8xPZ2A@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAB5wL7btFp-XRBfv_J-YperoK+e0uYje3ZYsfTzCuHcUWWrvbw@mail.gmail.com>
<CAB5wL7YFeL0VP7EZu6wtWx=UhXuDp3dTBVJwFvSRzBi7qNscfQ@mail.gmail.com>
<[email protected]>
<CAB5wL7Zy89DeVXHFvTL72KChkq=2jLXhH14zrC2n5sutLTqZDg@mail.gmail.com>
<[email protected]>
<CAB5wL7YQs7CDduxx0cv3iKPnQ7fxT-mj0AqPWv2WPO3aaF_VeQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAB5wL7Y5WPpyfFQ0aLQhtNqDQucOMw+TvCC-noG4u=o=mnqtGQ@mail.gmail.com>
> > > I'd say that your v6 patch is fine if the performance impact can be
> > > shown to be small.
> >
> > To keep the ball rolling, I ran a pg_upgrade --link from a debug-enabled
> > build on a database with 20000 empty tables.
> >
> > pg_upgrade took between 19 and 25 minutes, and I saw no noticeable effect
> > of the patch. So I guess this is good to go.
> >
> > Yours,
> > Laurenz Albe
>
> Thanks for the pg_upgrade test.
>
> Appreciated.
Rebased v6 on current master.
Attachments:
[application/octet-stream] v7-0001-pg_dump-skip-dangling-initprivs.patch (9.6K, ../CAB5wL7aTaOyijT3qJF9jAQNrv1K74sQkEyu+zkb54rYDc_NBdg@mail.gmail.com/2-v7-0001-pg_dump-skip-dangling-initprivs.patch)
download | inline diff:
From 3c76b1af9c64934797641f595d2c03b809c3088f Mon Sep 17 00:00:00 2001
From: huseyin <[email protected]>
Date: Wed, 24 Jun 2026 13:25:25 +0200
Subject: [PATCH] pg_dump: skip pg_init_privs entries for non-existent roles
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Before commit 53428740391, PostgreSQL did not record role dependencies
for pg_init_privs entries in pg_shdepend, meaning DROP ROLE could leave
behind ACL entries whose grantee or grantor OID no longer exists in
pg_authid. Cross-cluster restores can produce the same situation on
modern releases.
Dangling entries caused pg_dump to emit invalid SQL such as
"GRANT ... TO "87868"" with a numeric OID as role name, which fails
on restore or pg_upgrade. Additionally, when all initprivs entries for
an object are dangling and proacl is NULL, a spurious REVOKE was emitted.
Fix by filtering each aclitem whose grantor or non-PUBLIC grantee OID
does not appear in pg_roles (used instead of pg_authid to support
non-superuser pg_dump). The filtering is applied server-side in the
queries that fetch pg_init_privs data (getAdditionalACLs and the
column-level ACL prepared statement), where the OID check is
authoritative. This avoids modifying the WHERE clauses in
getAggregates/getFuncs, preserving pg_dump performance on large schemas.
If all entries for an object are dangling the result is NULL and no ACL
is emitted, which is correct -- we cannot restore grants involving roles
that do not exist.
Author: Hüseyin Demir <[email protected]>
Discussion: https://postgr.es/m/19483-80de42dc4e62cfd6%40postgresql.org
Backpatch-through: 14
---
src/bin/pg_dump/pg_dump.c | 37 ++++-
.../t/008_pg_dump_dangling_initprivs.pl | 140 ++++++++++++++++++
2 files changed, 174 insertions(+), 3 deletions(-)
create mode 100644 src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4d660d14b4c..9a720f6fa4c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -237,6 +237,35 @@ static int nsequences = 0;
fmtQualifiedId((obj)->dobj.namespace->dobj.name, \
(obj)->dobj.name)
+/*
+ * SQL expression that filters dangling role OIDs from a pg_init_privs
+ * aclitem[] column. An aclitem is dangling when its grantor or non-PUBLIC
+ * grantee OID no longer exists in pg_roles. We use pg_roles rather than
+ * pg_authid so that non-superuser pg_dump works.
+ *
+ * Applied only in queries that fetch pg_init_privs data (not in WHERE clauses
+ * of per-object queries) to avoid running this subquery per function/aggregate.
+ */
+#define SAFE_INITPRIVS(col) \
+ "NULLIF(\n" \
+ " ARRAY(\n" \
+ " SELECT elt FROM pg_catalog.unnest(" col ") AS elt\n" \
+ " WHERE NOT EXISTS (\n" \
+ " SELECT 1 FROM pg_catalog.aclexplode(ARRAY[elt]) ace\n" \
+ " WHERE NOT EXISTS (\n" \
+ " SELECT 1 FROM pg_catalog.pg_roles\n" \
+ " WHERE oid = ace.grantor\n" \
+ " )\n" \
+ " OR (ace.grantee <> 0\n" \
+ " AND NOT EXISTS (\n" \
+ " SELECT 1 FROM pg_catalog.pg_roles\n" \
+ " WHERE oid = ace.grantee\n" \
+ " ))\n" \
+ " )\n" \
+ " ),\n" \
+ " ARRAY[]::pg_catalog.aclitem[]\n" \
+ ")"
+
static void help(const char *progname);
static void setup_connection(Archive *AH,
const char *dumpencoding, const char *dumpsnapshot,
@@ -10748,8 +10777,9 @@ getAdditionalACLs(Archive *fout)
/* Fetch initial-privileges data */
printfPQExpBuffer(query,
- "SELECT objoid, classoid, objsubid, privtype, initprivs "
- "FROM pg_init_privs");
+ "SELECT pip.objoid, pip.classoid, pip.objsubid, pip.privtype,\n"
+ " " SAFE_INITPRIVS("pip.initprivs") " AS initprivs\n"
+ "FROM pg_catalog.pg_init_privs pip");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -16892,7 +16922,8 @@ dumpTable(Archive *fout, const TableInfo *tbinfo)
"SELECT at.attname, "
"at.attacl, "
"'{}' AS acldefault, "
- "pip.privtype, pip.initprivs "
+ "pip.privtype,\n"
+ " " SAFE_INITPRIVS("pip.initprivs") " AS initprivs\n"
"FROM pg_catalog.pg_attribute at "
"LEFT JOIN pg_catalog.pg_init_privs pip ON "
"(at.attrelid = pip.objoid "
diff --git a/src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl b/src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl
new file mode 100644
index 00000000000..1580a25638c
--- /dev/null
+++ b/src/bin/pg_dump/t/008_pg_dump_dangling_initprivs.pl
@@ -0,0 +1,140 @@
+# Copyright (c) 2024-2026, PostgreSQL Global Development Group
+#
+# Tests that pg_dump silently skips pg_init_privs entries that reference
+# roles no longer present in pg_authid, rather than emitting invalid GRANT
+# statements with numeric OIDs as role names.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE DATABASE regress_dangling');
+
+# --- Setup ---
+# Simulate dangling pg_init_privs entries by inserting grants for a role
+# and then deleting the role directly from pg_authid (bypassing pg_shdepend).
+$node->safe_psql(
+ 'regress_dangling',
+ q{
+SET allow_system_table_mods = true;
+
+-- Roles for testing
+CREATE ROLE ghost_grantee;
+CREATE ROLE ghost_grantor;
+CREATE ROLE "007";
+
+-- Case 1: dangling grantee (function)
+CREATE FUNCTION public.test_func_grantee() RETURNS int LANGUAGE sql AS 'SELECT 1';
+INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs)
+SELECT p.oid,
+ (SELECT oid FROM pg_class WHERE relname = 'pg_proc'),
+ 0, 'e',
+ ARRAY[('ghost_grantee=X/' || current_user)::aclitem]
+FROM pg_proc p
+WHERE p.proname = 'test_func_grantee'
+ AND p.pronamespace = 'public'::regnamespace;
+
+-- Case 2: dangling grantor (function)
+CREATE FUNCTION public.test_func_grantor() RETURNS int LANGUAGE sql AS 'SELECT 2';
+INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs)
+SELECT p.oid,
+ (SELECT oid FROM pg_class WHERE relname = 'pg_proc'),
+ 0, 'e',
+ ARRAY[(current_user || '=X/ghost_grantor')::aclitem]
+FROM pg_proc p
+WHERE p.proname = 'test_func_grantor'
+ AND p.pronamespace = 'public'::regnamespace;
+
+-- Case 3: dangling column-level grantee (table)
+CREATE TABLE public.test_tbl (id int, secret text);
+INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs)
+SELECT c.oid,
+ (SELECT oid FROM pg_class WHERE relname = 'pg_class'),
+ 2, 'e',
+ ARRAY[('ghost_grantee=r/' || current_user)::aclitem]
+FROM pg_class c
+WHERE c.relname = 'test_tbl'
+ AND c.relnamespace = 'public'::regnamespace;
+
+-- Case 4: spurious REVOKE -- all-dangling initprivs on a catalog function
+-- with NULL proacl (simulates the spurious-selection scenario)
+CREATE FUNCTION public.test_func_revoke() RETURNS int LANGUAGE sql AS 'SELECT 3';
+INSERT INTO pg_init_privs (objoid, classoid, objsubid, privtype, initprivs)
+SELECT p.oid,
+ (SELECT oid FROM pg_class WHERE relname = 'pg_proc'),
+ 0, 'e',
+ ARRAY[('ghost_grantee=X/' || current_user)::aclitem]
+FROM pg_proc p
+WHERE p.proname = 'test_func_revoke'
+ AND p.pronamespace = 'public'::regnamespace;
+
+-- Case 5: valid all-digit role "007" with a grant (must NOT be filtered)
+CREATE FUNCTION public.test_func_007() RETURNS int LANGUAGE sql AS 'SELECT 7';
+GRANT EXECUTE ON FUNCTION public.test_func_007() TO "007";
+
+-- Now delete the ghost roles to create dangling OIDs
+DELETE FROM pg_authid WHERE rolname = 'ghost_grantee';
+DELETE FROM pg_authid WHERE rolname = 'ghost_grantor';
+
+
+});
+
+my $tempdir = PostgreSQL::Test::Utils::tempdir;
+my $dump_file = "$tempdir/dangling.sql";
+
+# pg_dump must succeed even with dangling pg_init_privs entries.
+command_ok(
+ [
+ 'pg_dump',
+ '--port' => $node->port,
+ '--schema-only',
+ '-f' => $dump_file,
+ 'regress_dangling',
+ ],
+ 'pg_dump succeeds with dangling pg_init_privs entries');
+
+my $dump = slurp_file($dump_file);
+
+# --- Case 1: dangling grantee ---
+like($dump, qr/CREATE FUNCTION public\.test_func_grantee/,
+ 'case 1: function is present in dump');
+unlike($dump, qr/GRANT\b.*\btest_func_grantee/,
+ 'case 1: no GRANT for function with dangling grantee');
+
+# --- Case 2: dangling grantor ---
+like($dump, qr/CREATE FUNCTION public\.test_func_grantor/,
+ 'case 2: function is present in dump');
+unlike($dump, qr/GRANT\b.*\btest_func_grantor/,
+ 'case 2: no GRANT for function with dangling grantor');
+
+# --- Case 3: column-level dangling ---
+like($dump, qr/CREATE TABLE public\.test_tbl/,
+ 'case 3: table is present in dump');
+unlike($dump, qr/GRANT\b.*\btest_tbl\b.*secret/i,
+ 'case 3: no column GRANT for dangling column-level entry');
+
+# --- Case 4: spurious REVOKE ---
+like($dump, qr/CREATE FUNCTION public\.test_func_revoke/,
+ 'case 4: function is present in dump');
+unlike($dump, qr/REVOKE\b.*\btest_func_revoke/,
+ 'case 4: no spurious REVOKE for function with all-dangling initprivs');
+
+# --- Case 5: valid all-digit role "007" ---
+like($dump, qr/CREATE FUNCTION public\.test_func_007/,
+ 'case 5: function is present in dump');
+like($dump, qr/GRANT\b.*\btest_func_007\b.*TO\s+"007"/,
+ 'case 5: GRANT to valid all-digit role "007" is preserved');
+
+# --- General: no numeric OID as role name (other than the valid "007") ---
+# Match any GRANT ... TO "digits" where the digits are NOT "007".
+unlike($dump, qr/GRANT\b.*\bTO\s+"(?!007")[0-9]+"/,
+ 'no GRANT with bare numeric OID as role name (other than valid "007")');
+
+done_testing();
--
2.50.1 (Apple Git-155)
view thread (28+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19483: pg_upgrade fails with orphan records in pg_init_priv catalog table
In-Reply-To: <CAB5wL7aTaOyijT3qJF9jAQNrv1K74sQkEyu+zkb54rYDc_NBdg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox