From d5cc2db9bd610523915d1512c2fcad84e8bae3b6 Mon Sep 17 00:00:00 2001 From: amit Date: Tue, 17 Apr 2018 15:51:42 +0900 Subject: [PATCH v2] Optimize convert_tuples_by_name_map usage a bit --- src/backend/access/common/tupconvert.c | 79 ++++++++------------- src/backend/catalog/index.c | 31 +++++---- src/backend/catalog/partition.c | 13 ++-- src/backend/catalog/pg_constraint.c | 5 +- src/backend/commands/indexcmds.c | 14 ++-- src/backend/commands/tablecmds.c | 32 +++++---- src/backend/executor/execPartition.c | 122 ++++++++++++++------------------- src/backend/parser/parse_utilcmd.c | 9 +-- 8 files changed, 143 insertions(+), 162 deletions(-) diff --git a/src/backend/access/common/tupconvert.c b/src/backend/access/common/tupconvert.c index 2d0d2f4b32..2ee4a7f40f 100644 --- a/src/backend/access/common/tupconvert.c +++ b/src/backend/access/common/tupconvert.c @@ -214,57 +214,20 @@ convert_tuples_by_name(TupleDesc indesc, TupleConversionMap *map; AttrNumber *attrMap; int n = outdesc->natts; - int i; - bool same; /* Verify compatibility and prepare attribute-number map */ attrMap = convert_tuples_by_name_map(indesc, outdesc, msg); /* - * Check to see if the map is one-to-one, in which case we need not do a - * tuple conversion. We must also insist that both tupdescs either - * specify or don't specify an OID column, else we need a conversion to - * add/remove space for that. (For some callers, presence or absence of - * an OID column perhaps would not really matter, but let's be safe.) + * If attributes are at the same positions in the input and output + * descriptors, there is no need for tuple conversion. Also, we must also + * insist that both tupdescs either specify or don't specify an OID column, + * else we need a conversion to add/remove space for that. (For some + * callers, presence or absence of an OID column perhaps would not really + * matter, but let's be safe.) */ - if (indesc->natts == outdesc->natts && - indesc->tdhasoid == outdesc->tdhasoid) - { - same = true; - for (i = 0; i < n; i++) - { - Form_pg_attribute inatt; - Form_pg_attribute outatt; - - if (attrMap[i] == (i + 1)) - continue; - - /* - * If it's a dropped column and the corresponding input column is - * also dropped, we needn't convert. However, attlen and attalign - * must agree. - */ - inatt = TupleDescAttr(indesc, i); - outatt = TupleDescAttr(outdesc, i); - if (attrMap[i] == 0 && - inatt->attisdropped && - inatt->attlen == outatt->attlen && - inatt->attalign == outatt->attalign) - continue; - - same = false; - break; - } - } - else - same = false; - - if (same) - { - /* Runtime conversion is not needed */ - pfree(attrMap); + if (attrMap == NULL && indesc->tdhasoid == outdesc->tdhasoid) return NULL; - } /* Prepare the map structure */ map = (TupleConversionMap *) palloc(sizeof(TupleConversionMap)); @@ -285,9 +248,10 @@ convert_tuples_by_name(TupleDesc indesc, /* * Return a palloc'd bare attribute map for tuple conversion, matching input - * and output columns by name. (Dropped columns are ignored in both input and - * output.) This is normally a subroutine for convert_tuples_by_name, but can - * be used standalone. + * and output columns by name or NULL if the attributes appear at the same + * positions in input and output (Dropped columns are ignored in both input + * and output.) This is normally a subroutine for convert_tuples_by_name, but + * can be used standalone. */ AttrNumber * convert_tuples_by_name_map(TupleDesc indesc, @@ -297,12 +261,13 @@ convert_tuples_by_name_map(TupleDesc indesc, AttrNumber *attrMap; int n; int i; + bool all_attrpos_same = true; n = outdesc->natts; attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber)); for (i = 0; i < n; i++) { - Form_pg_attribute outatt = TupleDescAttr(outdesc, i); + Form_pg_attribute outatt= TupleDescAttr(outdesc, i); char *attname; Oid atttypid; int32 atttypmod; @@ -331,6 +296,8 @@ convert_tuples_by_name_map(TupleDesc indesc, format_type_be(outdesc->tdtypeid), format_type_be(indesc->tdtypeid)))); attrMap[i] = (AttrNumber) (j + 1); + if (i != j) + all_attrpos_same = false; break; } } @@ -344,6 +311,17 @@ convert_tuples_by_name_map(TupleDesc indesc, format_type_be(indesc->tdtypeid)))); } + /* + * No need of mapping if both descriptors have the same number of + * attributes and individual attributes all at the same positions in both + * descriptors. + */ + if (all_attrpos_same && indesc->natts == outdesc->natts) + { + pfree(attrMap); + return NULL; + } + return attrMap; } @@ -373,7 +351,7 @@ do_convert_tuple(HeapTuple tuple, TupleConversionMap *map) */ for (i = 0; i < outnatts; i++) { - int j = attrMap[i]; + int j = attrMap ? attrMap[i] : i; outvalues[i] = invalues[j]; outisnull[i] = inisnull[j]; @@ -392,7 +370,8 @@ void free_conversion_map(TupleConversionMap *map) { /* indesc and outdesc are not ours to free */ - pfree(map->attrMap); + if (map->attrMap) + pfree(map->attrMap); pfree(map->invalues); pfree(map->inisnull); pfree(map->outvalues); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index dec4265d68..ed0c2f6c9b 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1848,13 +1848,18 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2, */ for (i = 0; i < info1->ii_NumIndexAttrs; i++) { - if (maplen < info2->ii_IndexAttrNumbers[i]) + AttrNumber attnum1 = info1->ii_IndexAttrNumbers[i]; + AttrNumber attnum2; + + if (attmap != NULL && maplen < info2->ii_IndexAttrNumbers[i]) elog(ERROR, "incorrect attribute map"); + attnum2 = attmap != NULL + ? attmap[info2->ii_IndexAttrNumbers[i] - 1] + : info2->ii_IndexAttrNumbers[i]; + /* ignore expressions at this stage */ - if ((info1->ii_IndexAttrNumbers[i] != InvalidAttrNumber) && - (attmap[info2->ii_IndexAttrNumbers[i] - 1] != - info1->ii_IndexAttrNumbers[i])) + if (attnum1 != InvalidAttrNumber && attnum2 != attnum1) return false; /* collation and opfamily is not valid for including columns */ @@ -1876,11 +1881,12 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2, if (info1->ii_Expressions != NIL) { bool found_whole_row; - Node *mapped; + Node *mapped = (Node *) info2->ii_Expressions; - mapped = map_variable_attnos((Node *) info2->ii_Expressions, - 1, 0, attmap, maplen, - InvalidOid, &found_whole_row); + if (attmap != NULL) + mapped = map_variable_attnos((Node *) info2->ii_Expressions, + 1, 0, attmap, maplen, + InvalidOid, &found_whole_row); if (found_whole_row) { /* @@ -1900,11 +1906,12 @@ CompareIndexInfo(IndexInfo *info1, IndexInfo *info2, if (info1->ii_Predicate != NULL) { bool found_whole_row; - Node *mapped; + Node *mapped = (Node *) info2->ii_Predicate; - mapped = map_variable_attnos((Node *) info2->ii_Predicate, - 1, 0, attmap, maplen, - InvalidOid, &found_whole_row); + if (attmap != NULL) + mapped = map_variable_attnos((Node *) info2->ii_Predicate, + 1, 0, attmap, maplen, + InvalidOid, &found_whole_row); if (found_whole_row) { /* diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index de801ad788..0db7d0d8e3 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -178,12 +178,13 @@ map_partition_varattnos(List *expr, int fromrel_varno, part_attnos = convert_tuples_by_name_map(RelationGetDescr(to_rel), RelationGetDescr(from_rel), gettext_noop("could not convert row type")); - expr = (List *) map_variable_attnos((Node *) expr, - fromrel_varno, 0, - part_attnos, - RelationGetDescr(from_rel)->natts, - RelationGetForm(to_rel)->reltype, - &my_found_whole_row); + if (part_attnos != NULL) + expr = (List *) map_variable_attnos((Node *) expr, + fromrel_varno, 0, + part_attnos, + RelationGetDescr(from_rel)->natts, + RelationGetForm(to_rel)->reltype, + &my_found_whole_row); } if (found_whole_row) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index c5b5395791..46fe53cc8b 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -488,7 +488,7 @@ CloneForeignKeyConstraints(Oid parentId, Oid relationId, List **cloned) memcpy(conkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber)); for (i = 0; i < nelem; i++) - mapped_conkey[i] = attmap[conkey[i] - 1]; + mapped_conkey[i] = attmap ? attmap[conkey[i] - 1] : conkey[i]; datum = fastgetattr(tuple, Anum_pg_constraint_confkey, tupdesc, &isnull); @@ -621,7 +621,8 @@ CloneForeignKeyConstraints(Oid parentId, Oid relationId, List **cloned) } systable_endscan(scan); - pfree(attmap); + if (attmap) + pfree(attmap); if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE) { diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index f2dcc1c51f..797bb54d63 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -918,7 +918,7 @@ DefineIndex(Oid relationId, convert_tuples_by_name_map(RelationGetDescr(childrel), parentDesc, gettext_noop("could not convert row type")); - maplen = parentDesc->natts; + maplen = attmap != NULL ? parentDesc->natts : 0; foreach(cell, childidxs) @@ -992,10 +992,11 @@ DefineIndex(Oid relationId, IndexStmt *childStmt = copyObject(stmt); bool found_whole_row; - childStmt->whereClause = - map_variable_attnos(stmt->whereClause, 1, 0, - attmap, maplen, - InvalidOid, &found_whole_row); + if (attmap != NULL) + childStmt->whereClause = + map_variable_attnos(stmt->whereClause, 1, 0, + attmap, maplen, + InvalidOid, &found_whole_row); if (found_whole_row) elog(ERROR, "cannot convert whole-row table reference"); @@ -1009,7 +1010,8 @@ DefineIndex(Oid relationId, false, quiet); } - pfree(attmap); + if (attmap) + pfree(attmap); } /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index c1a9bda433..6f37dd153a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -9220,6 +9220,7 @@ ATPrepAlterColumnType(List **wqueue, { Oid childrelid = lfirst_oid(child); Relation childrel; + AlterTableCmd *child_cmd = cmd; if (childrelid == relid) continue; @@ -9238,24 +9239,26 @@ ATPrepAlterColumnType(List **wqueue, bool found_whole_row; /* create a copy to scribble on */ - cmd = copyObject(cmd); + child_cmd = copyObject(cmd); attmap = convert_tuples_by_name_map(RelationGetDescr(childrel), RelationGetDescr(rel), gettext_noop("could not convert row type")); - ((ColumnDef *) cmd->def)->cooked_default = - map_variable_attnos(def->cooked_default, - 1, 0, - attmap, RelationGetDescr(rel)->natts, - InvalidOid, &found_whole_row); + if (attmap != NULL) + ((ColumnDef *) child_cmd->def)->cooked_default = + map_variable_attnos(def->cooked_default, + 1, 0, + attmap, RelationGetDescr(rel)->natts, + InvalidOid, &found_whole_row); if (found_whole_row) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot convert whole-row table reference"), errdetail("USING expression contains a whole-row table reference."))); - pfree(attmap); + if (attmap) + pfree(attmap); } - ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode); + ATPrepCmd(wqueue, childrel, child_cmd, false, true, lockmode); relation_close(childrel, NoLock); } } @@ -14381,6 +14384,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) Relation idxRel = index_open(idx, AccessShareLock); IndexInfo *info; AttrNumber *attmap; + int maplen; bool found = false; Oid constraintOid; @@ -14399,6 +14403,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) attmap = convert_tuples_by_name_map(RelationGetDescr(attachrel), RelationGetDescr(rel), gettext_noop("could not convert row type")); + maplen = attmap != NULL ? RelationGetDescr(rel)->natts : 0; constraintOid = get_relation_idx_constraint_oid(RelationGetRelid(rel), idx); /* @@ -14420,8 +14425,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel) idxRel->rd_indcollation, attachrelIdxRels[i]->rd_opfamily, idxRel->rd_opfamily, - attmap, - RelationGetDescr(rel)->natts)) + attmap, maplen)) { /* * If this index is being created in the parent because of a @@ -14828,6 +14832,7 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) IndexInfo *childInfo; IndexInfo *parentInfo; AttrNumber *attmap; + int maplen; bool found; int i; PartitionDesc partDesc; @@ -14874,13 +14879,13 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) attmap = convert_tuples_by_name_map(RelationGetDescr(partTbl), RelationGetDescr(parentTbl), gettext_noop("could not convert row type")); + maplen = attmap != NULL ? RelationGetDescr(partTbl)->natts : 0; if (!CompareIndexInfo(childInfo, parentInfo, partIdx->rd_indcollation, parentIdx->rd_indcollation, partIdx->rd_opfamily, parentIdx->rd_opfamily, - attmap, - RelationGetDescr(partTbl)->natts)) + attmap, maplen)) ereport(ERROR, (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), errmsg("cannot attach index \"%s\" as a partition of index \"%s\"", @@ -14917,7 +14922,8 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name) ConstraintSetParentConstraint(cldConstrId, constraintOid); update_relispartition(NULL, partIdxId, true); - pfree(attmap); + if (attmap) + pfree(attmap); validatePartitionedIndex(parentIdx, parentTbl); } diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 23a74bc3d9..f600af3f0e 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -311,9 +311,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, Relation firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc; ResultRelInfo *leaf_part_rri; MemoryContext oldContext; - AttrNumber *part_attnos = NULL; + AttrNumber *attmap; bool found_whole_row; - bool equalTupdescs; /* * We locked all the partitions in ExecSetupPartitionTupleRouting @@ -361,9 +360,13 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, (node != NULL && node->onConflictAction != ONCONFLICT_NONE)); - /* if tuple descs are identical, we don't need to map the attrs */ - equalTupdescs = equalTupleDescs(RelationGetDescr(partrel), - RelationGetDescr(firstResultRel)); + /* + * Get a attribute conversion map to convert expressions, which if NULL, + * the original expressions can be used as is. + */ + attmap = convert_tuples_by_name_map(RelationGetDescr(partrel), + RelationGetDescr(firstResultRel), + gettext_noop("could not convert row type")); /* * Build WITH CHECK OPTION constraints for the partition. Note that we @@ -405,16 +408,12 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, /* * Convert Vars in it to contain this partition's attribute numbers. */ - if (!equalTupdescs) + if (attmap != NULL) { - part_attnos = - convert_tuples_by_name_map(RelationGetDescr(partrel), - RelationGetDescr(firstResultRel), - gettext_noop("could not convert row type")); wcoList = (List *) map_variable_attnos((Node *) wcoList, firstVarno, 0, - part_attnos, + attmap, RelationGetDescr(firstResultRel)->natts, RelationGetForm(partrel)->reltype, &found_whole_row); @@ -464,25 +463,18 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, */ returningList = linitial(node->returningLists); - if (!equalTupdescs) - { - /* - * Convert Vars in it to contain this partition's attribute numbers. - */ - if (part_attnos == NULL) - part_attnos = - convert_tuples_by_name_map(RelationGetDescr(partrel), - RelationGetDescr(firstResultRel), - gettext_noop("could not convert row type")); + /* + * Convert Vars in it to contain this partition's attribute numbers. + */ + if (attmap != NULL) returningList = (List *) map_variable_attnos((Node *) returningList, firstVarno, 0, - part_attnos, + attmap, RelationGetDescr(firstResultRel)->natts, RelationGetForm(partrel)->reltype, &found_whole_row); - /* We ignore the value of found_whole_row. */ - } + /* We ignore the value of found_whole_row. */ leaf_part_rri->ri_returningList = returningList; @@ -565,7 +557,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * CONFLICT SET state, skipping a bunch of work. Otherwise, we * need to create state specific to this partition. */ - if (map == NULL) + if (attmap == NULL) leaf_part_rri->ri_onConflict = resultRelInfo->ri_onConflict; else { @@ -583,33 +575,26 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, * target relation (firstVarno). */ onconflset = (List *) copyObject((Node *) node->onConflictSet); - if (!equalTupdescs) - { - if (part_attnos == NULL) - part_attnos = - convert_tuples_by_name_map(RelationGetDescr(partrel), - RelationGetDescr(firstResultRel), - gettext_noop("could not convert row type")); - onconflset = (List *) - map_variable_attnos((Node *) onconflset, - INNER_VAR, 0, - part_attnos, - RelationGetDescr(firstResultRel)->natts, - RelationGetForm(partrel)->reltype, - &found_whole_row); - /* We ignore the value of found_whole_row. */ - onconflset = (List *) - map_variable_attnos((Node *) onconflset, - firstVarno, 0, - part_attnos, - RelationGetDescr(firstResultRel)->natts, - RelationGetForm(partrel)->reltype, - &found_whole_row); - /* We ignore the value of found_whole_row. */ + onconflset = (List *) + map_variable_attnos((Node *) onconflset, + INNER_VAR, 0, + attmap, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + onconflset = (List *) + map_variable_attnos((Node *) onconflset, + firstVarno, 0, + attmap, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ - /* Finally, adjust this tlist to match the partition. */ - onconflset = adjust_partition_tlist(onconflset, map); - } + /* Finally, adjust this tlist to match the partition. */ + Assert(map != NULL && map->attrMap != NULL); + onconflset = adjust_partition_tlist(onconflset, map); /* * Build UPDATE SET's projection info. The user of this @@ -637,25 +622,22 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, List *clause; clause = copyObject((List *) node->onConflictWhere); - if (!equalTupdescs) - { - clause = (List *) - map_variable_attnos((Node *) clause, - INNER_VAR, 0, - part_attnos, - RelationGetDescr(firstResultRel)->natts, - RelationGetForm(partrel)->reltype, - &found_whole_row); + clause = (List *) + map_variable_attnos((Node *) clause, + INNER_VAR, 0, + attmap, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); + /* We ignore the value of found_whole_row. */ + clause = (List *) + map_variable_attnos((Node *) clause, + firstVarno, 0, + attmap, + RelationGetDescr(firstResultRel)->natts, + RelationGetForm(partrel)->reltype, + &found_whole_row); /* We ignore the value of found_whole_row. */ - clause = (List *) - map_variable_attnos((Node *) clause, - firstVarno, 0, - part_attnos, - RelationGetDescr(firstResultRel)->natts, - RelationGetForm(partrel)->reltype, - &found_whole_row); - /* We ignore the value of found_whole_row. */ - } leaf_part_rri->ri_onConflict->oc_WhereClause = ExecInitQual((List *) clause, &mtstate->ps); } @@ -663,6 +645,8 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, } } + if (attmap) + pfree(attmap); Assert(proute->partitions[partidx] == NULL); proute->partitions[partidx] = leaf_part_rri; diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index c6f3628def..378331e216 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -1504,10 +1504,11 @@ generateClonedIndexStmt(RangeVar *heapRel, Oid heapRelid, Relation source_idx, indexpr_item = lnext(indexpr_item); /* Adjust Vars to match new table's column numbering */ - indexkey = map_variable_attnos(indexkey, - 1, 0, - attmap, attmap_length, - InvalidOid, &found_whole_row); + if (attmap != NULL) + indexkey = map_variable_attnos(indexkey, + 1, 0, + attmap, attmap_length, + InvalidOid, &found_whole_row); /* As in transformTableLikeClause, reject whole-row variables */ if (found_whole_row) -- 2.11.0