From 01b5e9a1deba60246d816e31e4042ff46a593306 Mon Sep 17 00:00:00 2001 From: "dgrowley@gmail.com" Date: Mon, 25 Jun 2018 16:31:47 +1200 Subject: [PATCH v3] Improve performance of tuple conversion map generation 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. --- src/backend/access/common/tupconvert.c | 37 ++++++++++++++++++++++++++-------- src/backend/optimizer/prep/prepunion.c | 20 +++++++++--------- 2 files changed, 40 insertions(+), 17 deletions(-) diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c index 2d0d2f4b32..11038c69cb 100644 --- a/src/backend/access/common/tupconvert.c +++ b/src/backend/access/common/tupconvert.c @@ -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,28 @@ 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 +352,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 +365,6 @@ convert_tuples_by_name_map(TupleDesc indesc, format_type_be(outdesc->tdtypeid), format_type_be(indesc->tdtypeid)))); } - return attrMap; } diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 7d75e1eda9..95247b4a1c 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -51,6 +51,7 @@ #include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/selfuncs.h" +#include "utils/syscache.h" typedef struct @@ -1895,6 +1896,7 @@ 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; @@ -1940,7 +1942,7 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation, * 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. + * that before looking for it in syscache. * * Note: the test for (att = ...) != NULL cannot fail, it's just a * notational device to include the assignment into the if-clause. @@ -1952,16 +1954,16 @@ make_inh_translation_list(Relation oldrelation, Relation newrelation, new_attno = old_attno; else { - 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 = 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 */ -- 2.17.1