Make index insert compatible with outside callers

We split aminsert method to aminsert and aminsertextended. aminsert is a method for indexes implemented in other extensions, it accepts ItemPointer tupleid. aminsertextended is for internal Postgres indexes and Orioledb, it accepts Datum tupleid. They are not supposed to call aminsert method, so that it is set NULL for them. We can not rely that extensions are aware of aminsertextended, so index_insert() calls aminsert if it's not NULL preferentially. Signature of index_insert() is reverted so that it could be called by other extensions. Datum tupleid is confined inside index_insert method.
orioledb · Oct 18, 2024 · 333f7e3 · 333f7e3
1 parent 313aa6c
commit 333f7e3
Show file tree

Hide file tree

Showing 18 changed files with 62 additions and 27 deletions.
diff --git a/contrib/bloom/blutils.c b/contrib/bloom/blutils.c
@@ -131,7 +131,8 @@ blhandler(PG_FUNCTION_ARGS)
 
 	amroutine->ambuild = blbuild;
 	amroutine->ambuildempty = blbuildempty;
-	amroutine->aminsert = blinsert;
+	amroutine->aminsert = NULL;
+	amroutine->aminsertextended = blinsert;
 	amroutine->aminsertcleanup = NULL;
 	amroutine->ambulkdelete = blbulkdelete;
 	amroutine->amvacuumcleanup = blvacuumcleanup;

diff --git a/doc/src/sgml/indexam.sgml b/doc/src/sgml/indexam.sgml
@@ -141,6 +141,7 @@ typedef struct IndexAmRoutine
     ambuild_function ambuild;
     ambuildempty_function ambuildempty;
     aminsert_function aminsert;
+    aminsert_extended_function aminsertextended;
     aminsertcleanup_function aminsertcleanup;
     ambulkdelete_function ambulkdelete;
     amvacuumcleanup_function amvacuumcleanup;

diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
@@ -273,7 +273,8 @@ brinhandler(PG_FUNCTION_ARGS)
 
 	amroutine->ambuild = brinbuild;
 	amroutine->ambuildempty = brinbuildempty;
-	amroutine->aminsert = brininsert;
+	amroutine->aminsert = NULL;
+	amroutine->aminsertextended = brininsert;
 	amroutine->aminsertcleanup = brininsertcleanup;
 	amroutine->ambulkdelete = brinbulkdelete;
 	amroutine->amvacuumcleanup = brinvacuumcleanup;

diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
@@ -338,7 +338,7 @@ toast_save_datum(Relation rel, Datum value,
 			/* Only index relations marked as ready can be updated */
 			if (toastidxs[i]->rd_index->indisready)
 				index_insert(toastidxs[i], t_values, t_isnull,
-							 ItemPointerGetDatum(&(toasttup->t_self)),
+							 &(toasttup->t_self),
 							 toastrel,
 							 toastidxs[i]->rd_index->indisunique ?
 							 UNIQUE_CHECK_YES : UNIQUE_CHECK_NO,

diff --git a/src/backend/access/gin/ginutil.c b/src/backend/access/gin/ginutil.c
@@ -63,7 +63,8 @@ ginhandler(PG_FUNCTION_ARGS)
 
 	amroutine->ambuild = ginbuild;
 	amroutine->ambuildempty = ginbuildempty;
-	amroutine->aminsert = gininsert;
+	amroutine->aminsert = NULL;
+	amroutine->aminsertextended = gininsert;
 	amroutine->aminsertcleanup = NULL;
 	amroutine->ambulkdelete = ginbulkdelete;
 	amroutine->amvacuumcleanup = ginvacuumcleanup;

diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
@@ -85,7 +85,8 @@ gisthandler(PG_FUNCTION_ARGS)
 
 	amroutine->ambuild = gistbuild;
 	amroutine->ambuildempty = gistbuildempty;
-	amroutine->aminsert = gistinsert;
+	amroutine->aminsert = NULL;
+	amroutine->aminsertextended = gistinsert;
 	amroutine->aminsertcleanup = NULL;
 	amroutine->ambulkdelete = gistbulkdelete;
 	amroutine->amvacuumcleanup = gistvacuumcleanup;

diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
@@ -83,7 +83,8 @@ hashhandler(PG_FUNCTION_ARGS)
 
 	amroutine->ambuild = hashbuild;
 	amroutine->ambuildempty = hashbuildempty;
-	amroutine->aminsert = hashinsert;
+	amroutine->aminsert = NULL;
+	amroutine->aminsertextended = hashinsert;
 	amroutine->aminsertcleanup = NULL;
 	amroutine->ambulkdelete = hashbulkdelete;
 	amroutine->amvacuumcleanup = hashvacuumcleanup;

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
@@ -2313,7 +2313,7 @@ heapam_index_validate_scan(Relation heapRelation,
 			index_insert(indexRelation,
 						 values,
 						 isnull,
-						 ItemPointerGetDatum(&rootTuple),
+						 &rootTuple,
 						 heapRelation,
 						 indexInfo->ii_Unique ?
 						 UNIQUE_CHECK_YES : UNIQUE_CHECK_NO,

diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
@@ -213,24 +213,39 @@ bool
 index_insert(Relation indexRelation,
 			 Datum *values,
 			 bool *isnull,
-			 Datum tupleid,
+			 ItemPointer tupleid,
 			 Relation heapRelation,
 			 IndexUniqueCheck checkUnique,
 			 bool indexUnchanged,
 			 IndexInfo *indexInfo)
 {
 	RELATION_CHECKS;
-	CHECK_REL_PROCEDURE(aminsert);
+
+	if (indexRelation->rd_indam->aminsertextended == NULL && indexRelation->rd_indam->aminsert == NULL )
+		elog(ERROR, "at least one function aminsert or aminsertextended should be defined for index \"%s\"", \
+			RelationGetRelationName(indexRelation));
 
 	if (!(indexRelation->rd_indam->ampredlocks))
 		CheckForSerializableConflictIn(indexRelation,
 									   (ItemPointer) NULL,
 									   InvalidBlockNumber);
 
-	return indexRelation->rd_indam->aminsert(indexRelation, values, isnull,
+	if (indexRelation->rd_indam->aminsert)
+	{
+		/* compatibility method for extension AM's not aware of aminsertextended */
+		return indexRelation->rd_indam->aminsert(indexRelation, values, isnull,
 											 tupleid, heapRelation,
 											 checkUnique, indexUnchanged,
 											 indexInfo);
+	}
+	else
+	{
+		/* index insert method for internal AM's and Orioledb that are aware of aminsertextended */
+		return indexRelation->rd_indam->aminsertextended(indexRelation, values, isnull,
+											 ItemPointerGetDatum(tupleid), heapRelation,
+											 checkUnique, indexUnchanged,
+											 indexInfo);
+	}
 }
 
 /* -------------------------

diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
@@ -127,7 +127,8 @@ bthandler(PG_FUNCTION_ARGS)
 
 	amroutine->ambuild = btbuild;
 	amroutine->ambuildempty = btbuildempty;
-	amroutine->aminsert = btinsert;
+	amroutine->aminsert = NULL;
+	amroutine->aminsertextended = btinsert;
 	amroutine->aminsertcleanup = NULL;
 	amroutine->ambulkdelete = btbulkdelete;
 	amroutine->amvacuumcleanup = btvacuumcleanup;

diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
@@ -70,7 +70,8 @@ spghandler(PG_FUNCTION_ARGS)
 
 	amroutine->ambuild = spgbuild;
 	amroutine->ambuildempty = spgbuildempty;
-	amroutine->aminsert = spginsert;
+	amroutine->aminsert = NULL;
+	amroutine->aminsertextended = spginsert;
 	amroutine->aminsertcleanup = NULL;
 	amroutine->ambulkdelete = spgbulkdelete;
 	amroutine->amvacuumcleanup = spgvacuumcleanup;

diff --git a/src/backend/catalog/indexing.c b/src/backend/catalog/indexing.c
@@ -170,7 +170,7 @@ CatalogIndexInsert(CatalogIndexState indstate, HeapTuple heapTuple,
 		index_insert(index,		/* index relation */
 					 values,	/* array of index Datums */
 					 isnull,	/* is-null flags */
-					 ItemPointerGetDatum(&(heapTuple->t_self)),	/* tid of heap tuple */
+					 &(heapTuple->t_self),	/* tid of heap tuple */
 					 heapRelation,
 					 index->rd_index->indisunique ?
 					 UNIQUE_CHECK_YES : UNIQUE_CHECK_NO,

diff --git a/src/backend/commands/constraint.c b/src/backend/commands/constraint.c
@@ -171,7 +171,7 @@ unique_key_recheck(PG_FUNCTION_ARGS)
 		 * the row is now dead, because that is the TID the index will know
 		 * about.
 		 */
-		index_insert(indexRel, values, isnull, ItemPointerGetDatum(&checktid),
+		index_insert(indexRel, values, isnull, &checktid,
 					 trigdata->tg_relation, UNIQUE_CHECK_EXISTING,
 					 false, indexInfo);
 

diff --git a/src/backend/executor/execIndexing.c b/src/backend/executor/execIndexing.c
@@ -313,19 +313,19 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
 	ExprContext *econtext;
 	Datum		values[INDEX_MAX_KEYS];
 	bool		isnull[INDEX_MAX_KEYS];
-	Datum		tupleid;
+	ItemPointer	tupleid;
 
 
 	if (table_get_row_ref_type(resultRelInfo->ri_RelationDesc) == ROW_REF_ROWID)
 	{
 		bool	isnull;
-		tupleid = slot_getsysattr(slot, RowIdAttributeNumber, &isnull);
+		tupleid = DatumGetItemPointer(slot_getsysattr(slot, RowIdAttributeNumber, &isnull));
 		Assert(!isnull);
 	}
 	else
 	{
 		Assert(ItemPointerIsValid(&slot->tts_tid));
-		tupleid = PointerGetDatum(&slot->tts_tid);
+		tupleid = &slot->tts_tid;
 	}
 
 	/*
@@ -473,7 +473,6 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
 		{
 			bool		violationOK;
 			CEOUC_WAIT_MODE waitMode;
-			ItemPointer raw_tupleid = DatumGetItemPointer(tupleid);
 
 			if (applyNoDupErr)
 			{
@@ -494,7 +493,7 @@ ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
 			satisfiesConstraint =
 				check_exclusion_or_unique_constraint(heapRelation,
 													 indexRelation, indexInfo,
-													 raw_tupleid, values, isnull,
+													 tupleid, values, isnull,
 													 estate, false,
 													 waitMode, violationOK, NULL);
 		}
@@ -537,18 +536,18 @@ ExecUpdateIndexTuples(ResultRelInfo *resultRelInfo,
 	ExprContext *econtext;
 	Datum		values[INDEX_MAX_KEYS];
 	bool		isnull[INDEX_MAX_KEYS];
-	Datum		tupleid;
+	ItemPointer	tupleid;
 
 	if (table_get_row_ref_type(resultRelInfo->ri_RelationDesc) == ROW_REF_ROWID)
 	{
 		bool	isnull;
-		tupleid = slot_getsysattr(slot, RowIdAttributeNumber, &isnull);
+		tupleid = DatumGetItemPointer(slot_getsysattr(slot, RowIdAttributeNumber, &isnull));
 		Assert(!isnull);
 	}
 	else
 	{
 		Assert(ItemPointerIsValid(&slot->tts_tid));
-		tupleid = PointerGetDatum(&slot->tts_tid);
+		tupleid = &slot->tts_tid;
 	}
 
 	/*
@@ -717,7 +716,7 @@ ExecUpdateIndexTuples(ResultRelInfo *resultRelInfo,
 							 old_valid,
 							 values,	/* array of index Datums */
 							 isnull,	/* null flags */
-							 tupleid,	/* tid of heap tuple */
+							 ItemPointerGetDatum(tupleid),	/* tid of heap tuple */
 							 valuesOld,
 							 isnullOld,
 							 oldTupleid,
@@ -768,7 +767,6 @@ ExecUpdateIndexTuples(ResultRelInfo *resultRelInfo,
 		{
 			bool		violationOK;
 			CEOUC_WAIT_MODE waitMode;
-			ItemPointer raw_tupleid = DatumGetItemPointer(tupleid);
 
 			if (applyNoDupErr)
 			{
@@ -789,7 +787,7 @@ ExecUpdateIndexTuples(ResultRelInfo *resultRelInfo,
 			satisfiesConstraint =
 				check_exclusion_or_unique_constraint(heapRelation,
 													 indexRelation, indexInfo,
-													 raw_tupleid, values, isnull,
+													 tupleid, values, isnull,
 													 estate, false,
 													 waitMode, violationOK, NULL);
 		}

diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
@@ -105,13 +105,24 @@ typedef void (*ambuildempty_function) (Relation indexRelation);
 
 /* insert this tuple */
 typedef bool (*aminsert_function) (Relation indexRelation,
+								   Datum *values,
+								   bool *isnull,
+								   ItemPointer tupleid,
+								   Relation heapRelation,
+								   IndexUniqueCheck checkUnique,
+								   bool indexUnchanged,
+								   struct IndexInfo *indexInfo);
+
+/* extended version of aminsert taking Datum tupleid */
+typedef bool (*aminsert_extended_function) (Relation indexRelation,
 								   Datum *values,
 								   bool *isnull,
 								   Datum tupleid,
 								   Relation heapRelation,
 								   IndexUniqueCheck checkUnique,
 								   bool indexUnchanged,
 								   struct IndexInfo *indexInfo);
+
 /* update this tuple */
 typedef bool (*amupdate_function) (Relation indexRelation,
 								   bool new_valid,
@@ -288,6 +299,7 @@ typedef struct IndexAmRoutine
 	ambuild_function ambuild;
 	ambuildempty_function ambuildempty;
 	aminsert_function aminsert;
+	aminsert_extended_function aminsertextended;
 	aminsertcleanup_function aminsertcleanup;
 	amupdate_function amupdate;
 	amdelete_function amdelete;

diff --git a/src/include/access/genam.h b/src/include/access/genam.h
@@ -144,7 +144,7 @@ extern void index_close(Relation relation, LOCKMODE lockmode);
 
 extern bool index_insert(Relation indexRelation,
 						 Datum *values, bool *isnull,
-						 Datum tupleid,
+						 ItemPointer tupleid,
 						 Relation heapRelation,
 						 IndexUniqueCheck checkUnique,
 						 bool indexUnchanged,

diff --git a/src/test/modules/dummy_index_am/dummy_index_am.c b/src/test/modules/dummy_index_am/dummy_index_am.c
@@ -303,7 +303,8 @@ dihandler(PG_FUNCTION_ARGS)
 
 	amroutine->ambuild = dibuild;
 	amroutine->ambuildempty = dibuildempty;
-	amroutine->aminsert = diinsert;
+	amroutine->aminsert = NULL;
+	amroutine->aminsertextended = diinsert;
 	amroutine->ambulkdelete = dibulkdelete;
 	amroutine->amvacuumcleanup = divacuumcleanup;
 	amroutine->amcanreturn = NULL;

diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
@@ -3266,6 +3266,7 @@ amgetbitmap_function
 amgettuple_function
 aminitparallelscan_function
 aminsert_function
+aminsert_extended_function
 aminsertcleanup_function
 ammarkpos_function
 amoptions_function