This is an automated email from the ASF dual-hosted git repository.
gfphoenix78 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry.git
The following commit(s) were added to refs/heads/main by this push:
new 1e13f60f7b5 Move the global ext_dml_init_hook/ext_dml_fini_hook to AM
scoped
1e13f60f7b5 is described below
commit 1e13f60f7b5355c2997c36454e25a40a7ea27dd2
Author: Hao Wu <[email protected]>
AuthorDate: Mon Nov 10 15:33:05 2025 +0000
Move the global ext_dml_init_hook/ext_dml_fini_hook to AM scoped
The hooks ext_dml_init_hook and ext_dml_fini_hook are used to initialize
and cleanup resources for table DML operation. It's table AM specific.
The current behavior uses global function pointers. The side effect is
that the hooks must check whether the relation is expected and call
the hook chain for other table AM implementations.
This commit moves the hooks to the table AM structure, so the above
assumption could be discarded.
---
.../src/cpp/access/pax_access_handle.cc | 9 ++-----
.../pax_storage/src/cpp/access/pax_access_handle.h | 3 ---
src/backend/access/aocs/aocsam_handler.c | 6 +++--
.../access/appendonly/appendonlyam_handler.c | 6 +++--
src/backend/access/table/tableamapi.c | 2 ++
src/backend/commands/copyfrom.c | 26 ++-----------------
src/backend/commands/createas.c | 15 +----------
src/backend/commands/matview.c | 7 +----
src/backend/commands/tablecmds.c | 8 ++----
src/backend/executor/execPartition.c | 14 ++--------
src/backend/executor/nodeModifyTable.c | 18 ++-----------
src/include/access/tableam.h | 30 ++++++++++++++++------
src/include/cdb/cdbaocsam.h | 3 ---
src/include/cdb/cdbappendonlyam.h | 2 --
14 files changed, 44 insertions(+), 105 deletions(-)
diff --git a/contrib/pax_storage/src/cpp/access/pax_access_handle.cc
b/contrib/pax_storage/src/cpp/access/pax_access_handle.cc
index 6dcaecd3205..12caf0e9f64 100644
--- a/contrib/pax_storage/src/cpp/access/pax_access_handle.cc
+++ b/contrib/pax_storage/src/cpp/access/pax_access_handle.cc
@@ -412,8 +412,6 @@ void CCPaxAccessMethod::FinishBulkInsert(Relation relation,
int options) {
}
void CCPaxAccessMethod::ExtDmlInit(Relation rel, CmdType operation) {
- if (!RELATION_IS_PAX(rel)) return;
-
CBDB_TRY();
{ pax::CPaxDmlStateLocal::Instance()->InitDmlState(rel, operation); }
CBDB_CATCH_DEFAULT();
@@ -422,8 +420,6 @@ void CCPaxAccessMethod::ExtDmlInit(Relation rel, CmdType
operation) {
}
void CCPaxAccessMethod::ExtDmlFini(Relation rel, CmdType operation) {
- if (!RELATION_IS_PAX(rel)) return;
-
CBDB_TRY();
{ pax::CPaxDmlStateLocal::Instance()->FinishDmlState(rel, operation); }
CBDB_CATCH_DEFAULT();
@@ -791,6 +787,8 @@ static const TableAmRoutine kPaxColumnMethods = {
.scan_sample_next_block = pax::CCPaxAccessMethod::ScanSampleNextBlock,
.scan_sample_next_tuple = pax::CCPaxAccessMethod::ScanSampleNextTuple,
+ .dml_init = pax::CCPaxAccessMethod::ExtDmlInit,
+ .dml_fini = pax::CCPaxAccessMethod::ExtDmlFini,
.amoptions = paxc::PaxAccessMethod::AmOptions,
.swap_relation_files = paxc::PaxAccessMethod::SwapRelationFiles,
.validate_column_encoding_clauses =
@@ -1191,9 +1189,6 @@ void _PG_init(void) { // NOLINT
prev_object_access_hook = object_access_hook;
object_access_hook = PaxObjectAccessHook;
- ext_dml_init_hook = pax::CCPaxAccessMethod::ExtDmlInit;
- ext_dml_finish_hook = pax::CCPaxAccessMethod::ExtDmlFini;
-
prev_ProcessUtilit_hook = ProcessUtility_hook;
ProcessUtility_hook = paxProcessUtility;
diff --git a/contrib/pax_storage/src/cpp/access/pax_access_handle.h
b/contrib/pax_storage/src/cpp/access/pax_access_handle.h
index d541a400d2b..9ed827b4709 100644
--- a/contrib/pax_storage/src/cpp/access/pax_access_handle.h
+++ b/contrib/pax_storage/src/cpp/access/pax_access_handle.h
@@ -197,6 +197,3 @@ class CCPaxAccessMethod final {
};
} // namespace pax
-
-extern ext_dml_func_hook_type ext_dml_init_hook;
-extern ext_dml_func_hook_type ext_dml_finish_hook;
diff --git a/src/backend/access/aocs/aocsam_handler.c
b/src/backend/access/aocs/aocsam_handler.c
index 22ca114cfe0..c2faa538e10 100644
--- a/src/backend/access/aocs/aocsam_handler.c
+++ b/src/backend/access/aocs/aocsam_handler.c
@@ -270,7 +270,7 @@ remove_dml_state(const Oid relationOid)
*
* This function should be called exactly once per relation.
*/
-void
+static void
aoco_dml_init(Relation relation, CmdType operation)
{
init_aoco_dml_states();
@@ -280,7 +280,7 @@ aoco_dml_init(Relation relation, CmdType operation)
/*
* This function should be called exactly once per relation.
*/
-void
+static void
aoco_dml_finish(Relation relation, CmdType operation)
{
AOCODMLState *state;
@@ -2658,6 +2658,8 @@ static TableAmRoutine ao_column_methods = {
.scan_sample_next_block = aoco_scan_sample_next_block,
.scan_sample_next_tuple = aoco_scan_sample_next_tuple,
+ .dml_init = aoco_dml_init,
+ .dml_fini = aoco_dml_finish,
.amoptions = ao_amoptions,
.swap_relation_files = aoco_swap_relation_files,
.validate_column_encoding_clauses =
aoco_validate_column_encoding_clauses,
diff --git a/src/backend/access/appendonly/appendonlyam_handler.c
b/src/backend/access/appendonly/appendonlyam_handler.c
index 218ade630d4..715cebd7579 100644
--- a/src/backend/access/appendonly/appendonlyam_handler.c
+++ b/src/backend/access/appendonly/appendonlyam_handler.c
@@ -228,7 +228,7 @@ remove_dml_state(const Oid relationOid)
*
* This function should be called exactly once per relation.
*/
-void
+static void
appendonly_dml_init(Relation relation, CmdType operation)
{
init_appendonly_dml_states();
@@ -238,7 +238,7 @@ appendonly_dml_init(Relation relation, CmdType operation)
/*
* This function should be called exactly once per relation.
*/
-void
+static void
appendonly_dml_finish(Relation relation, CmdType operation)
{
AppendOnlyDMLState *state;
@@ -2360,6 +2360,8 @@ static const TableAmRoutine ao_row_methods = {
.scan_sample_next_block = appendonly_scan_sample_next_block,
.scan_sample_next_tuple = appendonly_scan_sample_next_tuple,
+ .dml_init = appendonly_dml_init,
+ .dml_fini = appendonly_dml_finish,
.amoptions = ao_amoptions,
.swap_relation_files = appendonly_swap_relation_files,
};
diff --git a/src/backend/access/table/tableamapi.c
b/src/backend/access/table/tableamapi.c
index f9008164433..8257dc92d03 100644
--- a/src/backend/access/table/tableamapi.c
+++ b/src/backend/access/table/tableamapi.c
@@ -104,6 +104,8 @@ GetTableAmRoutine(Oid amhandler)
(routine->scan_bitmap_next_tuple == NULL));
Assert(routine->scan_sample_next_block != NULL);
Assert(routine->scan_sample_next_tuple != NULL);
+ /* dml_init and dml_fini should both set or ignored */
+ Assert((routine->dml_init != NULL) == (routine->dml_fini != NULL));
return routine;
}
diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c
index 18585a27e35..9cdee90fbe5 100644
--- a/src/backend/commands/copyfrom.c
+++ b/src/backend/commands/copyfrom.c
@@ -2007,18 +2007,7 @@ CopyFrom(CopyFromState cstate)
CopyInitDataParser(cstate);
- /*
- * GPDB_12_MERGE_FIXME: We still have to perform the initialization
- * here for AO relations. It is preferreable by all means to perform the
- * initialization via the table AP API, however it simply does not
- * provide a good enough interface for this yet.
- */
- if (RelationIsAoRows(resultRelInfo->ri_RelationDesc))
- appendonly_dml_init(resultRelInfo->ri_RelationDesc, CMD_INSERT);
- else if (RelationIsAoCols(resultRelInfo->ri_RelationDesc))
- aoco_dml_init(resultRelInfo->ri_RelationDesc, CMD_INSERT);
- else if (ext_dml_init_hook)
- ext_dml_init_hook(resultRelInfo->ri_RelationDesc, CMD_INSERT);
+ table_dml_init(resultRelInfo->ri_RelationDesc, CMD_INSERT);
for (;;)
{
@@ -2451,18 +2440,7 @@ CopyFrom(CopyFromState cstate)
if (bistate != NULL)
FreeBulkInsertState(bistate);
- /*
- * GPDB_12_MERGE_FIXME: We still have to perform the finishment
- * here for AO relations. It is preferreable by all means to perform the
- * finishment via the table AP API, however it simply does not
- * provide a good enough interface for this yet.
- */
- if (RelationIsAoRows(resultRelInfo->ri_RelationDesc))
- appendonly_dml_finish(resultRelInfo->ri_RelationDesc,
CMD_INSERT);
- else if (RelationIsAoCols(resultRelInfo->ri_RelationDesc))
- aoco_dml_finish(resultRelInfo->ri_RelationDesc, CMD_INSERT);
- else if (ext_dml_finish_hook)
- ext_dml_finish_hook(resultRelInfo->ri_RelationDesc, CMD_INSERT);
+ table_dml_fini(resultRelInfo->ri_RelationDesc, CMD_INSERT);
MemoryContextSwitchTo(oldcontext);
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 72db9a51e7c..6822032fe0d 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -838,21 +838,8 @@ CreateIntoRelDestReceiver(IntoClause *intoClause)
static void
intorel_startup_dummy(DestReceiver *self, int operation, TupleDesc typeinfo)
{
- /*
- * In PostgreSQL, this is a no-op, but in GPDB, AO relations do need
some
- * initialization of state, because the tableam API does not provide a
- * good enough interface for handling with this later, we need to
- * specifically all the init at start up.
- */
-
/* See intorel_initplan() for explanation */
-
- if (RelationIsAoRows(((DR_intorel *)self)->rel))
- appendonly_dml_init(((DR_intorel *)self)->rel, CMD_INSERT);
- else if (RelationIsAoCols(((DR_intorel *)self)->rel))
- aoco_dml_init(((DR_intorel *)self)->rel, CMD_INSERT);
- else if (ext_dml_init_hook)
- ext_dml_init_hook(((DR_intorel *)self)->rel, CMD_INSERT);
+ table_dml_init(((DR_intorel *)self)->rel, CMD_INSERT);
}
/*
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index ea3408654c2..1555ea9d334 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -966,12 +966,7 @@ transientrel_startup(DestReceiver *self, int operation,
TupleDesc typeinfo)
myState->bistate = GetBulkInsertState();
myState->processed = 0;
- if (RelationIsAoRows(myState->transientrel))
- appendonly_dml_init(myState->transientrel, CMD_INSERT);
- else if (RelationIsAoCols(myState->transientrel))
- aoco_dml_init(myState->transientrel, CMD_INSERT);
- else if (ext_dml_init_hook)
- ext_dml_init_hook(myState->transientrel, CMD_INSERT);
+ table_dml_init(myState->transientrel, CMD_INSERT);
/*
* Valid smgr_targblock implies something already wrote to the relation.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e731e37142c..42e00efe81d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7560,12 +7560,8 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap,
LOCKMODE lockmode)
snapshot = RegisterSnapshot(GetLatestSnapshot());
scan = table_beginscan(oldrel, snapshot, 0, NULL);
- if (newrel && RelationIsAoRows(newrel))
- appendonly_dml_init(newrel, CMD_INSERT);
- else if (newrel && RelationIsAoCols(newrel))
- aoco_dml_init(newrel, CMD_INSERT);
- else if (newrel && ext_dml_init_hook)
- ext_dml_init_hook(newrel, CMD_INSERT);
+ if (newrel)
+ table_dml_init(newrel, CMD_INSERT);
/*
* Switch to per-tuple memory context and reset it for each
tuple
diff --git a/src/backend/executor/execPartition.c
b/src/backend/executor/execPartition.c
index 679d861e485..629eca05483 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -916,12 +916,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState
*estate,
lappend(estate->es_tuple_routing_result_relations,
leaf_part_rri);
- if (RelationIsAoRows(leaf_part_rri->ri_RelationDesc))
- appendonly_dml_init(leaf_part_rri->ri_RelationDesc,
mtstate->operation);
- else if (RelationIsAoCols(leaf_part_rri->ri_RelationDesc))
- aoco_dml_init(leaf_part_rri->ri_RelationDesc,
mtstate->operation);
- else if (ext_dml_init_hook)
- ext_dml_init_hook(leaf_part_rri->ri_RelationDesc,
mtstate->operation);
+ table_dml_init(leaf_part_rri->ri_RelationDesc, mtstate->operation);
MemoryContextSwitchTo(oldcxt);
@@ -1234,12 +1229,7 @@ ExecCleanupTupleRouting(ModifyTableState *mtstate,
* Only leaf node can have a valid access method. If we find an
* appendoptimized table, ensure the DML operation is finished.
*/
- if (RelationIsAoRows(resultRelInfo->ri_RelationDesc))
- appendonly_dml_finish(resultRelInfo->ri_RelationDesc,
mtstate->operation);
- else if (RelationIsAoCols(resultRelInfo->ri_RelationDesc))
- aoco_dml_finish(resultRelInfo->ri_RelationDesc,
mtstate->operation);
- else if (ext_dml_finish_hook)
- ext_dml_finish_hook(resultRelInfo->ri_RelationDesc,
mtstate->operation);
+ table_dml_fini(resultRelInfo->ri_RelationDesc,
mtstate->operation);
ExecCloseIndices(resultRelInfo);
table_close(resultRelInfo->ri_RelationDesc, NoLock);
diff --git a/src/backend/executor/nodeModifyTable.c
b/src/backend/executor/nodeModifyTable.c
index 1d6bd2e8a93..27b48152195 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -70,9 +70,6 @@
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
-ext_dml_func_hook_type ext_dml_init_hook = NULL;
-ext_dml_func_hook_type ext_dml_finish_hook = NULL;
-
typedef struct MTTargetRelLookup
{
Oid relationOid; /* hash key, must be first */
@@ -3261,12 +3258,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate,
int eflags)
"supported
in serializable transactions")));
}
- if (RelationIsAoRows(resultRelInfo->ri_RelationDesc))
- appendonly_dml_init(resultRelInfo->ri_RelationDesc,
operation);
- else if (RelationIsAoCols(resultRelInfo->ri_RelationDesc))
- aoco_dml_init(resultRelInfo->ri_RelationDesc,
operation);
- else if (ext_dml_init_hook)
- ext_dml_init_hook(resultRelInfo->ri_RelationDesc,
operation);
+ table_dml_init(resultRelInfo->ri_RelationDesc, operation);
resultRelInfo++;
i++;
@@ -3717,13 +3709,7 @@ ExecEndModifyTable(ModifyTableState *node)
resultRelInfo->ri_FdwRoutine->EndForeignModify(node->ps.state,
resultRelInfo);
- if (RelationIsAoRows(resultRelInfo->ri_RelationDesc))
- appendonly_dml_finish(resultRelInfo->ri_RelationDesc,
-
node->operation);
- else if (RelationIsAoCols(resultRelInfo->ri_RelationDesc))
- aoco_dml_finish(resultRelInfo->ri_RelationDesc,
node->operation);
- else if (ext_dml_finish_hook)
- ext_dml_finish_hook(resultRelInfo->ri_RelationDesc,
node->operation);
+ table_dml_fini(resultRelInfo->ri_RelationDesc, node->operation);
/*
* Cleanup the initialized batch slots. This only matters for
FDWs
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 4a877e857b1..2253e400efb 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -39,6 +39,7 @@ struct SampleScanState;
struct TBMIterateResult;
struct VacuumParams;
struct ValidateIndexState;
+enum CmdType;
/**
* Flags represented the supported features of scan.
@@ -955,6 +956,14 @@ typedef struct TableAmRoutine
struct SampleScanState *scanstate,
TupleTableSlot *slot);
+ /**
+ * The pair of callbacks are invoked to perform any initialization and
+ * cleanup required for DML operations (INSERT, UPDATE, DELETE) on a
+ * relation of this table AM.
+ */
+ void (*dml_init) (Relation relation, enum CmdType operation);
+ void (*dml_fini) (Relation relation, enum CmdType operation);
+
/*
* This callback is used to parse reloptions for relation/matview/toast.
*/
@@ -1791,6 +1800,19 @@ table_finish_bulk_insert(Relation rel, int options)
rel->rd_tableam->finish_bulk_insert(rel, options);
}
+static inline void
+table_dml_init(Relation rel, enum CmdType operation)
+{
+ if (rel->rd_tableam && rel->rd_tableam->dml_init)
+ rel->rd_tableam->dml_init(rel, operation);
+}
+
+static inline void
+table_dml_fini(Relation rel, enum CmdType operation)
+{
+ if (rel->rd_tableam && rel->rd_tableam->dml_fini)
+ rel->rd_tableam->dml_fini(rel, operation);
+}
/* ------------------------------------------------------------------------
* DDL related functionality.
@@ -2357,13 +2379,5 @@ extern const TableAmRoutine
*GetHeapamTableAmRoutine(void);
extern bool check_default_table_access_method(char **newval, void **extra,
GucSource source);
-/* ----------------------------------------------------------------------------
- * Hook function to run init/fini for storage extensions
- * ----------------------------------------------------------------------------
- */
-enum CmdType;
-typedef void (*ext_dml_func_hook_type) (Relation relation, enum CmdType
operation);
-extern PGDLLIMPORT ext_dml_func_hook_type ext_dml_init_hook;
-extern PGDLLIMPORT ext_dml_func_hook_type ext_dml_finish_hook;
#endif /* TABLEAM_H */
diff --git a/src/include/cdb/cdbaocsam.h b/src/include/cdb/cdbaocsam.h
index ab990e2bf01..c18f86db1fd 100644
--- a/src/include/cdb/cdbaocsam.h
+++ b/src/include/cdb/cdbaocsam.h
@@ -421,9 +421,6 @@ extern void aocs_addcol_setfirstrownum(AOCSAddColumnDesc
desc,
extern bool aocs_get_target_tuple(AOCSScanDesc aoscan, int64 targrow,
TupleTableSlot *slot);
-extern void aoco_dml_init(Relation relation, CmdType operation);
-extern void aoco_dml_finish(Relation relation, CmdType operation);
-
extern bool extractcolumns_from_node(Node *expr, bool *cols, AttrNumber natts);
extern ExprState * aocs_predicate_pushdown_prepare(AOCSScanDesc scan,
List *qual,
diff --git a/src/include/cdb/cdbappendonlyam.h
b/src/include/cdb/cdbappendonlyam.h
index 58fc77d8f5c..1d2622a8ff2 100644
--- a/src/include/cdb/cdbappendonlyam.h
+++ b/src/include/cdb/cdbappendonlyam.h
@@ -488,14 +488,12 @@ extern bool appendonly_fetch(
AOTupleId *aoTid,
TupleTableSlot *slot);
extern void appendonly_fetch_finish(AppendOnlyFetchDesc aoFetchDesc);
-extern void appendonly_dml_init(Relation relation, CmdType operation);
extern AppendOnlyInsertDesc appendonly_insert_init(Relation rel, int segno);
extern void appendonly_insert(
AppendOnlyInsertDesc aoInsertDesc,
MemTuple instup,
AOTupleId *aoTupleId);
extern void appendonly_insert_finish(AppendOnlyInsertDesc aoInsertDesc,
dlist_head *head);
-extern void appendonly_dml_finish(Relation relation, CmdType operation);
extern AppendOnlyDeleteDesc appendonly_delete_init(Relation rel);
extern TM_Result appendonly_delete(
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
