Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Improve performance of tuple conversion map generation
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Fri, 13 Jul 2018 16:54:05 +0000 (19:54 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Fri, 13 Jul 2018 16:54:05 +0000 (19:54 +0300)
Previously convert_tuples_by_name_map naively performed a search of each
outdesc column starting at the first column in indesc and searched each
indesc column until a match was found.  When partitioned tables had many
columns this could result in slow generation of the tuple conversion maps.
For INSERT and UPDATE statements that touched few rows, this could mean a
very large overhead indeed.

We can do a bit better with this loop.  It's quite likely that the columns
in partitioned tables and their partitions are in the same order, so it
makes sense to start searching for each column outer column at the inner
column position 1 after where the previous match was found (per idea from
Alexander Kuzmenkov). This makes the best case search O(N) instead of
O(N^2).  The worst case is still O(N^2), but it seems unlikely that would
happen.

Likewise, in the planner, make_inh_translation_list's search for the
matching column could often end up falling back on an O(N^2) type search.
This commit also improves that by first checking the column that follows
the previous match, instead of the column with the same attnum.  If we
fail to match here we fallback on the syscache's hashtable lookup.

Author: David Rowley
Reviewed-by: Alexander Kuzmenkov
Discussion: https://www.postgresql.org/message-id/CAKJS1f9-wijVgMdRp6_qDMEQDJJ%2BA_n%3DxzZuTmLx5Fz6cwf%2B8A%40mail.gmail.com

src/backend/access/common/tupconvert.c
src/backend/optimizer/prep/prepunion.c

index 2d0d2f4b32f8e969a06b45b1ab54e960e268fd0d..3bc67b846d43e1f4035ea6fff07379d660e27800 100644 (file)
@@ -295,12 +295,16 @@ convert_tuples_by_name_map(TupleDesc indesc,
                           const char *msg)
 {
    AttrNumber *attrMap;
-   int         n;
+   int         outnatts;
+   int         innatts;
    int         i;
+   int         nextindesc = -1;
 
-   n = outdesc->natts;
-   attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
-   for (i = 0; i < n; i++)
+   outnatts = outdesc->natts;
+   innatts = indesc->natts;
+
+   attrMap = (AttrNumber *) palloc0(outnatts * sizeof(AttrNumber));
+   for (i = 0; i < outnatts; i++)
    {
        Form_pg_attribute outatt = TupleDescAttr(outdesc, i);
        char       *attname;
@@ -313,10 +317,27 @@ convert_tuples_by_name_map(TupleDesc indesc,
        attname = NameStr(outatt->attname);
        atttypid = outatt->atttypid;
        atttypmod = outatt->atttypmod;
-       for (j = 0; j < indesc->natts; j++)
+
+       /*
+        * Now search for an attribute with the same name in the indesc. It
+        * seems likely that a partitioned table will have the attributes in
+        * the same order as the partition, so the search below is optimized
+        * for that case.  It is possible that columns are dropped in one of
+        * the relations, but not the other, so we use the 'nextindesc'
+        * counter to track the starting point of the search.  If the inner
+        * loop encounters dropped columns then it will have to skip over
+        * them, but it should leave 'nextindesc' at the correct position for
+        * the next outer loop.
+        */
+       for (j = 0; j < innatts; j++)
        {
-           Form_pg_attribute inatt = TupleDescAttr(indesc, j);
+           Form_pg_attribute inatt;
 
+           nextindesc++;
+           if (nextindesc >= innatts)
+               nextindesc = 0;
+
+           inatt = TupleDescAttr(indesc, nextindesc);
            if (inatt->attisdropped)
                continue;
            if (strcmp(attname, NameStr(inatt->attname)) == 0)
@@ -330,7 +351,7 @@ convert_tuples_by_name_map(TupleDesc indesc,
                                       attname,
                                       format_type_be(outdesc->tdtypeid),
                                       format_type_be(indesc->tdtypeid))));
-               attrMap[i] = (AttrNumber) (j + 1);
+               attrMap[i] = inatt->attnum;
                break;
            }
        }
@@ -343,7 +364,6 @@ convert_tuples_by_name_map(TupleDesc indesc,
                               format_type_be(outdesc->tdtypeid),
                               format_type_be(indesc->tdtypeid))));
    }
-
    return attrMap;
 }
 
index 7d75e1eda935c572a1d2d7e06a7a70d5f10c42b7..690b6bbab7fcf902a526a43759163af5681f4c3c 100644 (file)
@@ -51,6 +51,7 @@
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
 #include "utils/selfuncs.h"
+#include "utils/syscache.h"
 
 
 typedef struct
@@ -1895,9 +1896,11 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation,
    List       *vars = NIL;
    TupleDesc   old_tupdesc = RelationGetDescr(oldrelation);
    TupleDesc   new_tupdesc = RelationGetDescr(newrelation);
+   Oid         new_relid = RelationGetRelid(newrelation);
    int         oldnatts = old_tupdesc->natts;
    int         newnatts = new_tupdesc->natts;
    int         old_attno;
+   int         new_attno = 0;
 
    for (old_attno = 0; old_attno < oldnatts; old_attno++)
    {
@@ -1906,7 +1909,6 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation,
        Oid         atttypid;
        int32       atttypmod;
        Oid         attcollation;
-       int         new_attno;
 
        att = TupleDescAttr(old_tupdesc, old_attno);
        if (att->attisdropped)
@@ -1939,29 +1941,25 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation,
         * Otherwise we have to search for the matching column by name.
         * There's no guarantee it'll have the same column position, because
         * of cases like ALTER TABLE ADD COLUMN and multiple inheritance.
-        * However, in simple cases it will be the same column number, so try
-        * that before we go groveling through all the columns.
-        *
-        * Note: the test for (att = ...) != NULL cannot fail, it's just a
-        * notational device to include the assignment into the if-clause.
+        * However, in simple cases, the relative order of columns is mostly
+        * the same in both relations, so try the column of newrelation that
+        * follows immediately after the one that we just found, and if that
+        * fails, let syscache handle it.
         */
-       if (old_attno < newnatts &&
-           (att = TupleDescAttr(new_tupdesc, old_attno)) != NULL &&
-           !att->attisdropped &&
-           strcmp(attname, NameStr(att->attname)) == 0)
-           new_attno = old_attno;
-       else
+       if (new_attno >= newnatts ||
+           (att = TupleDescAttr(new_tupdesc, new_attno))->attisdropped ||
+           strcmp(attname, NameStr(att->attname)) != 0)
        {
-           for (new_attno = 0; new_attno < newnatts; new_attno++)
-           {
-               att = TupleDescAttr(new_tupdesc, new_attno);
-               if (!att->attisdropped &&
-                   strcmp(attname, NameStr(att->attname)) == 0)
-                   break;
-           }
-           if (new_attno >= newnatts)
+           HeapTuple   newtup;
+
+           newtup = SearchSysCacheAttName(new_relid, attname);
+           if (!newtup)
                elog(ERROR, "could not find inherited attribute \"%s\" of relation \"%s\"",
                     attname, RelationGetRelationName(newrelation));
+           new_attno = ((Form_pg_attribute) GETSTRUCT(newtup))->attnum - 1;
+           ReleaseSysCache(newtup);
+
+           att = TupleDescAttr(new_tupdesc, new_attno);
        }
 
        /* Found it, check type and collation match */
@@ -1978,6 +1976,7 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation,
                                     atttypmod,
                                     attcollation,
                                     0));
+       new_attno++;
    }
 
    *translated_vars = vars;