Re: Can we get rid of repeated queries from pg_dump?

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: Can we get rid of repeated queries from pg_dump?
Дата
Msg-id 1082810.1630189581@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: Can we get rid of repeated queries from pg_dump?  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: Can we get rid of repeated queries from pg_dump?  (Gus Spier <gus.spier@gmail.com>)
Re: Can we get rid of repeated queries from pg_dump?  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Список pgsql-general
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

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 = 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 = 0; i < numTables; i++)
     {
         TableInfo  *tbinfo = &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[], int numTables)
             polinfo->polqual = NULL;
             polinfo->polwithcheck = 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 >= 100000)
-            appendPQExpBuffer(query,
-                              "SELECT oid, tableoid, pol.polname, pol.polcmd, pol.polpermissive, "
-                              "CASE WHEN pol.polroles = '{0}' THEN NULL ELSE "
-                              "   pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) from
pg_catalog.pg_rolesWHERE oid = 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 polwithcheck "
-                              "FROM pg_catalog.pg_policy pol "
-                              "WHERE polrelid = '%u'",
-                              tbinfo->dobj.catId.oid);
-        else
-            appendPQExpBuffer(query,
-                              "SELECT oid, tableoid, pol.polname, pol.polcmd, 't' as polpermissive, "
-                              "CASE WHEN pol.polroles = '{0}' THEN NULL ELSE "
-                              "   pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) from
pg_catalog.pg_rolesWHERE oid = 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 polwithcheck "
-                              "FROM pg_catalog.pg_policy pol "
-                              "WHERE polrelid = '%u'",
-                              tbinfo->dobj.catId.oid);
-        res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+    /*
+     * Now, read all RLS policies, and create PolicyInfo objects for all those
+     * that are of interest.
+     */
+    pg_log_info("reading row-level security policies");

-        ntups = PQntuples(res);
+    printfPQExpBuffer(query,
+                      "SELECT oid, tableoid, pol.polrelid, pol.polname, pol.polcmd, ");
+    if (fout->remoteVersion >= 100000)
+        appendPQExpBuffer(query, "pol.polpermissive, ");
+    else
+        appendPQExpBuffer(query, "'t' as polpermissive, ");
+    appendPQExpBuffer(query,
+                      "CASE WHEN pol.polroles = '{0}' THEN NULL ELSE "
+                      "   pg_catalog.array_to_string(ARRAY(SELECT pg_catalog.quote_ident(rolname) from
pg_catalog.pg_rolesWHERE oid = 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 polwithcheck "
+                      "FROM pg_catalog.pg_policy pol");

-        if (ntups == 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 = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);

+    ntups = PQntuples(res);
+    if (ntups > 0)
+    {
         i_oid = PQfnumber(res, "oid");
         i_tableoid = PQfnumber(res, "tableoid");
+        i_polrelid = PQfnumber(res, "polrelid");
         i_polname = PQfnumber(res, "polname");
         i_polcmd = PQfnumber(res, "polcmd");
         i_polpermissive = PQfnumber(res, "polpermissive");
@@ -3775,6 +3755,16 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables)

         for (j = 0; j < ntups; j++)
         {
+            Oid            polrelid = atooid(PQgetvalue(res, j, i_polrelid));
+            TableInfo  *tbinfo = 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 = DO_POLICY;
             polinfo[j].dobj.catId.tableoid =
                 atooid(PQgetvalue(res, j, i_tableoid));
@@ -3804,8 +3794,10 @@ getPolicies(Archive *fout, TableInfo tblinfo[], int numTables)
                 polinfo[j].polwithcheck
                     = pg_strdup(PQgetvalue(res, j, i_polwithcheck));
         }
-        PQclear(res);
     }
+
+    PQclear(res);
+
     destroyPQExpBuffer(query);
 }


В списке pgsql-general по дате отправления:

Предыдущее
От: "David G. Johnston"
Дата:
Сообщение: Re: use fopen iso-8859-1 resource
Следующее
От: Adrian Klaver
Дата:
Сообщение: Re: use fopen iso-8859-1 resource