From da841aa4dc279bb0053de56121c927ec943edff3 Mon Sep 17 00:00:00 2001 From: Alexander Korotkov Date: Thu, 11 Apr 2024 15:47:53 +0300 Subject: [PATCH] Revert: Let table AM insertion methods control index insertion This commit reverts b1484a3f19 per review by Andres Freund. Discussion: https://postgr.es/m/20240410165236.rwyrny7ihi4ddxw4%40awork3.anarazel.de --- src/backend/access/heap/heapam.c | 4 +--- src/backend/access/heap/heapam_handler.c | 4 +--- src/backend/access/table/tableam.c | 6 ++---- src/backend/catalog/indexing.c | 4 +--- src/backend/commands/copyfrom.c | 13 ++++-------- src/backend/commands/createas.c | 4 +--- src/backend/commands/matview.c | 4 +--- src/backend/commands/tablecmds.c | 22 +++++---------------- src/backend/executor/execReplication.c | 6 ++---- src/backend/executor/nodeModifyTable.c | 6 ++---- src/include/access/heapam.h | 2 +- src/include/access/tableam.h | 25 +++++++----------------- 12 files changed, 28 insertions(+), 72 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 08f6a10ac3..7f642edf45 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -2257,8 +2257,7 @@ heap_multi_insert_pages(HeapTuple *heaptuples, int done, int ntuples, Size saveF */ void heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, - CommandId cid, int options, BulkInsertState bistate, - bool *insert_indexes) + CommandId cid, int options, BulkInsertState bistate) { TransactionId xid = GetCurrentTransactionId(); HeapTuple *heaptuples; @@ -2607,7 +2606,6 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples, slots[i]->tts_tid = heaptuples[i]->t_self; pgstat_count_heap_insert(relation, ntuples); - *insert_indexes = true; } /* diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 2c423286d7..00d8270532 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -245,7 +245,7 @@ heapam_tuple_satisfies_snapshot(Relation rel, TupleTableSlot *slot, static TupleTableSlot * heapam_tuple_insert(Relation relation, TupleTableSlot *slot, CommandId cid, - int options, BulkInsertState bistate, bool *insert_indexes) + int options, BulkInsertState bistate) { bool shouldFree = true; HeapTuple tuple = ExecFetchSlotHeapTuple(slot, true, &shouldFree); @@ -261,8 +261,6 @@ heapam_tuple_insert(Relation relation, TupleTableSlot *slot, CommandId cid, if (shouldFree) pfree(tuple); - *insert_indexes = true; - return slot; } diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c index 805d222ceb..8d3675be95 100644 --- a/src/backend/access/table/tableam.c +++ b/src/backend/access/table/tableam.c @@ -273,11 +273,9 @@ table_tuple_get_latest_tid(TableScanDesc scan, ItemPointer tid) * default command ID and not allowing access to the speedup options. */ void -simple_table_tuple_insert(Relation rel, TupleTableSlot *slot, - bool *insert_indexes) +simple_table_tuple_insert(Relation rel, TupleTableSlot *slot) { - table_tuple_insert(rel, slot, GetCurrentCommandId(true), 0, NULL, - insert_indexes); + table_tuple_insert(rel, slot, GetCurrentCommandId(true), 0, NULL); } /* diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c index 4d404f22f8..d0d1abda58 100644 --- a/src/backend/catalog/indexing.c +++ b/src/backend/catalog/indexing.c @@ -273,14 +273,12 @@ void CatalogTuplesMultiInsertWithInfo(Relation heapRel, TupleTableSlot **slot, int ntuples, CatalogIndexState indstate) { - bool insertIndexes; - /* Nothing to do */ if (ntuples <= 0) return; heap_multi_insert(heapRel, slot, ntuples, - GetCurrentCommandId(true), 0, NULL, &insertIndexes); + GetCurrentCommandId(true), 0, NULL); /* * There is no equivalent to heap_multi_insert for the catalog indexes, so diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 9d2900041e..06bc14636d 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -395,7 +395,6 @@ CopyMultiInsertBufferFlush(CopyMultiInsertInfo *miinfo, bool line_buf_valid = cstate->line_buf_valid; uint64 save_cur_lineno = cstate->cur_lineno; MemoryContext oldcontext; - bool insertIndexes; Assert(buffer->bistate != NULL); @@ -415,8 +414,7 @@ CopyMultiInsertBufferFlush(CopyMultiInsertInfo *miinfo, nused, mycid, ti_options, - buffer->bistate, - &insertIndexes); + buffer->bistate); MemoryContextSwitchTo(oldcontext); for (i = 0; i < nused; i++) @@ -425,7 +423,7 @@ CopyMultiInsertBufferFlush(CopyMultiInsertInfo *miinfo, * If there are any indexes, update them for all the inserted * tuples, and run AFTER ROW INSERT triggers. */ - if (insertIndexes && resultRelInfo->ri_NumIndices > 0) + if (resultRelInfo->ri_NumIndices > 0) { List *recheckIndexes; @@ -1265,14 +1263,11 @@ CopyFrom(CopyFromState cstate) } else { - bool insertIndexes; - /* OK, store the tuple and create index entries for it */ table_tuple_insert(resultRelInfo->ri_RelationDesc, - myslot, mycid, ti_options, bistate, - &insertIndexes); + myslot, mycid, ti_options, bistate); - if (insertIndexes && resultRelInfo->ri_NumIndices > 0) + if (resultRelInfo->ri_NumIndices > 0) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, myslot, estate, diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index afd3dace07..62050f4dc5 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -578,7 +578,6 @@ static bool intorel_receive(TupleTableSlot *slot, DestReceiver *self) { DR_intorel *myState = (DR_intorel *) self; - bool insertIndexes; /* Nothing to insert if WITH NO DATA is specified. */ if (!myState->into->skipData) @@ -595,8 +594,7 @@ intorel_receive(TupleTableSlot *slot, DestReceiver *self) slot, myState->output_cid, myState->ti_options, - myState->bistate, - &insertIndexes); + myState->bistate); } /* We know this is a newly created relation, so there are no indexes */ diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 9ec13d0984..6d09b75556 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -476,7 +476,6 @@ static bool transientrel_receive(TupleTableSlot *slot, DestReceiver *self) { DR_transientrel *myState = (DR_transientrel *) self; - bool insertIndexes; /* * Note that the input slot might not be of the type of the target @@ -491,8 +490,7 @@ transientrel_receive(TupleTableSlot *slot, DestReceiver *self) slot, myState->output_cid, myState->ti_options, - myState->bistate, - &insertIndexes); + myState->bistate); /* We know this is a newly created relation, so there are no indexes */ diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index b3fcf6d3cd..571feea270 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -6391,12 +6391,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) /* Write the tuple out to the new relation */ if (newrel) - { - bool insertIndexes; - table_tuple_insert(newrel, insertslot, mycid, - ti_options, bistate, &insertIndexes); - } + ti_options, bistate); ResetExprContext(econtext); @@ -21037,7 +21033,6 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar while (table_scan_getnextslot(scan, ForwardScanDirection, srcslot)) { bool found = false; - bool insert_indexes; TupleTableSlot *insertslot; /* Extract data from old tuple. */ @@ -21090,12 +21085,9 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar ExecStoreVirtualTuple(insertslot); } - /* - * Write the tuple out to the new relation. We ignore the - * 'insert_indexes' flag since newPartRel has no indexes anyway. - */ + /* Write the tuple out to the new relation. */ (void) table_tuple_insert(pc->partRel, insertslot, mycid, - ti_options, pc->bistate, &insert_indexes); + ti_options, pc->bistate); ResetExprContext(econtext); @@ -21364,7 +21356,6 @@ moveMergedTablesRows(Relation rel, List *mergingPartitionsList, while (table_scan_getnextslot(scan, ForwardScanDirection, srcslot)) { TupleTableSlot *insertslot; - bool insert_indexes; /* Extract data from old tuple. */ slot_getallattrs(srcslot); @@ -21389,12 +21380,9 @@ moveMergedTablesRows(Relation rel, List *mergingPartitionsList, ExecStoreVirtualTuple(insertslot); } - /* - * Write the tuple out to the new relation. We ignore the - * 'insert_indexes' flag since newPartRel has no indexes anyway. - */ + /* Write the tuple out to the new relation. */ (void) table_tuple_insert(newPartRel, insertslot, mycid, - ti_options, bistate, &insert_indexes); + ti_options, bistate); CHECK_FOR_INTERRUPTS(); } diff --git a/src/backend/executor/execReplication.c b/src/backend/executor/execReplication.c index db685473fc..0cad843fb6 100644 --- a/src/backend/executor/execReplication.c +++ b/src/backend/executor/execReplication.c @@ -509,7 +509,6 @@ ExecSimpleRelationInsert(ResultRelInfo *resultRelInfo, if (!skip_tuple) { List *recheckIndexes = NIL; - bool insertIndexes; /* Compute stored generated columns */ if (rel->rd_att->constr && @@ -524,10 +523,9 @@ ExecSimpleRelationInsert(ResultRelInfo *resultRelInfo, ExecPartitionCheck(resultRelInfo, slot, estate, true); /* OK, store the tuple and create index entries for it */ - simple_table_tuple_insert(resultRelInfo->ri_RelationDesc, slot, - &insertIndexes); + simple_table_tuple_insert(resultRelInfo->ri_RelationDesc, slot); - if (insertIndexes && resultRelInfo->ri_NumIndices > 0) + if (resultRelInfo->ri_NumIndices > 0) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, slot, estate, false, false, NULL, NIL, false); diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index df63844b35..325d380b0a 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -1135,15 +1135,13 @@ ExecInsert(ModifyTableContext *context, } else { - bool insertIndexes; - /* insert the tuple normally */ slot = table_tuple_insert(resultRelationDesc, slot, estate->es_output_cid, - 0, NULL, &insertIndexes); + 0, NULL); /* insert index entries for tuple */ - if (insertIndexes && resultRelInfo->ri_NumIndices > 0) + if (resultRelInfo->ri_NumIndices > 0) recheckIndexes = ExecInsertIndexTuples(resultRelInfo, slot, estate, false, false, NULL, NIL, diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h index f84dbe629f..3abe226bd5 100644 --- a/src/include/access/heapam.h +++ b/src/include/access/heapam.h @@ -320,7 +320,7 @@ extern void heap_insert(Relation relation, HeapTuple tup, CommandId cid, int options, BulkInsertState bistate); extern void heap_multi_insert(Relation relation, struct TupleTableSlot **slots, int ntuples, CommandId cid, int options, - BulkInsertState bistate, bool *insert_indexes); + BulkInsertState bistate); extern TM_Result heap_delete(Relation relation, ItemPointer tid, CommandId cid, Snapshot crosscheck, int options, struct TM_FailureData *tmfd, bool changingPart, diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index a07f250b46..00deb36f5a 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -519,8 +519,7 @@ typedef struct TableAmRoutine /* see table_tuple_insert() for reference about parameters */ TupleTableSlot *(*tuple_insert) (Relation rel, TupleTableSlot *slot, CommandId cid, int options, - struct BulkInsertStateData *bistate, - bool *insert_indexes); + struct BulkInsertStateData *bistate); /* see table_tuple_insert_speculative() for reference about parameters */ void (*tuple_insert_speculative) (Relation rel, @@ -538,8 +537,7 @@ typedef struct TableAmRoutine /* see table_multi_insert() for reference about parameters */ void (*multi_insert) (Relation rel, TupleTableSlot **slots, int nslots, - CommandId cid, int options, struct BulkInsertStateData *bistate, - bool *insert_indexes); + CommandId cid, int options, struct BulkInsertStateData *bistate); /* see table_tuple_delete() for reference about parameters */ TM_Result (*tuple_delete) (Relation rel, @@ -1387,12 +1385,6 @@ table_index_delete_tuples(Relation rel, TM_IndexDeleteOp *delstate) * behavior) is also just passed through to RelationGetBufferForTuple. If * `bistate` is provided, table_finish_bulk_insert() needs to be called. * - * The table AM's implementation of tuple_insert should set `*insert_indexes` - * to true if it expects the caller to insert the relevant index tuples - * (as heap table AM does). It should set `*insert_indexes` to false if - * it cares about index inserts itself and doesn't want the caller to do - * index inserts. - * * Returns the slot containing the inserted tuple, which may differ from the * given slot. For instance, the source slot may be VirtualTupleTableSlot, but * the result slot may correspond to the table AM. On return the slot's @@ -1402,11 +1394,10 @@ table_index_delete_tuples(Relation rel, TM_IndexDeleteOp *delstate) */ static inline TupleTableSlot * table_tuple_insert(Relation rel, TupleTableSlot *slot, CommandId cid, - int options, struct BulkInsertStateData *bistate, - bool *insert_indexes) + int options, struct BulkInsertStateData *bistate) { return rel->rd_tableam->tuple_insert(rel, slot, cid, options, - bistate, insert_indexes); + bistate); } /* @@ -1458,11 +1449,10 @@ table_tuple_complete_speculative(Relation rel, TupleTableSlot *slot, */ static inline void table_multi_insert(Relation rel, TupleTableSlot **slots, int nslots, - CommandId cid, int options, struct BulkInsertStateData *bistate, - bool *insert_indexes) + CommandId cid, int options, struct BulkInsertStateData *bistate) { rel->rd_tableam->multi_insert(rel, slots, nslots, - cid, options, bistate, insert_indexes); + cid, options, bistate); } /* @@ -2087,8 +2077,7 @@ table_scan_sample_next_tuple(TableScanDesc scan, * ---------------------------------------------------------------------------- */ -extern void simple_table_tuple_insert(Relation rel, TupleTableSlot *slot, - bool *insert_indexes); +extern void simple_table_tuple_insert(Relation rel, TupleTableSlot *slot); extern void simple_table_tuple_delete(Relation rel, ItemPointer tid, Snapshot snapshot, TupleTableSlot *oldSlot);