From aecb434bf9bc0ce1f4871122d5ab55d3db0957f5 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Fri, 14 Nov 2025 08:02:31 -0500 Subject: [PATCH v1 1/2] Refactor how we form HeapTuples for CatalogTuple(Insert|Update) This commit provides a set of macros that should be used when inserting or updating tuples before calling CatalogTupleUpdate() or CatalogTupleInsert(). Historically there have been two methods for accomplishing this task; Form/GETSTRUCT, and values/nulls/replaces. This new method provides a more intuitive and less error-prone approach without changing the fundamentals of the process. In addition, it is now possible to retain knowledge of the set of mutated attributes when working with catalog tuples. This isn't used in practice (yet), but could be a way to avoid the overhead of HeapDetermineColumnsInfo() in heap_update() where (while holding a lock) we re-discover that set by comparing old/new HeapTuple datums when trying to find what indexed attributes have new values and prevent HOT updates. The "Form/GETSTRUCT" model allows for direct access to the tuple data that is then modified, copied, and then updated via CatalogTupleUpdate(). Old: Form_pg_index relform = (Form_pg_index) GETSTRUCT(tuple); relform->inisclustered = false; CatalogTupleUpdate(pg_index, &tuple->t_self, tuple); New: Bitmapset *updated = NULL; Form_pg_index relform = (Form_pg_index) GETSTRUCT(tuple); HeapTupleUpdateField(pg_index, indisclustered, false, indexForm, updated); CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple, updated, NULL); bms_free(updated); The "values/nulls/replaces" model collects the necessary information to either update or create a heap tuple using heap_modify_tuple() or heap_form_tuple() or sometimes heap_modify_tuple_by_cols(). While all those functions remain unchanged now the prefered function for catalog tuple modification is heap_update_tuple(). Old: bool nulls[Natts_pg_type]; bool replaces[Natts_pg_type]; Datum values[Natts_pg_type]; values[Anum_pg_type_typtype - 1] = CharGetDatum(typeType); nulls[Anum_pg_type_typdefaultbin - 1] = true; replaces[Anum_pg_type_oid - 1] = false; tup = heap_modify_tuple(tuple, desc, values, nulls, replaces); CatalogTupleUpdate(pg_type_desc, &tuple->t_self, tuple); New: Datum values[Natts_pg_type] = {0}; bool nulls[Natts_pg_type] = {false}; Bitmapset *updated = NULL; HeapTupleUpdateValue(pg_type, typtype, CharGetDatum(typeType), values, nulls, updated); HeapTupleUpdateValueNull(pg_type, typdefaultbin, values, nulls, updated); HeapTupleSetColumnNotUpdated(pg_type, oid, updated); tup = heap_update_tuple(tuple, desc, values, nulls, updated); CatalogTupleUpdate(pg_type_desc, &tuple->t_self, tuple, updated, NULL); In addition CatalogTupleUpdate and CatalogTupleInsert now have an optional argument for the CatalogIndexState, when not provided it is created and then released for the caller. The heap_update_tuple() function is functionally equivalent to heap_modify_tuple(), but takes a Bitmapset called "updated" rather than an array of bool generally called "replaces" as a method for indicating what was modified. Additionally, this new function tries to balance the tradeoffs of calling heap_getattr() versus heap_deform_tuple() based on the ratio of attributes updated and their known runtime complexities. Both paths are functionally equivalent. The changes also include initialization of the values/nulls arrays rather than loops or memset(). Some effort was also given to unify naming of these variables and their order so as to lower cognitive overhead. There is no impact to non-catalog related paths. --- src/backend/access/common/heaptuple.c | 119 +++++++++++ src/backend/catalog/indexing.c | 138 ++++++------- src/backend/catalog/pg_aggregate.c | 74 +++---- src/backend/catalog/pg_constraint.c | 131 ++++++------ src/backend/commands/alter.c | 60 +++--- src/backend/commands/analyze.c | 74 +++---- src/backend/statistics/attribute_stats.c | 241 +++++++++++------------ src/include/access/htup.h | 61 ++++++ src/include/access/htup_details.h | 8 + src/include/catalog/indexing.h | 17 +- 10 files changed, 550 insertions(+), 373 deletions(-) diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index 74a52d87067..364f03b043e 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -1325,6 +1325,125 @@ heap_modify_tuple_by_cols(HeapTuple tuple, return newTuple; } +/* + * heap_update_tuple + * Form a new tuple from an old tuple and a set of replacement values. + * + * Creates a new HeapTuple by selectively replacing attributes from the original + * tuple with new values. The 'updated' Bitmapset specifies which attributes + * (by attribute number, 1-based adjusted by FirstLowInvalidHeapAttributeNumber) + * should be replaced with corresponding entries from new_values and new_isnull + * arrays (0-based indices). + * + * Performance strategy: + * - If updating many attributes (> 2*natts/3), use heap_getattr() to extract + * only the few non-updated attributes. This is O(k*n) where k is the number + * of non-updated attributes, which is small when updating many. + * - If updating few attributes (<= 2*natts/3), use heap_deform_tuple() to + * extract all attributes at once (O(n)), then replace the updated ones. + * This avoids the O(n^2) cost of many heap_getattr() calls. + * + * The threshold of 2*natts/3 balances the fixed O(n) cost of heap_deform_tuple + * against the variable O(k*n) cost of heap_getattr, where k = natts - num_updated. + */ +HeapTuple +heap_update_tuple(HeapTuple tuple, + TupleDesc desc, + const Datum *new_values, + const bool *new_nulls, + const Bitmapset *updated) +{ + int natts = desc->natts; + int num_updated; + Datum *values; + bool *nulls; + HeapTuple new_tuple; + + Assert(!bms_is_empty(updated)); + + num_updated = bms_num_members(updated); + Assert(num_updated <= natts); + + values = (Datum *) palloc0(natts * sizeof(Datum)); + nulls = (bool *) palloc0(natts * sizeof(bool)); + + /* + * Choose strategy based on update density. When updating most attributes, + * it's cheaper to extract the few unchanged ones individually. + */ + if (num_updated > (2 * natts) / 3) + { + /* Updating many: use heap_getattr for the few non-updated attributes */ + for (int i = 0; i < natts; i++) + { + /* + * Convert array index to attribute number, then to bitmapset + * member. Array index i (0-based) -> attnum (1-based) -> bms + * member. + */ + AttrNumber attnum = i + 1; + int member = attnum - FirstLowInvalidHeapAttributeNumber; + + if (bms_is_member(member, updated)) + { + /* Use replacement value */ + if (unlikely(!new_values || !new_nulls)) + values[i] = heap_getattr(tuple, attnum, desc, &nulls[i]); + + if (likely(new_values)) + values[i] = new_values[i]; + + if (likely(new_nulls)) + nulls[i] = new_nulls[i]; + } + else + { + /* Extract original value using heap_getattr (1-based) */ + values[i] = heap_getattr(tuple, attnum, desc, &nulls[i]); + } + } + } + else + { + int member = -1; + + /* Updating few: deform entire tuple, then replace updated attributes */ + heap_deform_tuple(tuple, desc, values, nulls); + + while ((member = bms_next_member(updated, member)) >= 0) + { + /* + * Convert bitmapset member to attribute number, then to array + * index. bms_member -> attnum (1-based) -> array index i + * (0-based). + */ + AttrNumber attnum = member + FirstLowInvalidHeapAttributeNumber; + int i = attnum - 1; + + Assert(i >= 0 && i < natts); + + if (likely(new_values)) + values[i] = new_values[i]; + + if (likely(new_nulls)) + nulls[i] = new_nulls[i]; + } + } + + /* Create the new tuple */ + new_tuple = heap_form_tuple(desc, values, nulls); + + pfree(values); + pfree(nulls); + + /* Preserve tuple identity and location information from the original */ + new_tuple->t_data->t_ctid = tuple->t_data->t_ctid; + new_tuple->t_self = tuple->t_self; + new_tuple->t_tableOid = tuple->t_tableOid; + + return new_tuple; +} + /* * heap_deform_tuple * Given a tuple, extract data into values/isnull arrays; this is diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c index 004c5121000..f35c1b6cd88 100644 --- a/src/backend/catalog/indexing.c +++ b/src/backend/catalog/indexing.c @@ -225,58 +225,67 @@ CatalogTupleCheckConstraints(Relation heapRel, HeapTuple tup) * * This is a convenience routine for the common case of inserting a single * tuple in a system catalog; it inserts a new heap tuple, keeping indexes - * current. Avoid using it for multiple tuples, since opening the indexes - * and building the index info structures is moderately expensive. - * (Use CatalogTupleInsertWithInfo in such cases.) + * current. + * + * If 'indstate' is NULL, the function opens and closes the indexes internally. + * This is convenient for single-tuple updates but has overhead from opening the + * indexes and building index info structures. + * + * If 'indstate' is provided (non-NULL), it is used directly without opening or + * closing indexes. This allows callers to amortize index management costs across + * multiple tuple updates. Callers must use CatalogOpenIndexes() before the first + * update and CatalogCloseIndexes() after the last update. + * + * XXX: At some point we might cache the CatalogIndexState data somewhere (perhaps + * in the relcache) so that callers needn't trouble over this. */ void -CatalogTupleInsert(Relation heapRel, HeapTuple tup) +CatalogTupleInsert(Relation heapRel, HeapTuple tup, + CatalogIndexState indstate) { - CatalogIndexState indstate; - - CatalogTupleCheckConstraints(heapRel, tup); - - indstate = CatalogOpenIndexes(heapRel); + bool close_indexes = false; - simple_heap_insert(heapRel, tup); - - CatalogIndexInsert(indstate, tup, TU_All); - CatalogCloseIndexes(indstate); -} + /* Open indexes if not provided by caller */ + if (indstate == NULL) + { + indstate = CatalogOpenIndexes(heapRel); + close_indexes = true; + } -/* - * CatalogTupleInsertWithInfo - as above, but with caller-supplied index info - * - * This should be used when it's important to amortize CatalogOpenIndexes/ - * CatalogCloseIndexes work across multiple insertions. At some point we - * might cache the CatalogIndexState data somewhere (perhaps in the relcache) - * so that callers needn't trouble over this ... but we don't do so today. - */ -void -CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup, - CatalogIndexState indstate) -{ CatalogTupleCheckConstraints(heapRel, tup); simple_heap_insert(heapRel, tup); CatalogIndexInsert(indstate, tup, TU_All); + + /* Close indexes only if we opened them ourselves */ + if (close_indexes) + CatalogCloseIndexes(indstate); } /* - * CatalogTuplesMultiInsertWithInfo - as above, but for multiple tuples + * CatalogTuplesMultiInsert - as above, but for multiple tuples * * Insert multiple tuples into the given catalog relation at once, with an * amortized cost of CatalogOpenIndexes. */ void -CatalogTuplesMultiInsertWithInfo(Relation heapRel, TupleTableSlot **slot, - int ntuples, CatalogIndexState indstate) +CatalogTuplesMultiInsert(Relation heapRel, TupleTableSlot **slot, + int ntuples, CatalogIndexState indstate) { + bool close_indexes = false; + /* Nothing to do */ if (ntuples <= 0) return; + /* Open indexes if not provided by caller */ + if (indstate == NULL) + { + indstate = CatalogOpenIndexes(heapRel); + close_indexes = true; + } + heap_multi_insert(heapRel, slot, ntuples, GetCurrentCommandId(true), 0, NULL); @@ -296,6 +305,10 @@ CatalogTuplesMultiInsertWithInfo(Relation heapRel, TupleTableSlot **slot, if (should_free) heap_freetuple(tuple); } + + /* Close indexes only if we opened them ourselves */ + if (close_indexes) + CatalogCloseIndexes(indstate); } /* @@ -303,47 +316,45 @@ CatalogTuplesMultiInsertWithInfo(Relation heapRel, TupleTableSlot **slot, * * Update the tuple identified by "otid", replacing it with the data in "tup". * - * This is a convenience routine for the common case of updating a single - * tuple in a system catalog; it updates one heap tuple, keeping indexes - * current. Avoid using it for multiple tuples, since opening the indexes - * and building the index info structures is moderately expensive. - * (Use CatalogTupleUpdateWithInfo in such cases.) + * This function updates a heap tuple in a system catalog and keeps its indexes + * current. The 'updated' bitmapset specifies which columns were modified, using + * the same encoding as HeapDetermineColumnsInfo(). + * + * If 'indstate' is NULL, the function opens and closes the indexes internally. + * This is convenient for single-tuple updates but has overhead from opening the + * indexes and building index info structures. + * + * If 'indstate' is provided (non-NULL), it is used directly without opening or + * closing indexes. This allows callers to amortize index management costs across + * multiple tuple updates. Callers must use CatalogOpenIndexes() before the first + * update and CatalogCloseIndexes() after the last update. + * + * XXX: At some point we might cache the CatalogIndexState data somewhere (perhaps + * in the relcache) so that callers needn't trouble over this. */ void -CatalogTupleUpdate(Relation heapRel, const ItemPointerData *otid, HeapTuple tup) +CatalogTupleUpdate(Relation heapRel, const ItemPointerData *otid, HeapTuple tuple, + const Bitmapset *updated, CatalogIndexState indstate) { - CatalogIndexState indstate; TU_UpdateIndexes updateIndexes = TU_All; + bool close_indexes = false; - CatalogTupleCheckConstraints(heapRel, tup); - - indstate = CatalogOpenIndexes(heapRel); - - simple_heap_update(heapRel, otid, tup, &updateIndexes); - - CatalogIndexInsert(indstate, tup, updateIndexes); - CatalogCloseIndexes(indstate); -} + CatalogTupleCheckConstraints(heapRel, tuple); -/* - * CatalogTupleUpdateWithInfo - as above, but with caller-supplied index info - * - * This should be used when it's important to amortize CatalogOpenIndexes/ - * CatalogCloseIndexes work across multiple updates. At some point we - * might cache the CatalogIndexState data somewhere (perhaps in the relcache) - * so that callers needn't trouble over this ... but we don't do so today. - */ -void -CatalogTupleUpdateWithInfo(Relation heapRel, const ItemPointerData *otid, HeapTuple tup, - CatalogIndexState indstate) -{ - TU_UpdateIndexes updateIndexes = TU_All; + /* Open indexes if not provided by caller */ + if (indstate == NULL) + { + indstate = CatalogOpenIndexes(heapRel); + close_indexes = true; + } - CatalogTupleCheckConstraints(heapRel, tup); + simple_heap_update(heapRel, otid, tuple, &updateIndexes); - simple_heap_update(heapRel, otid, tup, &updateIndexes); + CatalogIndexInsert(indstate, tuple, updateIndexes); - CatalogIndexInsert(indstate, tup, updateIndexes); + /* Close indexes only if we opened them ourselves */ + if (close_indexes) + CatalogCloseIndexes(indstate); } /* @@ -355,11 +366,6 @@ CatalogTupleUpdateWithInfo(Relation heapRel, const ItemPointerData *otid, HeapTu * cleanup will be done later by VACUUM. However, callers of this function * shouldn't have to know that; we'd like a uniform abstraction for all * catalog tuple changes. Hence, provide this currently-trivial wrapper. - * - * The abstraction is a bit leaky in that we don't provide an optimized - * CatalogTupleDeleteWithInfo version, because there is currently nothing to - * optimize. If we ever need that, rather than touching a lot of call sites, - * it might be better to do something about caching CatalogIndexState. */ void CatalogTupleDelete(Relation heapRel, const ItemPointerData *tid) diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index a1cb5719a0c..ddfa94b3aba 100644 --- a/src/backend/catalog/pg_aggregate.c +++ b/src/backend/catalog/pg_aggregate.c @@ -79,9 +79,9 @@ AggregateCreate(const char *aggName, Relation aggdesc; HeapTuple tup; HeapTuple oldtup; - bool nulls[Natts_pg_aggregate]; - Datum values[Natts_pg_aggregate]; - bool replaces[Natts_pg_aggregate]; + bool nulls[Natts_pg_aggregate] = {false}; + Datum values[Natts_pg_aggregate] = {0}; + Bitmapset *updated = NULL; Form_pg_proc proc; Oid transfn; Oid finalfn = InvalidOid; /* can be omitted */ @@ -102,7 +102,6 @@ AggregateCreate(const char *aggName, Oid procOid; TupleDesc tupDesc; char *detailmsg; - int i; ObjectAddress myself, referenced; ObjectAddresses *addrs; @@ -584,7 +583,7 @@ AggregateCreate(const char *aggName, /* * permission checks on used types */ - for (i = 0; i < numArgs; i++) + for (int i = 0; i < numArgs; i++) { aclresult = object_aclcheck(TypeRelationId, aggArgTypes[i], GetUserId(), ACL_USAGE); if (aclresult != ACLCHECK_OK) @@ -651,40 +650,34 @@ AggregateCreate(const char *aggName, tupDesc = aggdesc->rd_att; /* initialize nulls and values */ - for (i = 0; i < Natts_pg_aggregate; i++) - { - nulls[i] = false; - values[i] = (Datum) 0; - replaces[i] = true; - } - values[Anum_pg_aggregate_aggfnoid - 1] = ObjectIdGetDatum(procOid); - values[Anum_pg_aggregate_aggkind - 1] = CharGetDatum(aggKind); - values[Anum_pg_aggregate_aggnumdirectargs - 1] = Int16GetDatum(numDirectArgs); - values[Anum_pg_aggregate_aggtransfn - 1] = ObjectIdGetDatum(transfn); - values[Anum_pg_aggregate_aggfinalfn - 1] = ObjectIdGetDatum(finalfn); - values[Anum_pg_aggregate_aggcombinefn - 1] = ObjectIdGetDatum(combinefn); - values[Anum_pg_aggregate_aggserialfn - 1] = ObjectIdGetDatum(serialfn); - values[Anum_pg_aggregate_aggdeserialfn - 1] = ObjectIdGetDatum(deserialfn); - values[Anum_pg_aggregate_aggmtransfn - 1] = ObjectIdGetDatum(mtransfn); - values[Anum_pg_aggregate_aggminvtransfn - 1] = ObjectIdGetDatum(minvtransfn); - values[Anum_pg_aggregate_aggmfinalfn - 1] = ObjectIdGetDatum(mfinalfn); - values[Anum_pg_aggregate_aggfinalextra - 1] = BoolGetDatum(finalfnExtraArgs); - values[Anum_pg_aggregate_aggmfinalextra - 1] = BoolGetDatum(mfinalfnExtraArgs); - values[Anum_pg_aggregate_aggfinalmodify - 1] = CharGetDatum(finalfnModify); - values[Anum_pg_aggregate_aggmfinalmodify - 1] = CharGetDatum(mfinalfnModify); - values[Anum_pg_aggregate_aggsortop - 1] = ObjectIdGetDatum(sortop); - values[Anum_pg_aggregate_aggtranstype - 1] = ObjectIdGetDatum(aggTransType); - values[Anum_pg_aggregate_aggtransspace - 1] = Int32GetDatum(aggTransSpace); - values[Anum_pg_aggregate_aggmtranstype - 1] = ObjectIdGetDatum(aggmTransType); - values[Anum_pg_aggregate_aggmtransspace - 1] = Int32GetDatum(aggmTransSpace); + HeapTupleUpdateValue(pg_aggregate, aggfnoid, ObjectIdGetDatum(procOid), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggkind, CharGetDatum(aggKind), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggnumdirectargs, Int16GetDatum(numDirectArgs), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggtransfn, ObjectIdGetDatum(transfn), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggfinalfn, ObjectIdGetDatum(finalfn), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggcombinefn, ObjectIdGetDatum(combinefn), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggserialfn, ObjectIdGetDatum(serialfn), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggdeserialfn, ObjectIdGetDatum(deserialfn), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggmtransfn, ObjectIdGetDatum(mtransfn), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggminvtransfn, ObjectIdGetDatum(minvtransfn), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggmfinalfn, ObjectIdGetDatum(mfinalfn), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggfinalextra, BoolGetDatum(finalfnExtraArgs), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggmfinalextra, BoolGetDatum(mfinalfnExtraArgs), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggfinalmodify, CharGetDatum(finalfnModify), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggmfinalmodify, CharGetDatum(mfinalfnModify), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggsortop, ObjectIdGetDatum(sortop), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggtranstype, ObjectIdGetDatum(aggTransType), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggtransspace, Int32GetDatum(aggTransSpace), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggmtranstype, ObjectIdGetDatum(aggmTransType), values, nulls, updated); + HeapTupleUpdateValue(pg_aggregate, aggmtransspace, Int32GetDatum(aggmTransSpace), values, nulls, updated); if (agginitval) - values[Anum_pg_aggregate_agginitval - 1] = CStringGetTextDatum(agginitval); + HeapTupleUpdateValue(pg_aggregate, agginitval, CStringGetTextDatum(agginitval), values, nulls, updated); else - nulls[Anum_pg_aggregate_agginitval - 1] = true; + HeapTupleUpdateValueNull(pg_aggregate, agginitval, values, nulls, updated); if (aggminitval) - values[Anum_pg_aggregate_aggminitval - 1] = CStringGetTextDatum(aggminitval); + HeapTupleUpdateValue(pg_aggregate, aggminitval, CStringGetTextDatum(aggminitval), values, nulls, updated); else - nulls[Anum_pg_aggregate_aggminitval - 1] = true; + HeapTupleUpdateValueNull(pg_aggregate, aggminitval, values, nulls, updated); if (replace) oldtup = SearchSysCache1(AGGFNOID, ObjectIdGetDatum(procOid)); @@ -717,20 +710,17 @@ AggregateCreate(const char *aggName, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), errmsg("cannot change number of direct arguments of an aggregate function"))); - replaces[Anum_pg_aggregate_aggfnoid - 1] = false; - replaces[Anum_pg_aggregate_aggkind - 1] = false; - replaces[Anum_pg_aggregate_aggnumdirectargs - 1] = false; - - tup = heap_modify_tuple(oldtup, tupDesc, values, nulls, replaces); - CatalogTupleUpdate(aggdesc, &tup->t_self, tup); + tup = heap_update_tuple(oldtup, tupDesc, values, nulls, updated); + CatalogTupleUpdate(aggdesc, &tup->t_self, tup, updated, NULL); ReleaseSysCache(oldtup); } else { tup = heap_form_tuple(tupDesc, values, nulls); - CatalogTupleInsert(aggdesc, tup); + CatalogTupleInsert(aggdesc, tup, NULL); } + bms_free(updated); table_close(aggdesc, RowExclusiveLock); /* diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 9944e4bd2d1..855bdf46015 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -85,8 +85,8 @@ CreateConstraintEntry(const char *constraintName, Relation conDesc; Oid conOid; HeapTuple tup; - bool nulls[Natts_pg_constraint]; - Datum values[Natts_pg_constraint]; + Datum values[Natts_pg_constraint] = {0}; + bool nulls[Natts_pg_constraint] = {false}; ArrayType *conkeyArray; ArrayType *confkeyArray; ArrayType *conpfeqopArray; @@ -184,70 +184,70 @@ CreateConstraintEntry(const char *constraintName, conOid = GetNewOidWithIndex(conDesc, ConstraintOidIndexId, Anum_pg_constraint_oid); - values[Anum_pg_constraint_oid - 1] = ObjectIdGetDatum(conOid); - values[Anum_pg_constraint_conname - 1] = NameGetDatum(&cname); - values[Anum_pg_constraint_connamespace - 1] = ObjectIdGetDatum(constraintNamespace); - values[Anum_pg_constraint_contype - 1] = CharGetDatum(constraintType); - values[Anum_pg_constraint_condeferrable - 1] = BoolGetDatum(isDeferrable); - values[Anum_pg_constraint_condeferred - 1] = BoolGetDatum(isDeferred); - values[Anum_pg_constraint_conenforced - 1] = BoolGetDatum(isEnforced); - values[Anum_pg_constraint_convalidated - 1] = BoolGetDatum(isValidated); - values[Anum_pg_constraint_conrelid - 1] = ObjectIdGetDatum(relId); - values[Anum_pg_constraint_contypid - 1] = ObjectIdGetDatum(domainId); - values[Anum_pg_constraint_conindid - 1] = ObjectIdGetDatum(indexRelId); - values[Anum_pg_constraint_conparentid - 1] = ObjectIdGetDatum(parentConstrId); - values[Anum_pg_constraint_confrelid - 1] = ObjectIdGetDatum(foreignRelId); - values[Anum_pg_constraint_confupdtype - 1] = CharGetDatum(foreignUpdateType); - values[Anum_pg_constraint_confdeltype - 1] = CharGetDatum(foreignDeleteType); - values[Anum_pg_constraint_confmatchtype - 1] = CharGetDatum(foreignMatchType); - values[Anum_pg_constraint_conislocal - 1] = BoolGetDatum(conIsLocal); - values[Anum_pg_constraint_coninhcount - 1] = Int16GetDatum(conInhCount); - values[Anum_pg_constraint_connoinherit - 1] = BoolGetDatum(conNoInherit); - values[Anum_pg_constraint_conperiod - 1] = BoolGetDatum(conPeriod); + HeapTupleSetValue(pg_constraint, oid, ObjectIdGetDatum(conOid), values); + HeapTupleSetValue(pg_constraint, conname, NameGetDatum(&cname), values); + HeapTupleSetValue(pg_constraint, connamespace, ObjectIdGetDatum(constraintNamespace), values); + HeapTupleSetValue(pg_constraint, contype, CharGetDatum(constraintType), values); + HeapTupleSetValue(pg_constraint, condeferrable, BoolGetDatum(isDeferrable), values); + HeapTupleSetValue(pg_constraint, condeferred, BoolGetDatum(isDeferred), values); + HeapTupleSetValue(pg_constraint, conenforced, BoolGetDatum(isEnforced), values); + HeapTupleSetValue(pg_constraint, convalidated, BoolGetDatum(isValidated), values); + HeapTupleSetValue(pg_constraint, conrelid, ObjectIdGetDatum(relId), values); + HeapTupleSetValue(pg_constraint, contypid, ObjectIdGetDatum(domainId), values); + HeapTupleSetValue(pg_constraint, conindid, ObjectIdGetDatum(indexRelId), values); + HeapTupleSetValue(pg_constraint, conparentid, ObjectIdGetDatum(parentConstrId), values); + HeapTupleSetValue(pg_constraint, confrelid, ObjectIdGetDatum(foreignRelId), values); + HeapTupleSetValue(pg_constraint, confupdtype, CharGetDatum(foreignUpdateType), values); + HeapTupleSetValue(pg_constraint, confdeltype, CharGetDatum(foreignDeleteType), values); + HeapTupleSetValue(pg_constraint, confmatchtype, CharGetDatum(foreignMatchType), values); + HeapTupleSetValue(pg_constraint, conislocal, BoolGetDatum(conIsLocal), values); + HeapTupleSetValue(pg_constraint, coninhcount, Int16GetDatum(conInhCount), values); + HeapTupleSetValue(pg_constraint, connoinherit, BoolGetDatum(conNoInherit), values); + HeapTupleSetValue(pg_constraint, conperiod, BoolGetDatum(conPeriod), values); if (conkeyArray) - values[Anum_pg_constraint_conkey - 1] = PointerGetDatum(conkeyArray); + HeapTupleSetValue(pg_constraint, conkey, PointerGetDatum(conkeyArray), values); else - nulls[Anum_pg_constraint_conkey - 1] = true; + HeapTupleSetValueNull(pg_constraint, conkey, values, nulls); if (confkeyArray) - values[Anum_pg_constraint_confkey - 1] = PointerGetDatum(confkeyArray); + HeapTupleSetValue(pg_constraint, confkey, PointerGetDatum(confkeyArray), values); else - nulls[Anum_pg_constraint_confkey - 1] = true; + HeapTupleSetValueNull(pg_constraint, confkey, values, nulls); if (conpfeqopArray) - values[Anum_pg_constraint_conpfeqop - 1] = PointerGetDatum(conpfeqopArray); + HeapTupleSetValue(pg_constraint, conpfeqop, PointerGetDatum(conpfeqopArray), values); else - nulls[Anum_pg_constraint_conpfeqop - 1] = true; + HeapTupleSetValueNull(pg_constraint, conpfeqop, values, nulls); if (conppeqopArray) - values[Anum_pg_constraint_conppeqop - 1] = PointerGetDatum(conppeqopArray); + HeapTupleSetValue(pg_constraint, conppeqop, PointerGetDatum(conppeqopArray), values); else - nulls[Anum_pg_constraint_conppeqop - 1] = true; + HeapTupleSetValueNull(pg_constraint, conppeqop, values, nulls); if (conffeqopArray) - values[Anum_pg_constraint_conffeqop - 1] = PointerGetDatum(conffeqopArray); + HeapTupleSetValue(pg_constraint, conffeqop, PointerGetDatum(conffeqopArray), values); else - nulls[Anum_pg_constraint_conffeqop - 1] = true; + HeapTupleSetValueNull(pg_constraint, conffeqop, values, nulls); if (confdelsetcolsArray) - values[Anum_pg_constraint_confdelsetcols - 1] = PointerGetDatum(confdelsetcolsArray); + HeapTupleSetValue(pg_constraint, confdelsetcols, PointerGetDatum(confdelsetcolsArray), values); else - nulls[Anum_pg_constraint_confdelsetcols - 1] = true; + HeapTupleSetValueNull(pg_constraint, confdelsetcols, values, nulls); if (conexclopArray) - values[Anum_pg_constraint_conexclop - 1] = PointerGetDatum(conexclopArray); + HeapTupleSetValue(pg_constraint, conexclop, PointerGetDatum(conexclopArray), values); else - nulls[Anum_pg_constraint_conexclop - 1] = true; + HeapTupleSetValueNull(pg_constraint, conexclop, values, nulls); if (conBin) - values[Anum_pg_constraint_conbin - 1] = CStringGetTextDatum(conBin); + HeapTupleSetValue(pg_constraint, conbin, CStringGetTextDatum(conBin), values); else - nulls[Anum_pg_constraint_conbin - 1] = true; + HeapTupleSetValueNull(pg_constraint, conbin, values, nulls); tup = heap_form_tuple(RelationGetDescr(conDesc), values, nulls); - CatalogTupleInsert(conDesc, tup); + CatalogTupleInsert(conDesc, tup, NULL); ObjectAddressSet(conobject, ConstraintRelationId, conOid); @@ -748,7 +748,7 @@ AdjustNotNullInheritance(Oid relid, AttrNumber attnum, { Relation pg_constraint; Form_pg_constraint conform; - bool changed = false; + Bitmapset *updated = NULL; pg_constraint = table_open(ConstraintRelationId, RowExclusiveLock); conform = (Form_pg_constraint) GETSTRUCT(tup); @@ -784,19 +784,21 @@ AdjustNotNullInheritance(Oid relid, AttrNumber attnum, ereport(ERROR, errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("too many inheritance parents")); - changed = true; + HeapTupleMarkColumnUpdated(pg_constraint, coninhcount, updated); } else if (!conform->conislocal) { - conform->conislocal = true; - changed = true; + HeapTupleUpdateField(pg_constraint, conislocal, true, conform, updated); } - if (changed) - CatalogTupleUpdate(pg_constraint, &tup->t_self, tup); + if (!bms_is_empty(updated)) + { + CatalogTupleUpdate(pg_constraint, &tup->t_self, tup, updated, NULL); + } table_close(pg_constraint, RowExclusiveLock); + bms_free(updated); return true; } @@ -928,6 +930,7 @@ RemoveConstraintById(Oid conId) Relation pgrel; HeapTuple relTup; Form_pg_class classForm; + Bitmapset *updated = NULL; pgrel = table_open(RelationRelationId, RowExclusiveLock); relTup = SearchSysCacheCopy1(RELOID, @@ -938,14 +941,14 @@ RemoveConstraintById(Oid conId) classForm = (Form_pg_class) GETSTRUCT(relTup); if (classForm->relchecks > 0) - classForm->relchecks--; + HeapTupleUpdateField(pg_class, relchecks, classForm->relchecks - 1, classForm, updated); else /* should not happen */ elog(WARNING, "relation \"%s\" has relchecks = %d", RelationGetRelationName(rel), classForm->relchecks); - CatalogTupleUpdate(pgrel, &relTup->t_self, relTup); - + CatalogTupleUpdate(pgrel, &relTup->t_self, relTup, updated, NULL); + bms_free(updated); heap_freetuple(relTup); table_close(pgrel, RowExclusiveLock); @@ -990,6 +993,7 @@ RenameConstraintById(Oid conId, const char *newname) Relation conDesc; HeapTuple tuple; Form_pg_constraint con; + Bitmapset *updated = NULL; conDesc = table_open(ConstraintRelationId, RowExclusiveLock); @@ -1020,11 +1024,13 @@ RenameConstraintById(Oid conId, const char *newname) /* OK, do the rename --- tuple is a copy, so OK to scribble on it */ namestrcpy(&(con->conname), newname); + HeapTupleMarkColumnUpdated(pg_constraint, conname, updated); - CatalogTupleUpdate(conDesc, &tuple->t_self, tuple); + CatalogTupleUpdate(conDesc, &tuple->t_self, tuple, updated, NULL); InvokeObjectPostAlterHook(ConstraintRelationId, conId, 0); + bms_free(updated); heap_freetuple(tuple); table_close(conDesc, RowExclusiveLock); } @@ -1072,15 +1078,17 @@ AlterConstraintNamespaces(Oid ownerId, Oid oldNspId, /* Don't update if the object is already part of the namespace */ if (conform->connamespace == oldNspId && oldNspId != newNspId) { + Bitmapset *updated = NULL; + tup = heap_copytuple(tup); conform = (Form_pg_constraint) GETSTRUCT(tup); - conform->connamespace = newNspId; - - CatalogTupleUpdate(conRel, &tup->t_self, tup); + HeapTupleUpdateField(pg_constraint, connamespace, newNspId, conform, updated); + CatalogTupleUpdate(conRel, &tup->t_self, tup, updated, NULL); + bms_free(updated); /* - * Note: currently, the constraint will not have its own + * NOTE: currently, the constraint will not have its own * dependency on the namespace, so we don't need to do * changeDependencyFor(). */ @@ -1116,6 +1124,7 @@ ConstraintSetParentConstraint(Oid childConstrId, newtup; ObjectAddress depender; ObjectAddress referenced; + Bitmapset *updated = NULL; constrRel = table_open(ConstraintRelationId, RowExclusiveLock); tuple = SearchSysCache1(CONSTROID, ObjectIdGetDatum(childConstrId)); @@ -1131,16 +1140,17 @@ ConstraintSetParentConstraint(Oid childConstrId, elog(ERROR, "constraint %u already has a parent constraint", childConstrId); - constrForm->conislocal = false; + HeapTupleUpdateField(pg_constraint, conislocal, false, constrForm, updated); if (pg_add_s16_overflow(constrForm->coninhcount, 1, &constrForm->coninhcount)) ereport(ERROR, errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("too many inheritance parents")); - constrForm->conparentid = parentConstrId; + HeapTupleMarkColumnUpdated(pg_constraint, coninhcount, updated); + HeapTupleUpdateField(pg_constraint, conparentid, parentConstrId, constrForm, updated); - CatalogTupleUpdate(constrRel, &tuple->t_self, newtup); + CatalogTupleUpdate(constrRel, &tuple->t_self, newtup, updated, NULL); ObjectAddressSet(depender, ConstraintRelationId, childConstrId); @@ -1152,14 +1162,14 @@ ConstraintSetParentConstraint(Oid childConstrId, } else { - constrForm->coninhcount--; - constrForm->conislocal = true; - constrForm->conparentid = InvalidOid; + HeapTupleUpdateField(pg_constraint, coninhcount, constrForm->coninhcount - 1, constrForm, updated); + HeapTupleUpdateField(pg_constraint, conislocal, true, constrForm, updated); + HeapTupleUpdateField(pg_constraint, conparentid, InvalidOid, constrForm, updated); /* Make sure there's no further inheritance. */ Assert(constrForm->coninhcount == 0); - CatalogTupleUpdate(constrRel, &tuple->t_self, newtup); + CatalogTupleUpdate(constrRel, &tuple->t_self, newtup, updated, NULL); deleteDependencyRecordsForClass(ConstraintRelationId, childConstrId, ConstraintRelationId, @@ -1169,6 +1179,7 @@ ConstraintSetParentConstraint(Oid childConstrId, DEPENDENCY_PARTITION_SEC); } + bms_free(updated); ReleaseSysCache(tuple); table_close(constrRel, RowExclusiveLock); } diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c index cb75e11fced..f5f8ec791c3 100644 --- a/src/backend/commands/alter.c +++ b/src/backend/commands/alter.c @@ -180,7 +180,7 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) AclResult aclresult; Datum *values; bool *nulls; - bool *replaces; + Bitmapset *updated = NULL; NameData nameattrdata; oldtup = SearchSysCache1(oidCacheId, ObjectIdGetDatum(objectId)); @@ -326,15 +326,21 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) /* Build modified tuple */ values = palloc0(RelationGetNumberOfAttributes(rel) * sizeof(Datum)); nulls = palloc0(RelationGetNumberOfAttributes(rel) * sizeof(bool)); - replaces = palloc0(RelationGetNumberOfAttributes(rel) * sizeof(bool)); + + /* + * NOTE: We can't use the HeapTupleMarkColumnUpdated() macro here because + * 'Anum_name' isn't a table/column name, it's a index for the relation + * passed into the function as an argument. + */ namestrcpy(&nameattrdata, new_name); values[Anum_name - 1] = NameGetDatum(&nameattrdata); - replaces[Anum_name - 1] = true; - newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel), - values, nulls, replaces); + updated = bms_add_member(updated, Anum_name - FirstLowInvalidHeapAttributeNumber); + + newtup = heap_update_tuple(oldtup, RelationGetDescr(rel), + values, nulls, updated); /* Perform actual update */ - CatalogTupleUpdate(rel, &oldtup->t_self, newtup); + CatalogTupleUpdate(rel, &oldtup->t_self, newtup, updated, NULL); InvokeObjectPostAlterHook(classId, objectId, 0); @@ -357,7 +363,7 @@ AlterObjectRename_internal(Relation rel, Oid objectId, const char *new_name) /* Release memory */ pfree(values); pfree(nulls); - pfree(replaces); + bms_free(updated); heap_freetuple(newtup); ReleaseSysCache(oldtup); @@ -705,7 +711,7 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) newtup; Datum *values; bool *nulls; - bool *replaces; + Bitmapset *updated = NULL; tup = SearchSysCacheCopy1(oidCacheId, ObjectIdGetDatum(objid)); if (!HeapTupleIsValid(tup)) /* should not happen */ @@ -804,19 +810,21 @@ AlterObjectNamespace_internal(Relation rel, Oid objid, Oid nspOid) /* Build modified tuple */ values = palloc0(RelationGetNumberOfAttributes(rel) * sizeof(Datum)); nulls = palloc0(RelationGetNumberOfAttributes(rel) * sizeof(bool)); - replaces = palloc0(RelationGetNumberOfAttributes(rel) * sizeof(bool)); + + /* NOTE: Don't use the HeapTupleMarkColumnUpdated() macro here either. */ values[Anum_namespace - 1] = ObjectIdGetDatum(nspOid); - replaces[Anum_namespace - 1] = true; - newtup = heap_modify_tuple(tup, RelationGetDescr(rel), - values, nulls, replaces); + updated = bms_add_member(updated, Anum_namespace - FirstLowInvalidHeapAttributeNumber); + + newtup = heap_update_tuple(tup, RelationGetDescr(rel), + values, nulls, updated); /* Perform actual update */ - CatalogTupleUpdate(rel, &tup->t_self, newtup); + CatalogTupleUpdate(rel, &tup->t_self, newtup, updated, NULL); /* Release memory */ pfree(values); pfree(nulls); - pfree(replaces); + bms_free(updated); /* update dependency to point to the new schema */ if (changeDependencyFor(classId, objid, @@ -967,7 +975,7 @@ AlterObjectOwner_internal(Oid classId, Oid objectId, Oid new_ownerId) HeapTuple newtup; Datum *values; bool *nulls; - bool *replaces; + Bitmapset *updated = NULL; /* Superusers can bypass permission checks */ if (!superuser()) @@ -1014,9 +1022,12 @@ AlterObjectOwner_internal(Oid classId, Oid objectId, Oid new_ownerId) nattrs = RelationGetNumberOfAttributes(rel); values = palloc0(nattrs * sizeof(Datum)); nulls = palloc0(nattrs * sizeof(bool)); - replaces = palloc0(nattrs * sizeof(bool)); + + /* + * NOTE: Don't use the HeapTupleMarkColumnUpdated() macro here either. + */ values[Anum_owner - 1] = ObjectIdGetDatum(new_ownerId); - replaces[Anum_owner - 1] = true; + updated = bms_add_member(updated, Anum_owner - FirstLowInvalidHeapAttributeNumber); /* * Determine the modified ACL for the new owner. This is only @@ -1032,16 +1043,21 @@ AlterObjectOwner_internal(Oid classId, Oid objectId, Oid new_ownerId) newAcl = aclnewowner(DatumGetAclP(datum), old_ownerId, new_ownerId); + + /* + * NOTE: Don't use the HeapTupleMarkColumnUpdated() macro here + * either. + */ values[Anum_acl - 1] = PointerGetDatum(newAcl); - replaces[Anum_acl - 1] = true; + updated = bms_add_member(updated, Anum_acl - FirstLowInvalidHeapAttributeNumber); } } - newtup = heap_modify_tuple(oldtup, RelationGetDescr(rel), - values, nulls, replaces); + newtup = heap_update_tuple(oldtup, RelationGetDescr(rel), + values, nulls, updated); /* Perform actual update */ - CatalogTupleUpdate(rel, &newtup->t_self, newtup); + CatalogTupleUpdate(rel, &newtup->t_self, newtup, updated, NULL); UnlockTuple(rel, &oldtup->t_self, InplaceUpdateTupleLock); @@ -1051,7 +1067,7 @@ AlterObjectOwner_internal(Oid classId, Oid objectId, Oid new_ownerId) /* Release memory */ pfree(values); pfree(nulls); - pfree(replaces); + bms_free(updated); } else UnlockTuple(rel, &oldtup->t_self, InplaceUpdateTupleLock); diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 25089fae3e0..514742195b8 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -12,6 +12,7 @@ * *------------------------------------------------------------------------- */ +#include "catalog/pg_statistic_d.h" #include "postgres.h" #include @@ -1668,48 +1669,34 @@ update_attstats(Oid relid, bool inh, int natts, VacAttrStats **vacattrstats) VacAttrStats *stats = vacattrstats[attno]; HeapTuple stup, oldtup; - int i, - k, + int k, n; - Datum values[Natts_pg_statistic]; - bool nulls[Natts_pg_statistic]; - bool replaces[Natts_pg_statistic]; + Datum values[Natts_pg_statistic] = {0}; + bool nulls[Natts_pg_statistic] = {false}; + Bitmapset *updated = NULL; /* Ignore attr if we weren't able to collect stats */ if (!stats->stats_valid) continue; - /* - * Construct a new pg_statistic tuple - */ - for (i = 0; i < Natts_pg_statistic; ++i) - { - nulls[i] = false; - replaces[i] = true; - } + /* Construct a new pg_statistic tuple */ + HeapTupleUpdateSetAllColumnsUpdated(pg_statistic, updated); + HeapTupleSetValue(pg_statistic, starelid, ObjectIdGetDatum(relid), values); + HeapTupleSetValue(pg_statistic, staattnum, Int16GetDatum(stats->tupattnum), values); + HeapTupleSetValue(pg_statistic, stainherit, BoolGetDatum(inh), values); + HeapTupleSetValue(pg_statistic, stanullfrac, Float4GetDatum(stats->stanullfrac), values); + HeapTupleSetValue(pg_statistic, stawidth, Int32GetDatum(stats->stawidth), values); + HeapTupleSetValue(pg_statistic, stadistinct, Float4GetDatum(stats->stadistinct), values); - values[Anum_pg_statistic_starelid - 1] = ObjectIdGetDatum(relid); - values[Anum_pg_statistic_staattnum - 1] = Int16GetDatum(stats->tupattnum); - values[Anum_pg_statistic_stainherit - 1] = BoolGetDatum(inh); - values[Anum_pg_statistic_stanullfrac - 1] = Float4GetDatum(stats->stanullfrac); - values[Anum_pg_statistic_stawidth - 1] = Int32GetDatum(stats->stawidth); - values[Anum_pg_statistic_stadistinct - 1] = Float4GetDatum(stats->stadistinct); - i = Anum_pg_statistic_stakind1 - 1; for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = Int16GetDatum(stats->stakind[k]); /* stakindN */ - } - i = Anum_pg_statistic_staop1 - 1; + HeapTupleSetValue(pg_statistic, stakind1 + k, Int16GetDatum(stats->stakind[k]), values); + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = ObjectIdGetDatum(stats->staop[k]); /* staopN */ - } - i = Anum_pg_statistic_stacoll1 - 1; + HeapTupleSetValue(pg_statistic, staop1 + k, ObjectIdGetDatum(stats->staop[k]), values); + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) - { - values[i++] = ObjectIdGetDatum(stats->stacoll[k]); /* stacollN */ - } - i = Anum_pg_statistic_stanumbers1 - 1; + HeapTupleSetValue(pg_statistic, stacoll1 + k, ObjectIdGetDatum(stats->stacoll[k]), values); + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) { if (stats->stanumbers[k] != NULL) @@ -1721,15 +1708,12 @@ update_attstats(Oid relid, bool inh, int natts, VacAttrStats **vacattrstats) for (n = 0; n < nnum; n++) numdatums[n] = Float4GetDatum(stats->stanumbers[k][n]); arry = construct_array_builtin(numdatums, nnum, FLOAT4OID); - values[i++] = PointerGetDatum(arry); /* stanumbersN */ + HeapTupleSetValue(pg_statistic, stanumbers1 + k, PointerGetDatum(arry), values); } else - { - nulls[i] = true; - values[i++] = (Datum) 0; - } + HeapTupleSetFieldNull(pg_statistic, stanumbers1 + k, nulls); } - i = Anum_pg_statistic_stavalues1 - 1; + for (k = 0; k < STATISTIC_NUM_SLOTS; k++) { if (stats->stavalues[k] != NULL) @@ -1742,12 +1726,11 @@ update_attstats(Oid relid, bool inh, int natts, VacAttrStats **vacattrstats) stats->statyplen[k], stats->statypbyval[k], stats->statypalign[k]); - values[i++] = PointerGetDatum(arry); /* stavaluesN */ + HeapTupleSetValue(pg_statistic, stavalues1 + k, PointerGetDatum(arry), values); } else { - nulls[i] = true; - values[i++] = (Datum) 0; + HeapTupleSetFieldNull(pg_statistic, stavalues1 + k, nulls); } } @@ -1764,22 +1747,23 @@ update_attstats(Oid relid, bool inh, int natts, VacAttrStats **vacattrstats) if (HeapTupleIsValid(oldtup)) { /* Yes, replace it */ - stup = heap_modify_tuple(oldtup, + stup = heap_update_tuple(oldtup, RelationGetDescr(sd), values, nulls, - replaces); + updated); ReleaseSysCache(oldtup); - CatalogTupleUpdateWithInfo(sd, &stup->t_self, stup, indstate); + CatalogTupleUpdate(sd, &stup->t_self, stup, updated, indstate); } else { /* No, insert new tuple */ stup = heap_form_tuple(RelationGetDescr(sd), values, nulls); - CatalogTupleInsertWithInfo(sd, stup, indstate); + CatalogTupleInsert(sd, stup, NULL); } heap_freetuple(stup); + bms_free(updated); } if (indstate != NULL) diff --git a/src/backend/statistics/attribute_stats.c b/src/backend/statistics/attribute_stats.c index ef4d768feab..817db4cdefb 100644 --- a/src/backend/statistics/attribute_stats.c +++ b/src/backend/statistics/attribute_stats.c @@ -18,6 +18,7 @@ #include "postgres.h" #include "access/heapam.h" +#include "access/htup.h" #include "catalog/indexing.h" #include "catalog/namespace.h" #include "catalog/pg_collation.h" @@ -120,15 +121,15 @@ static bool get_elem_stat_type(Oid atttypid, char atttyptype, Oid *elemtypid, Oid *elem_eq_opr); static Datum text_to_stavalues(const char *staname, FmgrInfo *array_in, Datum d, Oid typid, int32 typmod, bool *ok); -static void set_stats_slot(Datum *values, bool *nulls, bool *replaces, - int16 stakind, Oid staop, Oid stacoll, - Datum stanumbers, bool stanumbers_isnull, - Datum stavalues, bool stavalues_isnull); +static Bitmapset *set_stats_slot(Datum *values, bool *nulls, Bitmapset *updated, + int16 stakind, Oid staop, Oid stacoll, + Datum stanumbers, bool stanumbers_isnull, + Datum stavalues, bool stavalues_isnull); static void upsert_pg_statistic(Relation starel, HeapTuple oldtup, - const Datum *values, const bool *nulls, const bool *replaces); + const Datum *values, const bool *nulls, const Bitmapset *updated); static bool delete_pg_statistic(Oid reloid, AttrNumber attnum, bool stainherit); -static void init_empty_stats_tuple(Oid reloid, int16 attnum, bool inherited, - Datum *values, bool *nulls, bool *replaces); +static Bitmapset *init_empty_stats_tuple(Oid reloid, int16 attnum, bool inherited, + Datum *values, bool *nulls, Bitmapset *updated); /* * Insert or Update Attribute Statistics @@ -184,9 +185,8 @@ attribute_statistics_update(FunctionCallInfo fcinfo) !PG_ARGISNULL(RANGE_EMPTY_FRAC_ARG); Datum values[Natts_pg_statistic] = {0}; - bool nulls[Natts_pg_statistic] = {0}; - bool replaces[Natts_pg_statistic] = {0}; - + bool nulls[Natts_pg_statistic] = {false}; + Bitmapset *updated = NULL; bool result = true; stats_check_required_arg(fcinfo, attarginfo, ATTRELSCHEMA_ARG); @@ -361,25 +361,17 @@ attribute_statistics_update(FunctionCallInfo fcinfo) if (HeapTupleIsValid(statup)) heap_deform_tuple(statup, RelationGetDescr(starel), values, nulls); else - init_empty_stats_tuple(reloid, attnum, inherited, values, nulls, - replaces); + updated = init_empty_stats_tuple(reloid, attnum, inherited, values, nulls, updated); /* if specified, set to argument values */ if (!PG_ARGISNULL(NULL_FRAC_ARG)) - { - values[Anum_pg_statistic_stanullfrac - 1] = PG_GETARG_DATUM(NULL_FRAC_ARG); - replaces[Anum_pg_statistic_stanullfrac - 1] = true; - } + HeapTupleUpdateValue(pg_statistic, stanullfrac, PG_GETARG_DATUM(NULL_FRAC_ARG), values, nulls, updated); + if (!PG_ARGISNULL(AVG_WIDTH_ARG)) - { - values[Anum_pg_statistic_stawidth - 1] = PG_GETARG_DATUM(AVG_WIDTH_ARG); - replaces[Anum_pg_statistic_stawidth - 1] = true; - } + HeapTupleUpdateValue(pg_statistic, stawidth, PG_GETARG_DATUM(AVG_WIDTH_ARG), values, nulls, updated); + if (!PG_ARGISNULL(N_DISTINCT_ARG)) - { - values[Anum_pg_statistic_stadistinct - 1] = PG_GETARG_DATUM(N_DISTINCT_ARG); - replaces[Anum_pg_statistic_stadistinct - 1] = true; - } + HeapTupleUpdateValue(pg_statistic, stadistinct, PG_GETARG_DATUM(N_DISTINCT_ARG), values, nulls, updated); /* STATISTIC_KIND_MCV */ if (do_mcv) @@ -394,10 +386,10 @@ attribute_statistics_update(FunctionCallInfo fcinfo) if (converted) { - set_stats_slot(values, nulls, replaces, - STATISTIC_KIND_MCV, - eq_opr, atttypcoll, - stanumbers, false, stavalues, false); + updated = set_stats_slot(values, nulls, updated, + STATISTIC_KIND_MCV, + eq_opr, atttypcoll, + stanumbers, false, stavalues, false); } else result = false; @@ -417,10 +409,10 @@ attribute_statistics_update(FunctionCallInfo fcinfo) if (converted) { - set_stats_slot(values, nulls, replaces, - STATISTIC_KIND_HISTOGRAM, - lt_opr, atttypcoll, - 0, true, stavalues, false); + updated = set_stats_slot(values, nulls, updated, + STATISTIC_KIND_HISTOGRAM, + lt_opr, atttypcoll, + 0, true, stavalues, false); } else result = false; @@ -433,10 +425,10 @@ attribute_statistics_update(FunctionCallInfo fcinfo) ArrayType *arry = construct_array_builtin(elems, 1, FLOAT4OID); Datum stanumbers = PointerGetDatum(arry); - set_stats_slot(values, nulls, replaces, - STATISTIC_KIND_CORRELATION, - lt_opr, atttypcoll, - stanumbers, false, 0, true); + updated = set_stats_slot(values, nulls, updated, + STATISTIC_KIND_CORRELATION, + lt_opr, atttypcoll, + stanumbers, false, 0, true); } /* STATISTIC_KIND_MCELEM */ @@ -454,10 +446,10 @@ attribute_statistics_update(FunctionCallInfo fcinfo) if (converted) { - set_stats_slot(values, nulls, replaces, - STATISTIC_KIND_MCELEM, - elem_eq_opr, atttypcoll, - stanumbers, false, stavalues, false); + updated = set_stats_slot(values, nulls, updated, + STATISTIC_KIND_MCELEM, + elem_eq_opr, atttypcoll, + stanumbers, false, stavalues, false); } else result = false; @@ -468,10 +460,10 @@ attribute_statistics_update(FunctionCallInfo fcinfo) { Datum stanumbers = PG_GETARG_DATUM(ELEM_COUNT_HISTOGRAM_ARG); - set_stats_slot(values, nulls, replaces, - STATISTIC_KIND_DECHIST, - elem_eq_opr, atttypcoll, - stanumbers, false, 0, true); + updated = set_stats_slot(values, nulls, updated, + STATISTIC_KIND_DECHIST, + elem_eq_opr, atttypcoll, + stanumbers, false, 0, true); } /* @@ -494,10 +486,10 @@ attribute_statistics_update(FunctionCallInfo fcinfo) if (converted) { - set_stats_slot(values, nulls, replaces, - STATISTIC_KIND_BOUNDS_HISTOGRAM, - InvalidOid, InvalidOid, - 0, true, stavalues, false); + updated = set_stats_slot(values, nulls, updated, + STATISTIC_KIND_BOUNDS_HISTOGRAM, + InvalidOid, InvalidOid, + 0, true, stavalues, false); } else result = false; @@ -521,20 +513,21 @@ attribute_statistics_update(FunctionCallInfo fcinfo) if (converted) { - set_stats_slot(values, nulls, replaces, - STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM, - Float8LessOperator, InvalidOid, - stanumbers, false, stavalues, false); + updated = set_stats_slot(values, nulls, updated, + STATISTIC_KIND_RANGE_LENGTH_HISTOGRAM, + Float8LessOperator, InvalidOid, + stanumbers, false, stavalues, false); } else result = false; } - upsert_pg_statistic(starel, statup, values, nulls, replaces); + upsert_pg_statistic(starel, statup, values, nulls, updated); if (HeapTupleIsValid(statup)) ReleaseSysCache(statup); table_close(starel, RowExclusiveLock); + bms_free(updated); return result; } @@ -759,69 +752,63 @@ text_to_stavalues(const char *staname, FmgrInfo *array_in, Datum d, Oid typid, * Find and update the slot with the given stakind, or use the first empty * slot. */ -static void -set_stats_slot(Datum *values, bool *nulls, bool *replaces, +static Bitmapset * +set_stats_slot(Datum *values, bool *nulls, Bitmapset *updated, int16 stakind, Oid staop, Oid stacoll, Datum stanumbers, bool stanumbers_isnull, Datum stavalues, bool stavalues_isnull) { - int slotidx; + int i = 0; int first_empty = -1; - AttrNumber stakind_attnum; - AttrNumber staop_attnum; - AttrNumber stacoll_attnum; - /* find existing slot with given stakind */ - for (slotidx = 0; slotidx < STATISTIC_NUM_SLOTS; slotidx++) + /* + * NOTE: This might seem odd, to be adding 'i' to the name of the field on + * these macros, but that's what we need to do here to find the proper + * slot offset and record the proper value into the updated bitmap. + * + * Example: HeapTupleValue(pg_statistic, stakind1 + i, values); + * + * Becomes: values[Anum_pg_statistic_stakind1 + i - 1]; + * + * The result is you're indexing the i'th value, exactly what we needed. + */ + + /* Find existing slot with given stakind */ + for (i = 0; i < STATISTIC_NUM_SLOTS; i++) { - stakind_attnum = Anum_pg_statistic_stakind1 - 1 + slotidx; + Datum d = HeapTupleValue(pg_statistic, stakind1 + i, values); + int16 v = DatumGetInt16(d); + + if (first_empty < 0 && v == 0) + first_empty = i; - if (first_empty < 0 && - DatumGetInt16(values[stakind_attnum]) == 0) - first_empty = slotidx; - if (DatumGetInt16(values[stakind_attnum]) == stakind) + if (v == stakind) break; } - if (slotidx >= STATISTIC_NUM_SLOTS && first_empty >= 0) - slotidx = first_empty; + if (i >= STATISTIC_NUM_SLOTS && first_empty >= 0) + i = first_empty; - if (slotidx >= STATISTIC_NUM_SLOTS) + if (i >= STATISTIC_NUM_SLOTS) ereport(ERROR, - (errmsg("maximum number of statistics slots exceeded: %d", - slotidx + 1))); + (errmsg("maximum number of statistics slots exceeded: %d", i + 1))); - stakind_attnum = Anum_pg_statistic_stakind1 - 1 + slotidx; - staop_attnum = Anum_pg_statistic_staop1 - 1 + slotidx; - stacoll_attnum = Anum_pg_statistic_stacoll1 - 1 + slotidx; + if (DatumGetInt16(HeapTupleValue(pg_statistic, stakind1 + i, values)) != stakind) + HeapTupleUpdateValue(pg_statistic, stakind1 + i, Int16GetDatum(stakind), values, nulls, updated); + + if (DatumGetInt16(HeapTupleValue(pg_statistic, staop1 + i, values)) != staop) + HeapTupleUpdateValue(pg_statistic, staop1 + i, ObjectIdGetDatum(staop), values, nulls, updated); + + if (DatumGetInt16(HeapTupleValue(pg_statistic, stacoll1 + i, values + i)) != stacoll) + HeapTupleUpdateValue(pg_statistic, stacoll1 + i, ObjectIdGetDatum(stacoll), values, nulls, updated); - if (DatumGetInt16(values[stakind_attnum]) != stakind) - { - values[stakind_attnum] = Int16GetDatum(stakind); - replaces[stakind_attnum] = true; - } - if (DatumGetObjectId(values[staop_attnum]) != staop) - { - values[staop_attnum] = ObjectIdGetDatum(staop); - replaces[staop_attnum] = true; - } - if (DatumGetObjectId(values[stacoll_attnum]) != stacoll) - { - values[stacoll_attnum] = ObjectIdGetDatum(stacoll); - replaces[stacoll_attnum] = true; - } if (!stanumbers_isnull) - { - values[Anum_pg_statistic_stanumbers1 - 1 + slotidx] = stanumbers; - nulls[Anum_pg_statistic_stanumbers1 - 1 + slotidx] = false; - replaces[Anum_pg_statistic_stanumbers1 - 1 + slotidx] = true; - } + HeapTupleUpdateValue(pg_statistic, stanumbers1 + i, stanumbers, values, nulls, updated); + if (!stavalues_isnull) - { - values[Anum_pg_statistic_stavalues1 - 1 + slotidx] = stavalues; - nulls[Anum_pg_statistic_stavalues1 - 1 + slotidx] = false; - replaces[Anum_pg_statistic_stavalues1 - 1 + slotidx] = true; - } + HeapTupleUpdateValue(pg_statistic, stavalues1 + i, stavalues, values, nulls, updated); + + return updated; } /* @@ -829,20 +816,21 @@ set_stats_slot(Datum *values, bool *nulls, bool *replaces, */ static void upsert_pg_statistic(Relation starel, HeapTuple oldtup, - const Datum *values, const bool *nulls, const bool *replaces) + const Datum *values, const bool *nulls, const Bitmapset *updated) { HeapTuple newtup; if (HeapTupleIsValid(oldtup)) { - newtup = heap_modify_tuple(oldtup, RelationGetDescr(starel), - values, nulls, replaces); - CatalogTupleUpdate(starel, &newtup->t_self, newtup); + TupleDesc tupdesc = RelationGetDescr(starel); + + newtup = heap_update_tuple(oldtup, tupdesc, values, nulls, updated); + CatalogTupleUpdate(starel, &newtup->t_self, newtup, updated, NULL); } else { newtup = heap_form_tuple(RelationGetDescr(starel), values, nulls); - CatalogTupleInsert(starel, newtup); + CatalogTupleInsert(starel, newtup, NULL); } heap_freetuple(newtup); @@ -883,39 +871,38 @@ delete_pg_statistic(Oid reloid, AttrNumber attnum, bool stainherit) /* * Initialize values and nulls for a new stats tuple. */ -static void +static Bitmapset * init_empty_stats_tuple(Oid reloid, int16 attnum, bool inherited, - Datum *values, bool *nulls, bool *replaces) + Datum *values, bool *nulls, Bitmapset *updated) { + /* + * NOTE: It is customary to initialize the "nulls" array to all false (0) + * and later, when some field transitions from a value to NULL to mark + * that transition using the proper macro which will set the array + * position to true (1). Here we start with the assumption that all + * values will be set to NULL. + */ + HeapTupleUpdateSetAllColumnsUpdated(pg_statistic, updated); memset(nulls, true, sizeof(bool) * Natts_pg_statistic); - memset(replaces, true, sizeof(bool) * Natts_pg_statistic); /* must initialize non-NULL attributes */ + HeapTupleUpdateValue(pg_statistic, starelid, ObjectIdGetDatum(reloid), values, nulls, updated); + HeapTupleUpdateValue(pg_statistic, staattnum, Int16GetDatum(attnum), values, nulls, updated); + HeapTupleUpdateValue(pg_statistic, stainherit, BoolGetDatum(inherited), values, nulls, updated); - values[Anum_pg_statistic_starelid - 1] = ObjectIdGetDatum(reloid); - nulls[Anum_pg_statistic_starelid - 1] = false; - values[Anum_pg_statistic_staattnum - 1] = Int16GetDatum(attnum); - nulls[Anum_pg_statistic_staattnum - 1] = false; - values[Anum_pg_statistic_stainherit - 1] = BoolGetDatum(inherited); - nulls[Anum_pg_statistic_stainherit - 1] = false; - - values[Anum_pg_statistic_stanullfrac - 1] = DEFAULT_NULL_FRAC; - nulls[Anum_pg_statistic_stanullfrac - 1] = false; - values[Anum_pg_statistic_stawidth - 1] = DEFAULT_AVG_WIDTH; - nulls[Anum_pg_statistic_stawidth - 1] = false; - values[Anum_pg_statistic_stadistinct - 1] = DEFAULT_N_DISTINCT; - nulls[Anum_pg_statistic_stadistinct - 1] = false; + HeapTupleUpdateValue(pg_statistic, stanullfrac, DEFAULT_NULL_FRAC, values, nulls, updated); + HeapTupleUpdateValue(pg_statistic, stawidth, DEFAULT_AVG_WIDTH, values, nulls, updated); + HeapTupleUpdateValue(pg_statistic, stadistinct, DEFAULT_N_DISTINCT, values, nulls, updated); /* initialize stakind, staop, and stacoll slots */ - for (int slotnum = 0; slotnum < STATISTIC_NUM_SLOTS; slotnum++) + for (int i = 0; i < STATISTIC_NUM_SLOTS; i++) { - values[Anum_pg_statistic_stakind1 + slotnum - 1] = (Datum) 0; - nulls[Anum_pg_statistic_stakind1 + slotnum - 1] = false; - values[Anum_pg_statistic_staop1 + slotnum - 1] = ObjectIdGetDatum(InvalidOid); - nulls[Anum_pg_statistic_staop1 + slotnum - 1] = false; - values[Anum_pg_statistic_stacoll1 + slotnum - 1] = ObjectIdGetDatum(InvalidOid); - nulls[Anum_pg_statistic_stacoll1 + slotnum - 1] = false; + HeapTupleUpdateValue(pg_statistic, stakind1 + i, (Datum) 0, values, nulls, updated); + HeapTupleUpdateValue(pg_statistic, staop1 + i, ObjectIdGetDatum(InvalidOid), values, nulls, updated); + HeapTupleUpdateValue(pg_statistic, stacoll1 + i, ObjectIdGetDatum(InvalidOid), values, nulls, updated); } + + return updated; } /* diff --git a/src/include/access/htup.h b/src/include/access/htup.h index f6b766697e2..e67bb718efa 100644 --- a/src/include/access/htup.h +++ b/src/include/access/htup.h @@ -14,6 +14,7 @@ #ifndef HTUP_H #define HTUP_H +#include "catalog/pg_aggregate_d.h" #include "storage/itemptr.h" /* typedefs and forward declarations for structs defined in htup_details.h */ @@ -77,6 +78,66 @@ typedef HeapTupleData *HeapTuple; */ #define HeapTupleIsValid(tuple) ((tuple) != NULL) +#define HeapTupleValue(table_name, field, values) \ + (values)[Anum_##table_name##_##field - 1] + +/* + * These are useful when forming tuples for CatalogTupleInsert() + */ + +#define HeapTupleSetField(table_name, field, value, form_ptr) \ + (form_ptr)->field = (value) + +#define HeapTupleSetFieldNull(table_name, field, nulls) \ + (nulls)[Anum_##table_name##_##field - 1] = true + +#define HeapTupleSetValue(table_name, field, value, values) \ + (values)[Anum_##table_name##_##field - 1] = (value) + +#define HeapTupleSetValueNull(table_name, field, values, nulls) \ + do { \ + (values)[Anum_##table_name##_##field - 1] = (Datum) 0; \ + (nulls)[Anum_##table_name##_##field - 1] = true; \ + } while(0) + +/* + * These are useful when forming tuples for CatalogTupleUpdate() + * + * Updated catalog tuples need to track which fields were changed when + * calling heap_update_tuple(), so we use a bitmap to keep track of that. + */ +#define HeapTupleMarkColumnUpdated(table_name, field, updated) \ + (updated) = bms_add_member((updated), \ + Anum_##table_name##_##field - FirstLowInvalidHeapAttributeNumber) + +#define HeapTupleUpdateSetAllColumnsUpdated(table_name, updated) \ + (updated) = bms_add_range((updated), 1 - FirstLowInvalidHeapAttributeNumber, \ + Natts_##table_name - FirstLowInvalidHeapAttributeNumber) + +#define HeapTupleSetColumnNotUpdated(table_name, field, updated) \ + (updated) = bms_del_member((updated), \ + Anum_##table_name##_##field - FirstLowInvalidHeapAttributeNumber) + +#define HeapTupleUpdateField(table_name, field, value, form_ptr, updated) \ + do { \ + (form_ptr)->field = (value); \ + HeapTupleMarkColumnUpdated(table_name, field, updated); \ + } while(0) + +#define HeapTupleUpdateValue(table_name, field, value, values, nulls, updated) \ + do { \ + (values)[Anum_##table_name##_##field - 1] = (Datum) (value); \ + (nulls)[Anum_##table_name##_##field - 1] = false; \ + HeapTupleMarkColumnUpdated(table_name, field, updated); \ + } while(0) + +#define HeapTupleUpdateValueNull(table_name, field, values, nulls, updated) \ + do { \ + (values)[Anum_##table_name##_##field - 1] = (Datum) 0; \ + (nulls)[Anum_##table_name##_##field - 1] = true; \ + HeapTupleMarkColumnUpdated(table_name, field, updated); \ + } while(0) + /* HeapTupleHeader functions implemented in utils/time/combocid.c */ extern CommandId HeapTupleHeaderGetCmin(const HeapTupleHeaderData *tup); extern CommandId HeapTupleHeaderGetCmax(const HeapTupleHeaderData *tup); diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h index f3593acc8c2..814d764ee97 100644 --- a/src/include/access/htup_details.h +++ b/src/include/access/htup_details.h @@ -21,6 +21,9 @@ #include "storage/bufpage.h" #include "varatt.h" +/* Forward declarations */ +typedef struct Bitmapset Bitmapset; + /* * MaxTupleAttributeNumber limits the number of (user) columns in a tuple. * The key limit on this value is that the size of the fixed overhead for @@ -835,6 +838,11 @@ extern HeapTuple heap_modify_tuple_by_cols(HeapTuple tuple, const int *replCols, const Datum *replValues, const bool *replIsnull); +extern HeapTuple heap_update_tuple(HeapTuple tuple, + TupleDesc tupleDesc, + const Datum *replValues, + const bool *replIsnull, + const Bitmapset *replaces); extern void heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc, Datum *values, bool *isnull); extern void heap_freetuple(HeapTuple htup); diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h index 77c17d3fb7a..685ff3d2bd1 100644 --- a/src/include/catalog/indexing.h +++ b/src/include/catalog/indexing.h @@ -37,18 +37,13 @@ typedef struct ResultRelInfo *CatalogIndexState; */ extern CatalogIndexState CatalogOpenIndexes(Relation heapRel); extern void CatalogCloseIndexes(CatalogIndexState indstate); -extern void CatalogTupleInsert(Relation heapRel, HeapTuple tup); -extern void CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup, - CatalogIndexState indstate); -extern void CatalogTuplesMultiInsertWithInfo(Relation heapRel, - TupleTableSlot **slot, - int ntuples, - CatalogIndexState indstate); +extern void CatalogTupleInsert(Relation heapRel, HeapTuple tup, + CatalogIndexState indstate); +extern void CatalogTuplesMultiInsert(Relation heapRel, TupleTableSlot **slot, + int ntuples, CatalogIndexState indstate); extern void CatalogTupleUpdate(Relation heapRel, const ItemPointerData *otid, - HeapTuple tup); -extern void CatalogTupleUpdateWithInfo(Relation heapRel, - const ItemPointerData *otid, HeapTuple tup, - CatalogIndexState indstate); + HeapTuple tuple, const struct Bitmapset *updated, + CatalogIndexState indstate); extern void CatalogTupleDelete(Relation heapRel, const ItemPointerData *tid); #endif /* INDEXING_H */ -- 2.49.0