Clean up API for ambulkdelete/amvacuumcleanup as per today's discussion.

This formulation requires every AM to provide amvacuumcleanup, unlike before,
but it's surely a whole lot cleaner.  Also, add an 'amstorage' column to
pg_am so that we can get rid of hardwired knowledge in DefineOpClass().

Author: tglsfdc

doc/src/sgml/catalogs.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.121 2006/03/10 19:10:46 momjian Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.122 2006/05/02 22:25:09 tgl Exp $ -->
 <!--
  Documentation of the system catalogs, directed toward PostgreSQL developers
  -->
@@ -394,13 +394,27 @@
       <entry>Does the access method support null index entries?</entry>
      </row>
 
+     <row>
+      <entry><structfield>amstorage</structfield></entry>
+      <entry><type>bool</type></entry>
+      <entry></entry>
+      <entry>Can index storage data type differ from column data type?</entry>
+     </row>
+
      <row>
       <entry><structfield>amconcurrent</structfield></entry>
       <entry><type>bool</type></entry>
       <entry></entry>
       <entry>Does the access method support concurrent updates?</entry>
      </row>
 
+     <row>
+      <entry><structfield>amclusterable</structfield></entry>
+      <entry><type>bool</type></entry>
+      <entry></entry>
+      <entry>Can an index of this type be CLUSTERed on?</entry>
+     </row>
+
      <row>
       <entry><structfield>aminsert</structfield></entry>
       <entry><type>regproc</type></entry>

doc/src/sgml/indexam.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/indexam.sgml,v 2.9 2006/03/10 19:10:48 momjian Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/indexam.sgml,v 2.10 2006/05/02 22:25:09 tgl Exp $ -->
 
 <chapter id="indexam">
  <title>Index Access Method Interface Definition</title>
@@ -184,44 +184,52 @@ aminsert (Relation indexRelation,
   <para>
 <programlisting>
 IndexBulkDeleteResult *
-ambulkdelete (Relation indexRelation,
+ambulkdelete (IndexVacuumInfo *info,
+              IndexBulkDeleteResult *stats,
               IndexBulkDeleteCallback callback,
               void *callback_state);
 </programlisting>
    Delete tuple(s) from the index.  This is a <quote>bulk delete</> operation
    that is intended to be implemented by scanning the whole index and checking
    each entry to see if it should be deleted.
-   The passed-in <literal>callback</> function may be called, in the style
+   The passed-in <literal>callback</> function must be called, in the style
    <literal>callback(<replaceable>TID</>, callback_state) returns bool</literal>,
    to determine whether any particular index entry, as identified by its
    referenced TID, is to be deleted.  Must return either NULL or a palloc'd
    struct containing statistics about the effects of the deletion operation.
+   It is OK to return NULL if no information needs to be passed on to
+   <function>amvacuumcleanup</>.
   </para>
 
   <para>
-   If <literal>callback_state</> is NULL then no tuples are to be deleted.
-   The index AM may choose to optimize this case (eg by not scanning the
-   index) but it is still expected to deliver accurate statistics.
+   Because of limited <varname>maintenance_work_mem</>,
+   <function>ambulkdelete</> may need to be called more than once when many
+   tuples are to be deleted.  The <literal>stats</> argument is the result
+   of the previous call for this index (it is NULL for the first call within a
+   <command>VACUUM</> operation).  This allows the AM to accumulate statistics
+   across the whole operation.  Typically, <function>ambulkdelete</> will
+   modify and return the same struct if the passed <literal>stats</> is not
+   null.
   </para>
 
   <para>
 <programlisting>
 IndexBulkDeleteResult *
-amvacuumcleanup (Relation indexRelation,
-                 IndexVacuumCleanupInfo *info,
+amvacuumcleanup (IndexVacuumInfo *info,
                  IndexBulkDeleteResult *stats);
 </programlisting>
-   Clean up after a <command>VACUUM</command> operation (one or more
-   <function>ambulkdelete</> calls).  An index access method does not have
-   to provide this function (if so, the entry in <structname>pg_am</> must
-   be zero).  If it is provided, it is typically used for bulk cleanup
-   such as reclaiming empty index pages.  <literal>info</>
-   provides some additional arguments such as a message level for statistical
-   reports, and <literal>stats</> is whatever the last
-   <function>ambulkdelete</> call returned.  <function>amvacuumcleanup</>
-   may replace or modify this struct before returning it.  If the result
-   is not NULL it must be a palloc'd struct.  The statistics it contains
-   will be reported by <command>VACUUM</> if <literal>VERBOSE</> is given.
+   Clean up after a <command>VACUUM</command> operation (zero or more
+   <function>ambulkdelete</> calls).  This does not have to do anything
+   beyond returning index statistics, but it may perform bulk cleanup
+   such as reclaiming empty index pages.  <literal>stats</> is whatever the
+   last <function>ambulkdelete</> call returned, or NULL if
+   <function>ambulkdelete</> was not called because no tuples needed to be
+   deleted.  If the result is not NULL it must be a palloc'd struct.
+   The statistics it contains will be used to update <structname>pg_class</>,
+   and will be reported by <command>VACUUM</> if <literal>VERBOSE</> is given.
+   It is OK to return NULL if the index was not changed at all during the
+   <command>VACUUM</command> operation, but otherwise correct stats should
+   be returned.
   </para>
 
   <para>

src/backend/access/gin/ginvacuum.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *          $PostgreSQL: pgsql/src/backend/access/gin/ginvacuum.c,v 1.1 2006/05/02 11:28:54 teodor Exp $
+ *          $PostgreSQL: pgsql/src/backend/access/gin/ginvacuum.c,v 1.2 2006/05/02 22:25:10 tgl Exp $
  *-------------------------------------------------------------------------
  */
 
@@ -474,17 +474,25 @@ ginVacuumEntryPage(GinVacuumState *gvs, Buffer buffer, BlockNumber *roots, uint3
 
 Datum
 ginbulkdelete(PG_FUNCTION_ARGS) {
-	Relation    index = (Relation) PG_GETARG_POINTER(0);
-	IndexBulkDeleteCallback callback = (IndexBulkDeleteCallback) PG_GETARG_POINTER(1);
-	void       *callback_state = (void *) PG_GETARG_POINTER(2);
+	IndexVacuumInfo *info = (IndexVacuumInfo *) PG_GETARG_POINTER(0);
+	IndexBulkDeleteResult *stats = (IndexBulkDeleteResult *) PG_GETARG_POINTER(1);
+	IndexBulkDeleteCallback callback = (IndexBulkDeleteCallback) PG_GETARG_POINTER(2);
+	void	   *callback_state = (void *) PG_GETARG_POINTER(3);
+	Relation	index = info->index;
 	BlockNumber	blkno = GIN_ROOT_BLKNO;
 	GinVacuumState	gvs;
 	Buffer 		buffer;
 	BlockNumber	rootOfPostingTree[ BLCKSZ/ (sizeof(IndexTupleData)+sizeof(ItemId)) ];
 	uint32 nRoot;
 
+	/* first time through? */
+	if (stats == NULL)
+		stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
+	/* we'll re-count the tuples each time */
+	stats->num_index_tuples = 0;
+
 	gvs.index = index;
-	gvs.result = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
+	gvs.result = stats;
 	gvs.callback = callback;
 	gvs.callback_state = callback_state;
 	initGinState(&gvs.ginstate, index);
@@ -564,9 +572,9 @@ ginbulkdelete(PG_FUNCTION_ARGS) {
 
 Datum 
 ginvacuumcleanup(PG_FUNCTION_ARGS) {
-	Relation    index = (Relation) PG_GETARG_POINTER(0);
-	IndexVacuumCleanupInfo *info = (IndexVacuumCleanupInfo *) PG_GETARG_POINTER(1);
-	IndexBulkDeleteResult *stats = (IndexBulkDeleteResult *) PG_GETARG_POINTER(2);
+	IndexVacuumInfo *info = (IndexVacuumInfo *) PG_GETARG_POINTER(0);
+	IndexBulkDeleteResult *stats = (IndexBulkDeleteResult *) PG_GETARG_POINTER(1);
+	Relation    index = info->index;
 	bool	 needLock = !RELATION_IS_LOCAL(index);
     BlockNumber npages,
 				blkno;
@@ -576,6 +584,15 @@ ginvacuumcleanup(PG_FUNCTION_ARGS) {
 	BlockNumber lastBlock = GIN_ROOT_BLKNO,
 				   lastFilledBlock = GIN_ROOT_BLKNO;
 
+	/* Set up all-zero stats if ginbulkdelete wasn't called */
+	if (stats == NULL)
+		stats = (IndexBulkDeleteResult *) palloc0(sizeof(IndexBulkDeleteResult));
+	/*
+	 * XXX we always report the heap tuple count as the number of index
+	 * entries.  This is bogus if the index is partial, but it's real hard
+	 * to tell how many distinct heap entries are referenced by a GIN index.
+	 */
+	stats->num_index_tuples = info->num_heap_tuples;
 
 	if (info->vacuum_full) {
 		LockRelation(index, AccessExclusiveLock);

src/backend/access/gist/gistvacuum.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.18 2006/03/31 23:32:05 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/gist/gistvacuum.c,v 1.19 2006/05/02 22:25:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -343,9 +343,9 @@ gistVacuumUpdate(GistVacuum *gv, BlockNumber blkno, bool needunion)
 Datum
 gistvacuumcleanup(PG_FUNCTION_ARGS)
 {
-	Relation	rel = (Relation) PG_GETARG_POINTER(0);
-	IndexVacuumCleanupInfo *info = (IndexVacuumCleanupInfo *) PG_GETARG_POINTER(1);
-	GistBulkDeleteResult *stats = (GistBulkDeleteResult *) PG_GETARG_POINTER(2);
+	IndexVacuumInfo *info = (IndexVacuumInfo *) PG_GETARG_POINTER(0);
+	GistBulkDeleteResult *stats = (GistBulkDeleteResult *) PG_GETARG_POINTER(1);
+	Relation	rel = info->index;
 	BlockNumber npages,
 				blkno;
 	BlockNumber nFreePages,
@@ -355,6 +355,19 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
 				lastFilledBlock = GIST_ROOT_BLKNO;
 	bool		needLock;
 
+	/* Set up all-zero stats if gistbulkdelete wasn't called */
+	if (stats == NULL)
+	{
+		stats = (GistBulkDeleteResult *) palloc0(sizeof(GistBulkDeleteResult));
+		/* use heap's tuple count */
+		Assert(info->num_heap_tuples >= 0);
+		stats->std.num_index_tuples = info->num_heap_tuples;
+		/*
+		 * XXX the above is wrong if index is partial.  Would it be OK to
+		 * just return NULL, or is there work we must do below?
+		 */
+	}
+
 	/* gistVacuumUpdate may cause hard work */
 	if (info->vacuum_full)
 	{
@@ -460,13 +473,6 @@ gistvacuumcleanup(PG_FUNCTION_ARGS)
 	if (info->vacuum_full)
 		UnlockRelation(rel, AccessExclusiveLock);
 
-	/* if gistbulkdelete skipped the scan, use heap's tuple count */
-	if (stats->std.num_index_tuples < 0)
-	{
-		Assert(info->num_heap_tuples >= 0);
-		stats->std.num_index_tuples = info->num_heap_tuples;
-	}
-
 	PG_RETURN_POINTER(stats);
 }
 
@@ -509,36 +515,22 @@ pushStackIfSplited(Page page, GistBDItem *stack)
 Datum
 gistbulkdelete(PG_FUNCTION_ARGS)
 {
-	Relation	rel = (Relation) PG_GETARG_POINTER(0);
-	IndexBulkDeleteCallback callback = (IndexBulkDeleteCallback) PG_GETARG_POINTER(1);
-	void	   *callback_state = (void *) PG_GETARG_POINTER(2);
-	GistBulkDeleteResult *result;
+	IndexVacuumInfo *info = (IndexVacuumInfo *) PG_GETARG_POINTER(0);
+	GistBulkDeleteResult *stats = (GistBulkDeleteResult *) PG_GETARG_POINTER(1);
+	IndexBulkDeleteCallback callback = (IndexBulkDeleteCallback) PG_GETARG_POINTER(2);
+	void	   *callback_state = (void *) PG_GETARG_POINTER(3);
+	Relation	rel = info->index;
 	GistBDItem *stack,
 			   *ptr;
-	bool		needLock;
 
-	result = (GistBulkDeleteResult *) palloc0(sizeof(GistBulkDeleteResult));
+	/* first time through? */
+	if (stats == NULL)
+		stats = (GistBulkDeleteResult *) palloc0(sizeof(GistBulkDeleteResult));
+	/* we'll re-count the tuples each time */
+	stats->std.num_index_tuples = 0;
 
-	/*
-	 * We can skip the scan entirely if there's nothing to delete (indicated
-	 * by callback_state == NULL) and the index isn't partial.  For a partial
-	 * index we must scan in order to derive a trustworthy tuple count.
-	 *
-	 * XXX as of PG 8.2 this is dead code because GIST indexes are always
-	 * effectively partial ... but keep it anyway in case our null-handling
-	 * gets fixed.
-	 */
-	if (callback_state || vac_is_partial_index(rel))
-	{
-		stack = (GistBDItem *) palloc0(sizeof(GistBDItem));
-		stack->blkno = GIST_ROOT_BLKNO;
-	}
-	else
-	{
-		/* skip scan and set flag for gistvacuumcleanup */
-		stack = NULL;
-		result->std.num_index_tuples = -1;
-	}
+	stack = (GistBDItem *) palloc0(sizeof(GistBDItem));
+	stack->blkno = GIST_ROOT_BLKNO;
 
 	while (stack)
 	{
@@ -601,11 +593,11 @@ gistbulkdelete(PG_FUNCTION_ARGS)
 					i--;
 					maxoff--;
 					ntodelete++;
-					result->std.tuples_removed += 1;
+					stats->std.tuples_removed += 1;
 					Assert(maxoff == PageGetMaxOffsetNumber(page));
 				}
 				else
-					result->std.num_index_tuples += 1;
+					stats->std.num_index_tuples += 1;
 			}
 
 			if (ntodelete)
@@ -658,7 +650,7 @@ gistbulkdelete(PG_FUNCTION_ARGS)
 				stack->next = ptr;
 
 				if (GistTupleIsInvalid(idxtuple))
-					result->needFullVacuum = true;
+					stats->needFullVacuum = true;
 			}
 		}
 
@@ -671,13 +663,5 @@ gistbulkdelete(PG_FUNCTION_ARGS)
 		vacuum_delay_point();
 	}
 
-	needLock = !RELATION_IS_LOCAL(rel);
-
-	if (needLock)
-		LockRelationForExtension(rel, ExclusiveLock);
-	result->std.num_pages = RelationGetNumberOfBlocks(rel);
-	if (needLock)
-		UnlockRelationForExtension(rel, ExclusiveLock);
-
-	PG_RETURN_POINTER(result);
+	PG_RETURN_POINTER(stats);
 }