Add macro RelationIsNonblockRelation to expand code path like AO/CO (#347)

The builtin types of tables, in kernel, are standard heap table and AO/CO.
The custom table AM will fall one of the two code paths.
RelationIsAppendOptimized will only choose the builtin AO/CO relations.
Some custom table AM, like PAX, will run the same code path as AO/CO.

RelationIsNonblockRelation looks replacable by `!RelationIsHeap`, but
they have different meanings. RelationIsNonblockRelation expand the
relation type to run the code path like AO/CO. `!RelationIsHeap`
emphasizes NOT heap relation.

To work with this change, we preserve and oid for pax, to prevent
oid clash in the future.

Co-authored-by: wuhao <wuhao@hashdata.cn>
Reviewed-by: Zhang Mingli <avamingli@gmail.com>

Author: gfphoenix78

src/backend/commands/analyze.c

@@ -1621,8 +1621,9 @@ acquire_sample_rows(Relation onerel, int elevel,
 	 * GPDB_12_MERGE_FIXME: BlockNumber is uint32 and Number of tuples is uint64.
 	 * That means that after row number UINT_MAX we will never analyze the table.
 	 */
-	if (RelationIsAppendOptimized(onerel))
+	if (RelationIsNonblockRelation(onerel))
 	{
+		/* AO/CO/PAX use non-fixed block layout */
 		BlockNumber pages;
 		double		tuples;
 		double		allvisfrac;
@@ -1653,7 +1654,7 @@ acquire_sample_rows(Relation onerel, int elevel,
 	 * because Blocks is not same as Heap tables.
 	 * Set prefetch_maximum to zero seems the easiest way to bypass.
 	 */
-	if (RelationIsAppendOptimized(onerel))
+	if (RelationIsNonblockRelation(onerel))
 	{
 		prefetch_maximum = 0;
 	}

src/backend/commands/trigger.c

@@ -715,7 +715,7 @@ CreateTriggerFiringOn(CreateTrigStmt *stmt, const char *queryString,
 	}
 
 	/* Check GPDB limitations */
-	if (RelationIsAppendOptimized(rel) &&
+	if (RelationIsNonblockRelation(rel) &&
 		TRIGGER_FOR_ROW(tgtype) &&
 		!stmt->isconstraint)
 	{
@@ -3105,7 +3105,7 @@ GetTupleForTrigger(EState *estate,
 	Relation	relation = relinfo->ri_RelationDesc;
 
 	/* these should be rejected when you try to create such triggers, but let's check */
-	if (RelationIsAppendOptimized(relation))
+	if (RelationIsNonblockRelation(relation))
 		elog(ERROR, "UPDATE and DELETE triggers are not supported on append-only tables");
 
 	Assert(RelationIsHeap(relation));

src/backend/executor/nodeModifyTable.c

@@ -1463,7 +1463,7 @@ ldelete:;
 	 * triggers on an UPDATE that moves tuples from one partition to another.
 	 * Should we follow that example with cross-segment UPDATEs too?
 	 */
-	if (!RelationIsAppendOptimized(resultRelationDesc) && !splitUpdate)
+	if (!RelationIsNonblockRelation(resultRelationDesc) && !splitUpdate)
 	{
 		ExecARDeleteTriggers(estate, resultRelInfo, tupleid, oldtuple,
 							 ar_delete_trig_tcs);
@@ -1949,7 +1949,7 @@ lreplace:;
 				 * AO case, as visimap update within same command happens at end
 				 * of command.
 				 */
-				if (!RelationIsAppendOptimized(resultRelationDesc) &&
+				if (!RelationIsNonblockRelation(resultRelationDesc) &&
 					tmfd.cmax != estate->es_output_cid)
 					ereport(ERROR,
 							(errcode(ERRCODE_TRIGGERED_DATA_CHANGE_VIOLATION),
@@ -2075,7 +2075,7 @@ lreplace:;
 
 	/* AFTER ROW UPDATE Triggers */
 	/* GPDB: AO and AOCO tables don't support triggers */
-	if (!RelationIsAppendOptimized(resultRelationDesc))
+	if (!RelationIsNonblockRelation(resultRelationDesc))
 		ExecARUpdateTriggers(estate, resultRelInfo, tupleid, oldtuple, slot,
 						 recheckIndexes,
 						 mtstate->operation == CMD_INSERT ?
@@ -2664,8 +2664,11 @@ ExecModifyTable(PlanState *pstate)
 				 * We need to fix this problem by redesigning AO/AOCS storage format or
 				 * making the update plan is consistent whether it generated by pg optimizer
 				 * or ORCA optimizer in the future.
+				 *
+				 * PAX_STORAGE_FIXME(gongxun):we reuse the logic of the AO table to implement ExecUpdate,
+				 * If there is a better implementation, we need to revert it
 				 */
-				if (operation == CMD_UPDATE && !RelationIsHeap(resultRelInfo->ri_RelationDesc) &&
+				if (operation == CMD_UPDATE && RelationIsNonblockRelation(resultRelInfo->ri_RelationDesc) &&
 					AttributeNumberIsValid(resultRelInfo->ri_WholeRowNo))
 				{
 					/* ri_WholeRowNo refers to a wholerow attribute */
@@ -3137,7 +3140,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 					elog(ERROR, "could not find junk ctid column");
 
 				/* extra GPDB junk columns for update AO table */
-				if (operation == CMD_UPDATE && !RelationIsHeap(resultRelInfo->ri_RelationDesc))
+				if (operation == CMD_UPDATE && RelationIsNonblockRelation(resultRelInfo->ri_RelationDesc))
 				{
 					resultRelInfo->ri_WholeRowNo =
 						ExecFindJunkAttributeInTlist(subplan->targetlist, "wholerow");

src/backend/gpopt/translate/CTranslatorRelcacheToDXL.cpp

@@ -2435,17 +2435,17 @@ CTranslatorRelcacheToDXL::RetrieveRelStorageType(Relation rel)
 			break;
 		// FIXME: need to add support for custom table am!!!
 		//
-		// Why 7014 here?
+		// Why 7047 here?
 		// Because we defined a custom table am using columnar storage,
 		// the orca optimizer does not support am other than HEAP/AO/AOCS. At present,
 		// there is no way to extend orca to support custom table am. So here we use
-		// the am_id(7014) we assigned to the custom table am, and let the orca optimizer
+		// the am_id(7047) we assigned to the custom table am, and let the orca optimizer
 		// treat this columnar storage format as AOCS to generate an execution plan
 		//
-		// Why use the magic number 7014 instead of the macro definition?
+		// Why use the magic number 7047 instead of the macro definition?
 		// Just to make it look like it doesn't make sense,
 		// so others will notice that the logic needs to be refactored
-		case 7014:
+		case PAX_AM_OID:
 		case AO_COLUMN_TABLE_AM_OID:
 			rel_storage_type = IMDRelation::ErelstorageAppendOnlyCols;
 			break;

src/backend/optimizer/util/appendinfo.c

@@ -953,7 +953,7 @@ add_row_identity_columns(PlannerInfo *root, Index rtindex,
 		 * redesigning AO/AOCS storage format or making the update plan is
 		 * consistent whether it generated by pg optimizer or ORCA optimizer.
 		 */
-		if (commandType == CMD_UPDATE && !RelationIsHeap(target_relation))
+		if (commandType == CMD_UPDATE && RelationIsNonblockRelation(target_relation))
 		{
 			var = makeVar(rtindex,
 						  InvalidAttrNumber,

src/backend/utils/adt/dbsize.c

@@ -463,8 +463,8 @@ calculate_relation_size(Relation rel, ForkNumber forknum)
 	if (relation_size_hook)
 		return (*relation_size_hook) (rel, forknum);
 
-	/* Call into the tableam api for AO/AOCO relations */
-	if (RelationIsAppendOptimized(rel))
+	/* Call into the tableam api if the table is not heap, i.e. AO/CO/PAX relations. */
+	if (RelationIsNonblockRelation(rel))
 		return table_relation_size(rel, forknum);
 
 	relationpath = relpathbackend(rel->rd_node, rel->rd_backend, forknum);

src/include/catalog/duplicate_oids

@@ -32,6 +32,9 @@ my $oids = Catalog::FindAllOidsFromHeaders(@input_files);
 
 my %oidcounts;
 
+# preserved oid for PAX table AM
+$oidcounts{7047}++;
+
 foreach my $oid (@{$oids})
 {
 	$oidcounts{$oid}++;

src/include/utils/rel.h

@@ -20,6 +20,7 @@
 #include "fmgr.h"
 #include "access/tupdesc.h"
 #include "access/xlog.h"
+#include "catalog/pg_am.h"
 #include "catalog/pg_appendonly.h"
 #include "catalog/pg_class.h"
 #include "catalog/pg_index.h"
@@ -516,6 +517,29 @@ typedef struct ViewOptions
 #define RelationIsAppendOptimized(relation) \
 	AMHandlerIsAO((relation)->rd_amhandler)
 
+/*
+ * FIXME: CBDB should not know the am oid of PAX. We put here because the kernel
+ * can't distinguish the PAX and renamed heap(heap_psql) in test `psql`.
+ */
+#define PAX_AM_OID 7047
+/*
+ * CAUTION: this macro is a violation of the absraction that table AM and
+ * index AM interfaces provide.  Use of this macro is discouraged.  If
+ * table/index AM API falls short for your use case, consider enhancing the
+ * interface.
+ *
+ * RelationIsNonblockRelation looks replacable by `!RelationIsHeap`, but
+ * they have different meanings. RelationIsNonblockRelation expand the
+ * relation type to run the code path like AO/CO. `!RelationIsHeap`
+ * emphasizes NOT heap relation.
+ *
+ * RelationIsNonblockRelation
+ *      True iff relation(table) should run the code path as AO/CO
+ */
+#define RelationIsNonblockRelation(relation) \
+	(RelationIsAppendOptimized(relation) || \
+	 (relation)->rd_rel->relam == PAX_AM_OID)
+
 /*
  * RelationIsBitmapIndex
  *      True iff relation is a bitmap index