Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mK6mX-0006yw-Bo for pgsql-general@arkaria.postgresql.org; Sat, 28 Aug 2021 22:26:33 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1mK6mU-000688-Bj for pgsql-general@arkaria.postgresql.org; Sat, 28 Aug 2021 22:26:30 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mK6mT-000680-D6 for pgsql-general@lists.postgresql.org; Sat, 28 Aug 2021 22:26:29 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mK6mQ-00078V-T9 for pgsql-general@lists.postgresql.org; Sat, 28 Aug 2021 22:26:28 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 17SMQLdX1082811; Sat, 28 Aug 2021 18:26:22 -0400 From: Tom Lane To: depesz@depesz.com cc: Stephen Frost , Laurenz Albe , pgsql-general Subject: Re: Can we get rid of repeated queries from pg_dump? In-reply-to: <1004390.1630160930@sss.pgh.pa.us> References: <20210826084430.GA26282@depesz.com> <648517.1629986527@sss.pgh.pa.us> <20210826140827.GA15868@depesz.com> <649446.1629987629@sss.pgh.pa.us> <20210826160644.GA3120@depesz.com> <896732.1630099403@sss.pgh.pa.us> <20210828064650.GB11579@depesz.com> <1004390.1630160930@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Sat, 28 Aug 2021 10:28:50 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <1082743.1630189493.0@sss.pgh.pa.us> Date: Sat, 28 Aug 2021 18:26:21 -0400 Message-ID: <1082810.1630189581@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <1082743.1630189493.1@sss.pgh.pa.us> Here is a second patch, quite independent of the first one, that gets rid of some other repetitive queries. On the regression database, the number of queries needed to do "pg_dump -s regression" drops from 3260 to 2589, and on my machine it takes 1.8 sec instead of 2.1 sec. What's attacked here is a fairly silly decision in getPolicies() to query pg_policy once per table, when we could do so just once. It might have been okay if we skipped the per-table query for tables that lack policies, but it's not clear to me that we can know that without looking into pg_policy. In any case I doubt this is ever going to be less efficient than the original coding. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="query-pg_policy-just-once.patch"; charset="us-ascii" Content-ID: <1082743.1630189493.2@sss.pgh.pa.us> Content-Description: query-pg_policy-just-once.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 6adbd20778..befe68de1a 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -3656,7 +3656,7 @@ dumpBlobs(Archive *fout, const void *arg) = /* * getPolicies - * get information about policies on a dumpable table. + * get information about all RLS policies on dumpable tables. */ void getPolicies(Archive *fout, TableInfo tblinfo[], int numTables) @@ -3666,6 +3666,7 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int = numTables) PolicyInfo *polinfo; int i_oid; int i_tableoid; + int i_polrelid; int i_polname; int i_polcmd; int i_polpermissive; @@ -3681,6 +3682,10 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int= numTables) = query =3D createPQExpBuffer(); = + /* + * First, check which tables have RLS enabled. We represent RLS being + * enabled on a table by creating a PolicyInfo object with null polname. + */ for (i =3D 0; i < numTables; i++) { TableInfo *tbinfo =3D &tblinfo[i]; @@ -3689,15 +3694,6 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int= numTables) if (!(tbinfo->dobj.dump & DUMP_COMPONENT_POLICY)) continue; = - pg_log_info("reading row security enabled for table \"%s.%s\"", - tbinfo->dobj.namespace->dobj.name, - tbinfo->dobj.name); - - /* - * Get row security enabled information for the table. We represent - * RLS being enabled on a table by creating a PolicyInfo object with - * null polname. - */ if (tbinfo->rowsec) { /* @@ -3719,51 +3715,35 @@ getPolicies(Archive *fout, TableInfo tblinfo[], in= t numTables) polinfo->polqual =3D NULL; polinfo->polwithcheck =3D NULL; } + } = - pg_log_info("reading policies for table \"%s.%s\"", - tbinfo->dobj.namespace->dobj.name, - tbinfo->dobj.name); - - resetPQExpBuffer(query); - - /* Get the policies for the table. */ - if (fout->remoteVersion >=3D 100000) - appendPQExpBuffer(query, - "SELECT oid, tableoid, pol.polname, pol.polcmd, pol.polpermissiv= e, " - "CASE WHEN pol.polroles =3D '{0}' THEN NULL ELSE " - " pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ide= nt(rolname) from pg_catalog.pg_roles WHERE oid =3D ANY(pol.polroles)), ', = ') END AS polroles, " - "pg_catalog.pg_get_expr(pol.polqual, pol.polrelid) AS polqual, " - "pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid) AS polwi= thcheck " - "FROM pg_catalog.pg_policy pol " - "WHERE polrelid =3D '%u'", - tbinfo->dobj.catId.oid); - else - appendPQExpBuffer(query, - "SELECT oid, tableoid, pol.polname, pol.polcmd, 't' as polpermis= sive, " - "CASE WHEN pol.polroles =3D '{0}' THEN NULL ELSE " - " pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ide= nt(rolname) from pg_catalog.pg_roles WHERE oid =3D ANY(pol.polroles)), ', = ') END AS polroles, " - "pg_catalog.pg_get_expr(pol.polqual, pol.polrelid) AS polqual, " - "pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid) AS polwi= thcheck " - "FROM pg_catalog.pg_policy pol " - "WHERE polrelid =3D '%u'", - tbinfo->dobj.catId.oid); - res =3D ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); + /* + * Now, read all RLS policies, and create PolicyInfo objects for all tho= se + * that are of interest. + */ + pg_log_info("reading row-level security policies"); = - ntups =3D PQntuples(res); + printfPQExpBuffer(query, + "SELECT oid, tableoid, pol.polrelid, pol.polname, pol.polcmd, "); + if (fout->remoteVersion >=3D 100000) + appendPQExpBuffer(query, "pol.polpermissive, "); + else + appendPQExpBuffer(query, "'t' as polpermissive, "); + appendPQExpBuffer(query, + "CASE WHEN pol.polroles =3D '{0}' THEN NULL ELSE " + " pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident= (rolname) from pg_catalog.pg_roles WHERE oid =3D ANY(pol.polroles)), ', ')= END AS polroles, " + "pg_catalog.pg_get_expr(pol.polqual, pol.polrelid) AS polqual, " + "pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid) AS polwith= check " + "FROM pg_catalog.pg_policy pol"); = - if (ntups =3D=3D 0) - { - /* - * No explicit policies to handle (only the default-deny policy, - * which is handled as part of the table definition). Clean up - * and return. - */ - PQclear(res); - continue; - } + res =3D ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); = + ntups =3D PQntuples(res); + if (ntups > 0) + { i_oid =3D PQfnumber(res, "oid"); i_tableoid =3D PQfnumber(res, "tableoid"); + i_polrelid =3D PQfnumber(res, "polrelid"); i_polname =3D PQfnumber(res, "polname"); i_polcmd =3D PQfnumber(res, "polcmd"); i_polpermissive =3D PQfnumber(res, "polpermissive"); @@ -3775,6 +3755,16 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int= numTables) = for (j =3D 0; j < ntups; j++) { + Oid polrelid =3D atooid(PQgetvalue(res, j, i_polrelid)); + TableInfo *tbinfo =3D findTableByOid(polrelid); + + /* + * Ignore row security on tables not to be dumped. (This will + * result in some harmless wasted slots in polinfo[].) + */ + if (!(tbinfo->dobj.dump & DUMP_COMPONENT_POLICY)) + continue; + polinfo[j].dobj.objType =3D DO_POLICY; polinfo[j].dobj.catId.tableoid =3D atooid(PQgetvalue(res, j, i_tableoid)); @@ -3804,8 +3794,10 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int= numTables) polinfo[j].polwithcheck =3D pg_strdup(PQgetvalue(res, j, i_polwithcheck)); } - PQclear(res); } + + PQclear(res); + destroyPQExpBuffer(query); } = ------- =_aaaaaaaaaa0--