Discard index deletion requests made to columnarAM (#5331)

A write operation might trigger index deletion if index already had
dead entries for the key we are about to insert.
There are two ways of index deletion:
  a) simple deletion
  b) bottom-up deletion (>= pg14)

Since columnar_index_fetch_tuple never sets all_dead to true,
columnarAM doesn't ever expect to receive simple deletion requests
(columnar_index_delete_tuples) as we don't mark any index entries
as dead.

However, since columnarAM doesn't delete any dead entries via simple
deletion, postgres might ask for a more comprehensive deletion
(i.e.: bottom-up) at some point when pg >= 14.

So with this commit, we start gracefully ignoring bottom-up deletion
requests made to columnar_index_delete_tuples.

Given that users can anyway "VACUUM FULL" their columnar tables,
we don't see any problem in ignoring deletion requests.
(cherry picked from commit fe72e8bb48)

src/backend/columnar/columnar_tableam.c

@@ -476,8 +476,11 @@ columnar_index_fetch_tuple(struct IndexFetchTableData *sscan,
 	*call_again = false;
 
 	/*
-	 * No dead tuples are possible in columnar, set it to false if it's
-	 * passed to be non-NULL.
+	 * Initialize all_dead to false if passed to be non-NULL.
+	 *
+	 * XXX: For aborted writes, we should set all_dead to true but this would
+	 * require implementing columnar_index_delete_tuples for simple deletion
+	 * of dead tuples (TM_IndexDeleteOp.bottomup = false).
 	 */
 	if (all_dead)
 	{
@@ -655,7 +658,47 @@ static TransactionId
 columnar_index_delete_tuples(Relation rel,
 							 TM_IndexDeleteOp *delstate)
 {
-	elog(ERROR, "columnar_index_delete_tuples not implemented");
+	CheckCitusVersion(ERROR);
+
+	/*
+	 * XXX: We didn't bother implementing index_delete_tuple for neither of
+	 * simple deletion and bottom-up deletion cases. There is no particular
+	 * reason for that, just to keep things simple.
+	 *
+	 * See the rest of this function to see how we deal with
+	 * index_delete_tuples requests made to columnarAM.
+	 */
+
+	if (delstate->bottomup)
+	{
+		/*
+		 * Ignore any bottom-up deletion requests.
+		 *
+		 * Currently only caller in postgres that does bottom-up deletion is
+		 * _bt_bottomupdel_pass, which in turn calls _bt_delitems_delete_check.
+		 * And this function is okay with ndeltids being set to 0 by tableAM
+		 * for bottom-up deletion.
+		 */
+		delstate->ndeltids = 0;
+		return InvalidTransactionId;
+	}
+	else
+	{
+		/*
+		 * TableAM is not expected to set ndeltids to 0 for simple deletion
+		 * case, so here we cannot do the same trick that we do for
+		 * bottom-up deletion.
+		 * See the assertion around table_index_delete_tuples call in pg
+		 * function index_compute_xid_horizon_for_tuples.
+		 *
+		 * For this reason, to avoid receiving simple deletion requests for
+		 * columnar tables (bottomup = false), columnar_index_fetch_tuple
+		 * doesn't ever set all_dead to true in order to prevent triggering
+		 * simple deletion of index tuples. But let's throw an error to be on
+		 * the safe side.
+		 */
+		elog(ERROR, "columnar_index_delete_tuples not implemented for simple deletion");
+	}
 }
 
 

src/test/regress/expected/columnar_indexes.out

@@ -704,5 +704,90 @@ begin;
   insert into uniq select generate_series(1,100);
 ERROR:  cannot read from index when there is unflushed data in upper transactions
 rollback;
+-- Show that we nicely ignore index deletion requests made to columnarAM.
+--
+-- An INSERT command might trigger index deletion if index already had dead
+-- entries for the key we are about to insert.
+-- There are two ways of index deletion:
+--   a) simple deletion
+--   b) bottom-up deletion (>= pg14)
+--
+-- Since columnar_index_fetch_tuple never sets all_dead to true, columnarAM
+-- doesn't expect to receive simple deletion as we don't mark any index
+-- entries as dead.
+-- Otherwise, columnarAM would throw an error for all of below six test cases.
+--
+-- However, since columnarAM doesn't delete any dead entries via simple
+-- deletion, postgres might ask for a more comprehensive deletion (bottom-up)
+-- at some point when pg >= 14.
+-- For this reason, all following six test cases would certainly trigger
+-- bottom-up deletion. Show that we gracefully ignore such requests.
+CREATE TABLE index_tuple_delete (a int UNIQUE) USING COLUMNAR;
+ALTER TABLE index_tuple_delete SET (autovacuum_enabled = false);
+BEGIN;
+  -- i) rollback before flushing
+	INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+-- index deletion test-1
+BEGIN;
+  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+TRUNCATE index_tuple_delete;
+BEGIN;
+  -- ii) rollback after flushing
+	INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+  SELECT SUM(a) > 0 FROM index_tuple_delete;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+ROLLBACK;
+-- index deletion test-2
+BEGIN;
+  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+TRUNCATE index_tuple_delete;
+BEGIN;
+  -- iii) rollback before flushing, use savepoint
+  SAVEPOINT sp1;
+	  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+  ROLLBACK TO sp1;
+  -- index deletion test-3
+  SAVEPOINT sp2;
+    INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+  ROLLBACK TO sp2;
+  COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+ROLLBACK;
+-- index deletion test-4
+BEGIN;
+  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+TRUNCATE index_tuple_delete;
+BEGIN;
+  -- iv) rollback after flushing, use savepoint
+  SAVEPOINT sp1;
+	  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+    SELECT SUM(a) > 0 FROM index_tuple_delete;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+  ROLLBACK TO sp1;
+  -- index deletion test-5
+  SAVEPOINT sp2;
+    INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+  ROLLBACK TO sp2;
+  COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+ROLLBACK;
+-- index deletion test-6
+BEGIN;
+  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+COPY index_tuple_delete FROM PROGRAM 'seq 10000';
 SET client_min_messages TO WARNING;
 DROP SCHEMA columnar_indexes CASCADE;

src/test/regress/sql/columnar_indexes.sql

@@ -546,5 +546,94 @@ begin;
   insert into uniq select generate_series(1,100);
 rollback;
 
+-- Show that we nicely ignore index deletion requests made to columnarAM.
+--
+-- An INSERT command might trigger index deletion if index already had dead
+-- entries for the key we are about to insert.
+-- There are two ways of index deletion:
+--   a) simple deletion
+--   b) bottom-up deletion (>= pg14)
+--
+-- Since columnar_index_fetch_tuple never sets all_dead to true, columnarAM
+-- doesn't expect to receive simple deletion as we don't mark any index
+-- entries as dead.
+-- Otherwise, columnarAM would throw an error for all of below six test cases.
+--
+-- However, since columnarAM doesn't delete any dead entries via simple
+-- deletion, postgres might ask for a more comprehensive deletion (bottom-up)
+-- at some point when pg >= 14.
+-- For this reason, all following six test cases would certainly trigger
+-- bottom-up deletion. Show that we gracefully ignore such requests.
+CREATE TABLE index_tuple_delete (a int UNIQUE) USING COLUMNAR;
+ALTER TABLE index_tuple_delete SET (autovacuum_enabled = false);
+
+BEGIN;
+  -- i) rollback before flushing
+	INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+
+-- index deletion test-1
+BEGIN;
+  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+
+TRUNCATE index_tuple_delete;
+
+BEGIN;
+  -- ii) rollback after flushing
+	INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+  SELECT SUM(a) > 0 FROM index_tuple_delete;
+ROLLBACK;
+
+-- index deletion test-2
+BEGIN;
+  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+
+TRUNCATE index_tuple_delete;
+
+BEGIN;
+  -- iii) rollback before flushing, use savepoint
+  SAVEPOINT sp1;
+	  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+  ROLLBACK TO sp1;
+
+  -- index deletion test-3
+  SAVEPOINT sp2;
+    INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+  ROLLBACK TO sp2;
+  COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+ROLLBACK;
+
+-- index deletion test-4
+BEGIN;
+  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+
+TRUNCATE index_tuple_delete;
+
+BEGIN;
+  -- iv) rollback after flushing, use savepoint
+  SAVEPOINT sp1;
+	  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+    SELECT SUM(a) > 0 FROM index_tuple_delete;
+  ROLLBACK TO sp1;
+
+  -- index deletion test-5
+  SAVEPOINT sp2;
+    INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+  ROLLBACK TO sp2;
+  COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+ROLLBACK;
+
+-- index deletion test-6
+BEGIN;
+  INSERT INTO index_tuple_delete SELECT i FROM generate_series(0,10000)i;
+ROLLBACK;
+COPY index_tuple_delete FROM PROGRAM 'seq 10000';
+
 SET client_min_messages TO WARNING;
 DROP SCHEMA columnar_indexes CASCADE;