From: Andres Freund Date: Wed, 27 Feb 2019 02:21:44 +0000 (-0800) Subject: Store table oid and tuple's tid in tuple slots directly. X-Git-Tag: REL_12_BETA1~666 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b8d71745eac0a12740a70dc78cbcdedadade37f8;p=postgresql Store table oid and tuple's tid in tuple slots directly. After the introduction of tuple table slots all table AMs need to support returning the table oid of the tuple stored in a slot created by said AM. It does not make sense to re-implement that in every AM, therefore move handling of table OIDs into the TupleTableSlot structure itself. It's possible that we, at a later date, might want to get rid of HeapTupleData.t_tableOid entirely, but doing so before the abstractions for table AMs are integrated turns out to be too hard, so delay that for now. Similarly, every AM needs to support the concept of a tuple identifier (tid / item pointer) for its tuples. It's quite possible that we'll generalize the exact form of a tid at a future point (to allow for things like index organized tables), but for now many parts of the code know about tids, so there's not much point in abstracting tids away. Therefore also move into slot (rather than providing API to set/get the tid associated with the tuple in a slot). Once table AM includes insert/updating/deleting tuples, the responsibility to set the correct tid after such an action will move into that. After that change, code doing such modifications, should not have to deal with HeapTuples directly anymore. Author: Andres Freund, Haribabu Kommi and Ashutosh Bapat Discussion: https://postgr.es/m/20180703070645.wchpu5muyto5n647@alap3.anarazel.de --- diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index dbb06397e6..93aa163177 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -2710,10 +2710,10 @@ CopyFrom(CopyState cstate) tuple = heap_form_tuple(tupDesc, values, nulls); /* - * Constraints might reference the tableoid column, so initialize - * t_tableOid before evaluating them. + * Constraints might reference the tableoid column, so (re-)initialize + * tts_tableOid before evaluating them. */ - tuple->t_tableOid = RelationGetRelid(target_resultRelInfo->ri_RelationDesc); + myslot->tts_tableOid = RelationGetRelid(target_resultRelInfo->ri_RelationDesc); /* Triggers and stuff need to be invoked in query context. */ MemoryContextSwitchTo(oldcontext); @@ -2899,7 +2899,7 @@ CopyFrom(CopyState cstate) MemoryContextSwitchTo(oldcontext); } - tuple->t_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); + slot->tts_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); } skip_tuple = false; @@ -2995,14 +2995,18 @@ CopyFrom(CopyState cstate) /* * AFTER ROW Triggers might reference the tableoid - * column, so initialize t_tableOid before evaluating - * them. + * column, so (re-)initialize tts_tableOid before + * evaluating them. */ - tuple->t_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); + slot->tts_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); } else + { heap_insert(resultRelInfo->ri_RelationDesc, tuple, mycid, hi_options, bistate); + ItemPointerCopy(&tuple->t_self, &slot->tts_tid); + slot->tts_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); + } /* And create index entries for it */ if (resultRelInfo->ri_NumIndices > 0) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 35bdb0e0c6..0fb0b186bb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4860,7 +4860,10 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) /* Write the tuple out to the new relation */ if (newrel) + { heap_insert(newrel, tuple, mycid, hi_options, bistate); + ItemPointerCopy(&tuple->t_self, &newslot->tts_tid); + } ResetExprContext(econtext); diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index 589573b879..663d6e3264 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -424,6 +424,7 @@ ExecSimpleRelationInsert(EState *estate, TupleTableSlot *slot) /* OK, store the tuple and create index entries for it */ simple_heap_insert(rel, tuple); + ItemPointerCopy(&tuple->t_self, &slot->tts_tid); if (resultRelInfo->ri_NumIndices > 0) recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self), @@ -496,6 +497,7 @@ ExecSimpleRelationUpdate(EState *estate, EPQState *epqstate, /* OK, update the tuple and index entries for it */ simple_heap_update(rel, &hsearchslot->tuple->t_self, hslot->tuple); + ItemPointerCopy(&hslot->tuple->t_self, &slot->tts_tid); if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(hslot->tuple)) diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 757e7997fe..e04c048b54 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -436,6 +436,7 @@ tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree) hslot->tuple = tuple; hslot->off = 0; slot->tts_flags &= ~TTS_FLAG_EMPTY; + slot->tts_tid = tuple->t_self; if (shouldFree) slot->tts_flags |= TTS_FLAG_SHOULDFREE; @@ -817,6 +818,7 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, slot->tts_nvalid = 0; bslot->base.tuple = tuple; bslot->base.off = 0; + slot->tts_tid = tuple->t_self; /* * If tuple is on a disk page, keep the page pinned as long as we hold a @@ -1304,6 +1306,8 @@ ExecStoreHeapTuple(HeapTuple tuple, elog(ERROR, "trying to store a heap tuple into wrong type of slot"); tts_heap_store_tuple(slot, tuple, shouldFree); + slot->tts_tableOid = tuple->t_tableOid; + return slot; } @@ -1343,6 +1347,7 @@ ExecStoreBufferHeapTuple(HeapTuple tuple, elog(ERROR, "trying to store an on-disk heap tuple into wrong type of slot"); tts_buffer_heap_store_tuple(slot, tuple, buffer, false); + slot->tts_tableOid = tuple->t_tableOid; return slot; } @@ -1368,6 +1373,8 @@ ExecStorePinnedBufferHeapTuple(HeapTuple tuple, elog(ERROR, "trying to store an on-disk heap tuple into wrong type of slot"); tts_buffer_heap_store_tuple(slot, tuple, buffer, true); + slot->tts_tableOid = tuple->t_tableOid; + return slot; } diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c index 40b6fbe891..52af1dac5c 100644 --- a/src/backend/executor/nodeForeignscan.c +++ b/src/backend/executor/nodeForeignscan.c @@ -55,17 +55,11 @@ ForeignNext(ForeignScanState *node) MemoryContextSwitchTo(oldcontext); /* - * If any system columns are requested, we have to force the tuple into - * physical-tuple form to avoid "cannot extract system attribute from - * virtual tuple" errors later. We also insert a valid value for - * tableoid, which is the only actually-useful system column. + * Insert valid value into tableoid, the only actually-useful system + * column. */ if (plan->fsSystemCol && !TupIsNull(slot)) - { - HeapTuple tup = ExecFetchSlotHeapTuple(slot, true, NULL); - - tup->t_tableOid = RelationGetRelid(node->ss.ss_currentRelation); - } + slot->tts_tableOid = RelationGetRelid(node->ss.ss_currentRelation); return slot; } diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 566858c19b..fe62da06ea 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -166,20 +166,15 @@ ExecProcessReturning(ResultRelInfo *resultRelInfo, /* Make tuple and any needed join variables available to ExecProject */ if (tupleSlot) econtext->ecxt_scantuple = tupleSlot; - else - { - HeapTuple tuple; - - /* - * RETURNING expressions might reference the tableoid column, so - * initialize t_tableOid before evaluating them. - */ - Assert(!TupIsNull(econtext->ecxt_scantuple)); - tuple = ExecFetchSlotHeapTuple(econtext->ecxt_scantuple, true, NULL); - tuple->t_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); - } econtext->ecxt_outertuple = planSlot; + /* + * RETURNING expressions might reference the tableoid column, so + * reinitialize tts_tableOid before evaluating them. + */ + econtext->ecxt_scantuple->tts_tableOid = + RelationGetRelid(resultRelInfo->ri_RelationDesc); + /* Compute the RETURNING expressions */ return ExecProject(projectReturning); } @@ -332,19 +327,21 @@ ExecInsert(ModifyTableState *mtstate, /* * AFTER ROW Triggers or RETURNING expressions might reference the - * tableoid column, so initialize t_tableOid before evaluating them. + * tableoid column, so (re-)initialize tts_tableOid before evaluating + * them. */ - tuple->t_tableOid = RelationGetRelid(resultRelationDesc); + slot->tts_tableOid = RelationGetRelid(resultRelInfo->ri_RelationDesc); + } else { WCOKind wco_kind; /* - * Constraints might reference the tableoid column, so initialize - * t_tableOid before evaluating them. + * Constraints might reference the tableoid column, so (re-)initialize + * tts_tableOid before evaluating them. */ - tuple->t_tableOid = RelationGetRelid(resultRelationDesc); + slot->tts_tableOid = RelationGetRelid(resultRelationDesc); /* * Check any RLS WITH CHECK policies. @@ -458,6 +455,8 @@ ExecInsert(ModifyTableState *mtstate, estate->es_output_cid, HEAP_INSERT_SPECULATIVE, NULL); + slot->tts_tableOid = RelationGetRelid(resultRelationDesc); + ItemPointerCopy(&tuple->t_self, &slot->tts_tid); /* insert index entries for tuple */ recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self), @@ -503,6 +502,8 @@ ExecInsert(ModifyTableState *mtstate, heap_insert(resultRelationDesc, tuple, estate->es_output_cid, 0, NULL); + slot->tts_tableOid = RelationGetRelid(resultRelationDesc); + ItemPointerCopy(&tuple->t_self, &slot->tts_tid); /* insert index entries for tuple */ if (resultRelInfo->ri_NumIndices > 0) @@ -515,7 +516,7 @@ ExecInsert(ModifyTableState *mtstate, if (canSetTag) { (estate->es_processed)++; - setLastTid(&(tuple->t_self)); + setLastTid(&slot->tts_tid); } /* @@ -647,8 +648,6 @@ ExecDelete(ModifyTableState *mtstate, } else if (resultRelInfo->ri_FdwRoutine) { - HeapTuple tuple; - /* * delete from foreign table: let the FDW do it * @@ -670,12 +669,11 @@ ExecDelete(ModifyTableState *mtstate, /* * RETURNING expressions might reference the tableoid column, so - * initialize t_tableOid before evaluating them. + * (re)initialize tts_tableOid before evaluating them. */ if (TTS_EMPTY(slot)) ExecStoreAllNullTuple(slot); - tuple = ExecFetchSlotHeapTuple(slot, true, NULL); - tuple->t_tableOid = RelationGetRelid(resultRelationDesc); + slot->tts_tableOid = RelationGetRelid(resultRelationDesc); } else { @@ -985,9 +983,10 @@ ExecUpdate(ModifyTableState *mtstate, /* * AFTER ROW Triggers or RETURNING expressions might reference the - * tableoid column, so initialize t_tableOid before evaluating them. + * tableoid column, so (re-)initialize tts_tableOid before evaluating + * them. */ - tuple->t_tableOid = RelationGetRelid(resultRelationDesc); + slot->tts_tableOid = RelationGetRelid(resultRelationDesc); } else { @@ -995,10 +994,10 @@ ExecUpdate(ModifyTableState *mtstate, bool partition_constraint_failed; /* - * Constraints might reference the tableoid column, so initialize - * t_tableOid before evaluating them. + * Constraints might reference the tableoid column, so (re-)initialize + * tts_tableOid before evaluating them. */ - tuple->t_tableOid = RelationGetRelid(resultRelationDesc); + slot->tts_tableOid = RelationGetRelid(resultRelationDesc); /* * Check any RLS UPDATE WITH CHECK policies @@ -1184,6 +1183,8 @@ lreplace:; estate->es_crosscheck_snapshot, true /* wait for commit */ , &hufd, &lockmode); + ItemPointerCopy(&tuple->t_self, &slot->tts_tid); + switch (result) { case HeapTupleSelfUpdated: diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h index 1e94beafac..834786620a 100644 --- a/src/include/executor/tuptable.h +++ b/src/include/executor/tuptable.h @@ -15,6 +15,7 @@ #define TUPTABLE_H #include "access/htup.h" +#include "access/sysattr.h" #include "access/tupdesc.h" #include "access/htup_details.h" #include "storage/buf.h" @@ -126,6 +127,8 @@ typedef struct TupleTableSlot #define FIELDNO_TUPLETABLESLOT_ISNULL 6 bool *tts_isnull; /* current per-attribute isnull flags */ MemoryContext tts_mcxt; /* slot itself is in this context */ + ItemPointerData tts_tid; /* stored tuple's tid */ + Oid tts_tableOid; /* table oid of tuple */ } TupleTableSlot; /* routines for a TupleTableSlot implementation */ @@ -398,6 +401,17 @@ slot_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull) { AssertArg(attnum < 0); /* caller error */ + if (attnum == TableOidAttributeNumber) + { + *isnull = false; + return ObjectIdGetDatum(slot->tts_tableOid); + } + else if (attnum == SelfItemPointerAttributeNumber) + { + *isnull = false; + return PointerGetDatum(&slot->tts_tid); + } + /* Fetch the system attribute from the underlying tuple. */ return slot->tts_ops->getsysattr(slot, attnum, isnull); }