Refactor process_utility to reduce reliance on compressed_chunk_id

Using oid directly in process_utility allows us to skip some
catalog scans we would otherwise do when instantiating Chunk
object.

Author: svenklemm

src/chunk.c

@@ -1935,7 +1935,7 @@ chunk_scan_context_add_chunk(ChunkScanCtx *scanctx, ChunkStub *stub)
 }
 
 TM_Result
-ts_chunk_lock_for_creating_compressed_chunk(int32 chunk_id, int32 *compressed_chunk_id)
+ts_chunk_lock_for_creating_compressed_chunk(Chunk *chunk)
 {
 	ScanIterator iterator;
 	bool found = false;
@@ -1947,19 +1947,24 @@ ts_chunk_lock_for_creating_compressed_chunk(int32 chunk_id, int32 *compressed_ch
 	};
 
 	iterator = ts_scan_iterator_create(CHUNK, RowShareLock, CurrentMemoryContext);
-	ts_chunk_scan_iterator_set_chunk_id(&iterator, chunk_id);
+	ts_chunk_scan_iterator_set_chunk_id(&iterator, chunk->fd.id);
 	iterator.ctx.tuplock = &tuplock;
 
 	ts_scanner_foreach(&iterator)
 	{
 		TupleInfo *ti = ts_scan_iterator_tuple_info(&iterator);
 		lockresult = ti->lockresult;
 
-		if (lockresult == TM_Ok && compressed_chunk_id)
+		/*
+		 * Refresh the chunk's status from the locked tuple so the caller can
+		 * recheck whether the chunk was compressed concurrently.
+		 */
+		if (lockresult == TM_Ok)
 		{
 			bool isnull;
-			Datum value = slot_getattr(ti->slot, Anum_chunk_compressed_chunk_id, &isnull);
-			*compressed_chunk_id = isnull ? INVALID_CHUNK_ID : DatumGetInt32(value);
+			Datum value = slot_getattr(ti->slot, Anum_chunk_status, &isnull);
+			Assert(!isnull);
+			chunk->fd.status = DatumGetInt32(value);
 		}
 		found = true;
 	}
@@ -1968,7 +1973,7 @@ ts_chunk_lock_for_creating_compressed_chunk(int32 chunk_id, int32 *compressed_ch
 
 	if (!found)
 	{
-		elog(ERROR, "chunk with ID %d does not exist", chunk_id);
+		elog(ERROR, "chunk with ID %d does not exist", chunk->fd.id);
 	}
 
 	return lockresult;
@@ -3130,10 +3135,11 @@ ts_chunk_exists_with_compression(int32 hypertable_id)
 	init_scan_by_hypertable_id(&iterator, hypertable_id);
 	ts_scanner_foreach(&iterator)
 	{
-		bool isnull_chunk_id =
-			slot_attisnull(ts_scan_iterator_slot(&iterator), Anum_chunk_compressed_chunk_id);
+		bool status_isnull;
+		Datum status =
+			slot_getattr(ts_scan_iterator_slot(&iterator), Anum_chunk_status, &status_isnull);
 
-		if (!isnull_chunk_id)
+		if (!status_isnull && ts_flags_are_set_32(DatumGetInt32(status), CHUNK_STATUS_COMPRESSED))
 		{
 			found = true;
 			break;
@@ -3143,57 +3149,6 @@ ts_chunk_exists_with_compression(int32 hypertable_id)
 	return found;
 }
 
-static void
-init_scan_by_compressed_chunk_id(ScanIterator *iterator, int32 compressed_chunk_id)
-{
-	iterator->ctx.index =
-		catalog_get_index(ts_catalog_get(), CHUNK, CHUNK_COMPRESSED_CHUNK_ID_INDEX);
-	ts_scan_iterator_scan_key_init(iterator,
-								   Anum_chunk_compressed_chunk_id_idx_compressed_chunk_id,
-								   BTEqualStrategyNumber,
-								   F_INT4EQ,
-								   Int32GetDatum(compressed_chunk_id));
-}
-
-Chunk *
-ts_chunk_get_compressed_chunk_parent(const Chunk *chunk)
-{
-	ScanIterator iterator = ts_scan_iterator_create(CHUNK, AccessShareLock, CurrentMemoryContext);
-	Oid parent_id = InvalidOid;
-
-	init_scan_by_compressed_chunk_id(&iterator, chunk->fd.id);
-
-	ts_scanner_foreach(&iterator)
-	{
-		TupleInfo *ti = ts_scan_iterator_tuple_info(&iterator);
-		Datum datum;
-		bool isnull;
-
-		Assert(!OidIsValid(parent_id));
-		datum = slot_getattr(ti->slot, Anum_chunk_id, &isnull);
-
-		if (!isnull)
-		{
-			parent_id = DatumGetObjectId(datum);
-		}
-	}
-
-	if (OidIsValid(parent_id))
-	{
-		return ts_chunk_get_by_id(parent_id, true);
-	}
-
-	return NULL;
-}
-
-bool
-ts_chunk_contains_compressed_data(const Chunk *chunk)
-{
-	Chunk *parent_chunk = ts_chunk_get_compressed_chunk_parent(chunk);
-
-	return parent_chunk != NULL;
-}
-
 List *
 ts_chunk_get_chunk_ids_by_hypertable_id(int32 hypertable_id)
 {

src/chunk.h

@@ -212,7 +212,6 @@ extern TSDLLEXPORT List *ts_chunk_do_drop_chunks(Hypertable *ht, int64 older_tha
 extern TSDLLEXPORT Chunk *
 ts_chunk_find_or_create_without_cuts(const Hypertable *ht, Hypercube *hc, const char *schema_name,
 									 const char *table_name, Oid chunk_table_relid, bool *created);
-extern TSDLLEXPORT Chunk *ts_chunk_get_compressed_chunk_parent(const Chunk *chunk);
 extern TSDLLEXPORT bool ts_chunk_is_unordered(const Chunk *chunk);
 extern TSDLLEXPORT bool ts_chunk_is_partial(const Chunk *chunk);
 extern TSDLLEXPORT bool ts_chunk_is_compressed(const Chunk *chunk);
@@ -222,7 +221,6 @@ extern TSDLLEXPORT bool ts_chunk_validate_chunk_status_for_operation(const Chunk
 																	 ChunkOperation cmd,
 																	 bool throw_error);
 
-extern TSDLLEXPORT bool ts_chunk_contains_compressed_data(const Chunk *chunk);
 extern TSDLLEXPORT ChunkCompressionStatus ts_chunk_get_compression_status(int32 chunk_id);
 extern TSDLLEXPORT Datum ts_chunk_id_from_relid(PG_FUNCTION_ARGS);
 extern TSDLLEXPORT Datum ts_chunk_status_text(PG_FUNCTION_ARGS);
@@ -236,8 +234,7 @@ extern Chunk *ts_chunk_build_from_tuple_and_stub(Chunk **chunkptr, TupleInfo *ti
 												 const ChunkStub *stub,
 												 const ScanTupLock *slice_lock);
 
-extern TM_Result ts_chunk_lock_for_creating_compressed_chunk(int32 chunk_id,
-															 int32 *compressed_chunk_id);
+extern TM_Result ts_chunk_lock_for_creating_compressed_chunk(Chunk *chunk);
 extern ScanIterator ts_chunk_scan_iterator_create(MemoryContext result_mcxt);
 extern void ts_chunk_scan_iterator_set_chunk_id(ScanIterator *it, int32 chunk_id);
 extern bool ts_chunk_lock_if_exists(Oid chunk_oid, LOCKMODE chunk_lockmode);

src/chunk_tuple_routing.c

@@ -178,8 +178,7 @@ ts_chunk_tuple_routing_find_chunk(ChunkTupleRouting *ctr, Point *point)
 
 			DEBUG_WAITPOINT("insert_create_compressed");
 
-			lockres = ts_chunk_lock_for_creating_compressed_chunk(chunk->fd.id,
-																  &chunk->fd.compressed_chunk_id);
+			lockres = ts_chunk_lock_for_creating_compressed_chunk(chunk);
 
 			/*
 			 * Since the locking function blocks and follows the update chain,
@@ -191,15 +190,14 @@ ts_chunk_tuple_routing_find_chunk(ChunkTupleRouting *ctr, Point *point)
 				   "compressed chunk. Lock result %d",
 				   lockres);
 
-			/* recheck whether compressed chunk exists after acquiring the lock */
-			if (!chunk->fd.compressed_chunk_id)
+			/* recheck whether the chunk got compressed after acquiring the lock */
+			if (!ts_chunk_is_compressed(chunk))
 			{
 				Hypertable *compressed_ht =
 					ts_hypertable_get_by_id(ctr->hypertable->fd.compressed_hypertable_id);
 				Chunk *compressed_chunk =
 					ts_cm_functions->compression_chunk_create(compressed_ht, chunk);
 				ts_chunk_set_compressed_chunk(chunk, compressed_chunk->fd.id);
-				chunk->fd.compressed_chunk_id = compressed_chunk->fd.id;
 				created_compressed_chunk = true;
 
 				/* mark chunk as partial unless completely new chunk */

src/planner/planner.c

@@ -57,6 +57,7 @@
 #include "partitioning.h"
 #include "planner/planner.h"
 #include "sort_transform.h"
+#include "ts_catalog/compression_settings.h"
 #include "utils.h"
 
 #include "compat/compat.h"
@@ -1744,26 +1745,18 @@ replace_modify_hypertable_paths(PlannerInfo *root, List *pathlist, RelOptInfo *i
 			/* Check for DML on chunk directly */
 			if (!ht)
 			{
-				Chunk *chunk = ts_chunk_get_by_relid(rte->relid, false);
-				if (!chunk)
-				{
-					/* Not a hypertable or chunk, continue */
-					new_pathlist = lappend(new_pathlist, path);
-					continue;
-				}
-
-				ht = ts_hypertable_get_by_id(chunk->fd.hypertable_id);
-				if (ht->fd.compression_state == HypertableInternalCompressionTable)
+				/*
+				 * For operations on internal compressed chunks we block modifications
+				 * if the chunk belongs to a frozen chunk.
+				 * Direct modifications of uncompressed chunks is intercepted by chunk
+				 * tuple routing.
+				 * In all other cases of direct modification of chunks we dont interfere
+				 * and do not add a ModifyHypertable node.
+				 */
+				Oid uncompressed_relid = ts_relation_get_uncompressed_relid(rte->relid);
+				if (OidIsValid(uncompressed_relid))
 				{
-					/*
-					 * For operations on internal compressed chunks we block modifications
-					 * if the chunk belongs to a frozen chunk.
-					 * Direct modifications of uncompressed chunks is intercepted by chunk
-					 * tuple routing.
-					 * In all other cases of direct modification of chunks we dont interfere
-					 * and do not add a ModifyHypertable node.
-					 */
-					Chunk *uncompressed = ts_chunk_get_compressed_chunk_parent(chunk);
+					Chunk *uncompressed = ts_chunk_get_by_relid(uncompressed_relid, true);
 					if (ts_chunk_is_frozen(uncompressed))
 					{
 						ereport(ERROR,
@@ -1775,6 +1768,16 @@ replace_modify_hypertable_paths(PlannerInfo *root, List *pathlist, RelOptInfo *i
 					new_pathlist = lappend(new_pathlist, path);
 					continue;
 				}
+
+				int32 hypertable_id = ts_chunk_get_hypertable_id_by_reloid(rte->relid);
+				if (hypertable_id == INVALID_HYPERTABLE_ID)
+				{
+					/* Not a hypertable or chunk, continue */
+					new_pathlist = lappend(new_pathlist, path);
+					continue;
+				}
+
+				ht = ts_hypertable_get_by_id(hypertable_id);
 			}
 
 			switch (mt->operation)

src/process_utility.c

@@ -867,24 +867,15 @@ process_copy(ProcessUtilityArgs *args)
 
 		if (!ht)
 		{
-			Chunk *chunk = ts_chunk_get_by_relid(relid, false);
-
-			/* target is neither hypertable nor chunk so let postgres handle it */
-			if (!chunk)
-			{
-				ts_cache_release(&hcache);
-				return DDL_CONTINUE;
-			}
-
-			ht = ts_hypertable_get_by_id(chunk->fd.hypertable_id);
-			if (ht->fd.compression_state == HypertableInternalCompressionTable)
+			/*
+			 * For operations on internal compressed chunks we block modifications
+			 * if the chunk belongs to a frozen chunk otherwise let postgres handle it.
+			 * Uncompressed frozen chunks are intercepted as part of tuple routing.
+			 */
+			Oid uncompressed_relid = ts_relation_get_uncompressed_relid(relid);
+			if (OidIsValid(uncompressed_relid))
 			{
-				/*
-				 * For operations on internal compressed chunks we block modifications
-				 * if the chunk belongs to a frozen chunk otherwise let postgres handle it.
-				 * Uncompressed frozen chunks are intercepted as part of tuple routing.
-				 */
-				Chunk *uncompressed = ts_chunk_get_compressed_chunk_parent(chunk);
+				Chunk *uncompressed = ts_chunk_get_by_relid(uncompressed_relid, true);
 				if (ts_chunk_is_frozen(uncompressed))
 				{
 					ereport(ERROR,
@@ -896,6 +887,16 @@ process_copy(ProcessUtilityArgs *args)
 				ts_cache_release(&hcache);
 				return DDL_CONTINUE;
 			}
+
+			/* target is neither hypertable nor chunk so let postgres handle it */
+			int32 hypertable_id = ts_chunk_get_hypertable_id_by_reloid(relid);
+			if (hypertable_id == INVALID_HYPERTABLE_ID)
+			{
+				ts_cache_release(&hcache);
+				return DDL_CONTINUE;
+			}
+
+			ht = ts_hypertable_get_by_id(hypertable_id);
 		}
 	}
 
@@ -1154,13 +1155,13 @@ add_chunk_to_vacuum(Hypertable *ht, Oid chunk_relid, void *arg)
 	ctx->chunk_rels = lappend(ctx->chunk_rels, chunk_vacuum_rel);
 
 	/* If we have a compressed chunk make sure to analyze it as well */
-	if (chunk->fd.compressed_chunk_id != INVALID_CHUNK_ID)
+	if (ts_chunk_is_compressed(chunk))
 	{
-		Chunk *comp_chunk = ts_chunk_get_by_id(chunk->fd.compressed_chunk_id, false);
+		Oid compressed_relid = ts_relation_get_compressed_relid(chunk->table_id);
 		/* Compressed chunk might be missing due to concurrent operations */
-		if (comp_chunk)
+		if (OidIsValid(compressed_relid))
 		{
-			chunk_vacuum_rel = makeVacuumRelation(NULL, comp_chunk->table_id, NIL);
+			chunk_vacuum_rel = makeVacuumRelation(NULL, compressed_relid, NIL);
 			ctx->chunk_rels = lappend(ctx->chunk_rels, chunk_vacuum_rel);
 		}
 	}
@@ -1565,17 +1566,18 @@ process_truncate(ProcessUtilityArgs *args)
 							ts_continuous_agg_invalidate_chunk(ht, chunk);
 						}
 						/* Truncate the compressed chunk too */
-						if (chunk->fd.compressed_chunk_id != INVALID_CHUNK_ID)
+						if (ts_chunk_is_compressed(chunk))
 						{
-							Chunk *compressed_chunk =
-								ts_chunk_get_by_id(chunk->fd.compressed_chunk_id, false);
-							if (compressed_chunk != NULL)
+							Oid compressed_relid =
+								ts_relation_get_compressed_relid(chunk->table_id);
+							if (OidIsValid(compressed_relid))
 							{
 								/* Create list item into the same context of the list. */
 								oldctx = MemoryContextSwitchTo(parsetreectx);
-								rv = makeRangeVar(NameStr(compressed_chunk->fd.schema_name),
-												  NameStr(compressed_chunk->fd.table_name),
-												  -1);
+								char *schema_name =
+									get_namespace_name(get_rel_namespace(compressed_relid));
+								char *table_name = get_rel_name(compressed_relid);
+								rv = makeRangeVar(schema_name, table_name, -1);
 								MemoryContextSwitchTo(oldctx);
 								list_changed = true;
 							}
@@ -1712,7 +1714,7 @@ process_drop_chunk(ProcessUtilityArgs *args, DropStmt *stmt)
 		{
 			Hypertable *ht;
 
-			if (ts_chunk_contains_compressed_data(chunk))
+			if (ts_relation_is_compressed_chunk_relation(chunk->table_id))
 			{
 				ereport(ERROR,
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -1723,13 +1725,13 @@ process_drop_chunk(ProcessUtilityArgs *args, DropStmt *stmt)
 
 			/* if cascade is enabled, delete the compressed chunk with cascade too. Otherwise
 			 *  it would be blocked if there are dependent objects */
-			if (stmt->behavior == DROP_CASCADE && chunk->fd.compressed_chunk_id != INVALID_CHUNK_ID)
+			if (stmt->behavior == DROP_CASCADE && ts_chunk_is_compressed(chunk))
 			{
-				Chunk *compressed_chunk =
-					ts_chunk_get_by_id_with_slice_lock(chunk->fd.compressed_chunk_id,
-													   AccessExclusiveLock,
-													   &slice_lock,
-													   false);
+				Oid compressed_relid = ts_relation_get_compressed_relid(chunk->table_id);
+				Chunk *compressed_chunk = ts_chunk_get_by_relid_locked(compressed_relid,
+																	   AccessExclusiveLock,
+																	   &slice_lock,
+																	   false);
 				/* The chunk may have been delete by a CASCADE */
 				if (compressed_chunk != NULL)
 				{
@@ -4762,14 +4764,7 @@ process_altertable_end_index(Node *parsetree, CollectedCommand *cmd)
 static void
 process_altertable_chunk_propagate_to_compressed(AlterTableCmd *cmd, Oid relid)
 {
-	Chunk *chunk = ts_chunk_get_by_relid(relid, false);
-
-	if (chunk == NULL)
-	{
-		return;
-	}
-
-	if (ts_chunk_contains_compressed_data(chunk))
+	if (ts_relation_is_compressed_chunk_relation(relid))
 	{
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -4778,12 +4773,12 @@ process_altertable_chunk_propagate_to_compressed(AlterTableCmd *cmd, Oid relid)
 						 "instead.")));
 	}
 
+	Oid compressed_relid = ts_relation_get_compressed_relid(relid);
+
 	/* set tablespace for compressed chunk */
-	if (chunk->fd.compressed_chunk_id != INVALID_CHUNK_ID)
+	if (OidIsValid(compressed_relid))
 	{
-		Chunk *compressed_chunk = ts_chunk_get_by_id(chunk->fd.compressed_chunk_id, true);
-
-		AlterTableInternal(compressed_chunk->table_id, list_make1(cmd), false);
+		AlterTableInternal(compressed_relid, list_make1(cmd), false);
 	}
 }