This is an automated email from the ASF dual-hosted git repository. maxyang pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 300692f187e92fa4fc8802e2cbc3011ecd9876bf Author: Zhang Mingli <[email protected]> AuthorDate: Thu Jul 24 21:36:32 2025 +0800 Fix handling of large OIDs by introducing HTAB. The original Bitmapset implementation used signed int parameters, which cannot correctly handle Oid values (unsigned int) exceeding INT_MAX. This caused errors when passing certain relation OIDs in edge cases. To fix this, introduce a HATB to record and deduplicate Oids. Authored-by: Zhang Mingli [email protected] --- src/backend/cdb/dispatcher/cdbdisp_query.c | 16 +-- src/backend/executor/execMain.c | 27 ++-- src/backend/executor/nodeModifyTable.c | 219 +++++++++++++++++++++++------ src/include/cdb/cdbdisp_query.h | 6 +- src/include/nodes/execnodes.h | 6 +- src/test/regress/expected/matview_data.out | 26 ++++ src/test/regress/sql/matview_data.sql | 10 ++ 7 files changed, 238 insertions(+), 72 deletions(-) diff --git a/src/backend/cdb/dispatcher/cdbdisp_query.c b/src/backend/cdb/dispatcher/cdbdisp_query.c index 99f5179e756..ef4b84f0db0 100644 --- a/src/backend/cdb/dispatcher/cdbdisp_query.c +++ b/src/backend/cdb/dispatcher/cdbdisp_query.c @@ -138,7 +138,7 @@ static SerializedParams *serializeParamsForDispatch(QueryDesc *queryDesc, ParamExecData *execParams, Bitmapset *sendParams); -static Bitmapset* process_epd_iud_internal(List* subtagdata); +static List* process_epd_iud_internal(List* subtagdata); /* * Compose and dispatch the MPPEXEC commands corresponding to a plan tree @@ -1729,7 +1729,7 @@ ConsumeExtendProtocolData(ExtendProtocolSubTag subtag) } void -ConsumeAndProcessExtendProtocolData_IUD(Bitmapset **inserted, Bitmapset **updated, Bitmapset **deleted) +ConsumeAndProcessExtendProtocolData_IUD(List **inserted, List **updated, List **deleted) { List *ilist = NIL; List *ulist = NIL; @@ -1749,10 +1749,10 @@ ConsumeAndProcessExtendProtocolData_IUD(Bitmapset **inserted, Bitmapset **update return; } -static Bitmapset* +static List* process_epd_iud_internal(List* subtagdata) { - Bitmapset *res = NULL; + List *res = NIL; ListCell *lc; int count; @@ -1763,10 +1763,10 @@ process_epd_iud_internal(List* subtagdata) data += sizeof(int); for (int i = 0; i < count; i++) { - int relid; - memcpy(&relid, data, sizeof(int)); - data += sizeof(int); - res = bms_add_member(res, relid); + Oid relid; + memcpy(&relid, data, sizeof(Oid)); + data += sizeof(Oid); + res = list_append_unique_oid(res, relid); } } return res; diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index b3bd749dadc..cef77dcccf4 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -4279,40 +4279,39 @@ already_under_executor_run(void) static void MaintainMaterializedViewStatus(QueryDesc *queryDesc, CmdType operation) { - Bitmapset *inserted = NULL; - Bitmapset *updated = NULL; - Bitmapset *deleted = NULL; + List *inserted = NULL; + List *updated = NULL; + List *deleted = NULL; List *unique_result_relations = NIL; List *rtable = queryDesc->plannedstmt->rtable; int length = list_length(rtable); ListCell *lc; - int relid = -1; + Oid relid; /* * Process epd first to get the affected relations. */ ConsumeAndProcessExtendProtocolData_IUD(&inserted, &updated, &deleted); - relid = -1; - while((relid = bms_next_member(inserted, relid)) >= 0) + foreach (lc, inserted) { + relid = lfirst_oid(lc); /* Only need to transfer to UP direction. */ SetRelativeMatviewAuxStatus(relid, MV_DATA_STATUS_EXPIRED_INSERT_ONLY, MV_DATA_STATUS_TRANSFER_DIRECTION_UP); - } - relid = -1; - while((relid = bms_next_member(updated, relid)) >= 0) + foreach (lc, updated) { + relid = lfirst_oid(lc); SetRelativeMatviewAuxStatus(relid, MV_DATA_STATUS_EXPIRED, MV_DATA_STATUS_TRANSFER_DIRECTION_UP); } - relid = -1; - while((relid = bms_next_member(deleted, relid)) >= 0) + foreach (lc, deleted) { + relid = lfirst_oid(lc); SetRelativeMatviewAuxStatus(relid, MV_DATA_STATUS_EXPIRED, MV_DATA_STATUS_TRANSFER_DIRECTION_UP); @@ -4338,9 +4337,9 @@ MaintainMaterializedViewStatus(QueryDesc *queryDesc, CmdType operation) * Do a second check and fall back to partitioned table * in case that if we failed to find a one. */ - if (bms_is_empty(inserted) && - bms_is_empty(updated) && - bms_is_empty(deleted)) + if ((list_length(inserted) == 0) && + (list_length(updated) == 0) && + (list_length(deleted) == 0)) { ereport(WARNING, (errmsg("fail to find leafs of partitioned table: %s", get_rel_name(rte->relid)))); diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 3247c0cf2f1..dc12ffb47c5 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -61,7 +61,9 @@ #include "cdb/cdbhash.h" #include "cdb/cdbpq.h" #include "cdb/cdbvars.h" +#include "common/hashfn.h" /* hash_any */ #include "parser/parsetree.h" +#include "utils/hsearch.h" /* hash_destroy */ #include "utils/lsyscache.h" #include "utils/snapmgr.h" @@ -99,8 +101,43 @@ static TupleTableSlot *ExecPrepareTupleRouting(ModifyTableState *mtstate, TupleTableSlot *slot, ResultRelInfo **partRelInfo); +typedef struct ModifiedLeafRelidsKey +{ + CmdType cmd; + Oid relid; + +} ModifiedLeafRelidsKey; + +typedef struct ModifiedLeafRelidsData +{ + ModifiedLeafRelidsKey key; +} ModifiedLeafRelidsData; + +static uint32 +modified_leaf_hash(const void *key, Size keysize) +{ + Assert(keysize == sizeof(ModifiedLeafRelidsKey)); + return DatumGetUInt32(hash_any((const unsigned char*) key, + keysize)); +} + +static int +modified_leaf_compare(const void *key1, const void *key2, Size keysize) +{ + Assert(keysize == sizeof(ModifiedLeafRelidsKey)); + const ModifiedLeafRelidsKey *k1 = (ModifiedLeafRelidsKey*) key1; + const ModifiedLeafRelidsKey *k2 = (ModifiedLeafRelidsKey*) key2; + + if ((k1->cmd == k2->cmd) && + (k1->relid == k2->relid)) + { + return 0; + } + return 1; +} + static void -send_subtag(StringInfoData *buf, ExtendProtocolSubTag subtag, Bitmapset* relids); +send_subtag(StringInfoData *buf, ExtendProtocolSubTag subtag, List *relids); static void notify_modified_relations_to_QD(ModifyTableState *node); @@ -109,7 +146,7 @@ static void notify_modified_relations_local(ModifyTableState *node); static void -epd_add_subtag_data(ExtendProtocolSubTag subtag, Bitmapset *relids); +epd_add_subtag_data(ExtendProtocolSubTag subtag, List *relids); /* * Verify that the tuples to be produced by INSERT match the @@ -1052,9 +1089,12 @@ ExecInsert(ModifyTableState *mtstate, if (resultRelationDesc->rd_rel->relispartition) { - mtstate->mt_leaf_relids_inserted = - bms_add_member(mtstate->mt_leaf_relids_inserted, RelationGetRelid(resultRelationDesc)); - mtstate->has_leaf_changed = true; + ModifiedLeafRelidsKey key; + + key.cmd = CMD_INSERT; + key.relid = RelationGetRelid(resultRelationDesc); + + (void) hash_search(mtstate->modified_leaf_relids, &key, HASH_ENTER, NULL); } /* @@ -1520,10 +1560,12 @@ ldelete:; if (resultRelationDesc->rd_rel->relispartition) { + ModifiedLeafRelidsKey key; + + key.cmd = CMD_DELETE; + key.relid = RelationGetRelid(resultRelationDesc); - mtstate->mt_leaf_relids_deleted = - bms_add_member(mtstate->mt_leaf_relids_deleted, RelationGetRelid(resultRelationDesc)); - mtstate->has_leaf_changed = true; + (void) hash_search(mtstate->modified_leaf_relids, &key, HASH_ENTER, NULL); } /* Tell caller that the delete actually happened. */ @@ -2170,9 +2212,12 @@ lreplace:; if (resultRelationDesc->rd_rel->relispartition) { - mtstate->mt_leaf_relids_updated = - bms_add_member(mtstate->mt_leaf_relids_updated, RelationGetRelid(resultRelationDesc)); - mtstate->has_leaf_changed = true; + ModifiedLeafRelidsKey key; + + key.cmd = CMD_UPDATE; + key.relid = RelationGetRelid(resultRelationDesc); + + (void) hash_search(mtstate->modified_leaf_relids, &key, HASH_ENTER, NULL); } /* AFTER ROW UPDATE Triggers */ @@ -3088,6 +3133,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) ListCell *l; int i; Relation rel; + HASHCTL hash_ctl; /* check for unsupported flags */ Assert(!(eflags & (EXEC_FLAG_BACKWARD | EXEC_FLAG_MARK))); @@ -3100,10 +3146,16 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) mtstate->ps.state = estate; mtstate->ps.ExecProcNode = ExecModifyTable; - mtstate->mt_leaf_relids_inserted = NULL; - mtstate->mt_leaf_relids_updated = NULL; - mtstate->mt_leaf_relids_deleted = NULL; - mtstate->has_leaf_changed = false; + MemSet(&hash_ctl, 0, sizeof(hash_ctl)); + hash_ctl.keysize = sizeof(ModifiedLeafRelidsKey); + hash_ctl.entrysize = sizeof(ModifiedLeafRelidsData); + hash_ctl.hash = modified_leaf_hash; + hash_ctl.match = modified_leaf_compare; + hash_ctl.hcxt = CurrentMemoryContext; + mtstate->modified_leaf_relids = hash_create("ModifiedLeafRelids", + 4, + &hash_ctl, + HASH_ELEM | HASH_FUNCTION | HASH_COMPARE); mtstate->operation = operation; mtstate->canSetTag = node->canSetTag; @@ -3716,8 +3768,11 @@ ExecEndModifyTable(ModifyTableState *node) ExecEndNode(outerPlanState(node)); /* Notify modified leaf relids to QD */ - if (GP_ROLE_EXECUTE == Gp_role && node->has_leaf_changed) + if (GP_ROLE_EXECUTE == Gp_role && + hash_get_num_entries(node->modified_leaf_relids) > 0) notify_modified_relations_to_QD(node); + + hash_destroy(node->modified_leaf_relids); } void @@ -3764,16 +3819,52 @@ static void notify_modified_relations_to_QD(ModifyTableState *node) { StringInfoData buf; + HASH_SEQ_STATUS scan; + ModifiedLeafRelidsData *r; + List *inserted = NIL; + List *updated = NIL; + List *deleted = NIL; + + hash_seq_init(&scan, node->modified_leaf_relids); + pq_beginmessage(&buf, PQExtendProtocol); - if (!bms_is_empty(node->mt_leaf_relids_inserted)) - send_subtag(&buf, EP_TAG_I, node->mt_leaf_relids_inserted); + while ((r = (ModifiedLeafRelidsData *) hash_seq_search(&scan)) != NULL) + { + switch (r->key.cmd) + { + case CMD_INSERT: + inserted = lappend_oid(inserted, r->key.relid); + break; + case CMD_UPDATE: + updated = lappend_oid(updated, r->key.relid); + break; + case CMD_DELETE: + deleted = lappend_oid(deleted, r->key.relid); + break; + default: + Assert(false); + break; + } + } + + if (inserted != NIL) + { + send_subtag(&buf, EP_TAG_I, inserted); + pfree(inserted); + } - if (!bms_is_empty(node->mt_leaf_relids_updated)) - send_subtag(&buf, EP_TAG_U, node->mt_leaf_relids_updated); + if (updated != NIL) + { + send_subtag(&buf, EP_TAG_U, updated); + pfree(updated); + } - if (!bms_is_empty(node->mt_leaf_relids_deleted)) - send_subtag(&buf, EP_TAG_D, node->mt_leaf_relids_deleted); + if (deleted != NIL) + { + send_subtag(&buf, EP_TAG_D, deleted); + pfree(deleted); + } pq_sendint32(&buf, EP_TAG_MAX); /* Finish this run. */ pq_endmessage(&buf); @@ -3787,19 +3878,19 @@ notify_modified_relations_to_QD(ModifyTableState *node) * while length is the length of data followed. */ static void -send_subtag(StringInfoData *buf, ExtendProtocolSubTag subtag, Bitmapset* relids) +send_subtag(StringInfoData *buf, ExtendProtocolSubTag subtag, List *relids) { - bytea *res; int rlen; char *ptr; int rcount; - int relid = -1; + Oid relid; + ListCell *lc; pq_sendint32(buf, subtag); /* subtag */ - rcount = bms_num_members(relids); - rlen = sizeof(int)/* count of relids */ + sizeof(int) * rcount; + rcount = list_length(relids); + rlen = sizeof(int)/* count of relids */ + sizeof(Oid) * rcount; pq_sendint32(buf, rlen); /* length */ @@ -3808,10 +3899,12 @@ send_subtag(StringInfoData *buf, ExtendProtocolSubTag subtag, Bitmapset* relids) memcpy(ptr, &rcount, sizeof(int)); ptr += sizeof(int); - while ((relid = bms_next_member(relids, relid)) >= 0) + + foreach(lc, relids) { - memcpy(ptr, &relid, sizeof(int)); - ptr += sizeof(int); + relid = lfirst_oid(lc); + memcpy(ptr, &relid, sizeof(Oid)); + ptr += sizeof(Oid); } SET_VARSIZE(res, rlen + VARHDRSZ); @@ -3830,14 +3923,50 @@ notify_modified_relations_local(ModifyTableState *node) { Assert(epd); - if (!bms_is_empty(node->mt_leaf_relids_inserted)) - epd_add_subtag_data(EP_TAG_I, node->mt_leaf_relids_inserted); + HASH_SEQ_STATUS scan; + ModifiedLeafRelidsData *r; + List *inserted = NIL; + List *updated = NIL; + List *deleted = NIL; - if (!bms_is_empty(node->mt_leaf_relids_updated)) - epd_add_subtag_data(EP_TAG_U, node->mt_leaf_relids_updated); + hash_seq_init(&scan, node->modified_leaf_relids); - if (!bms_is_empty(node->mt_leaf_relids_deleted)) - epd_add_subtag_data(EP_TAG_D, node->mt_leaf_relids_deleted); + while ((r = (ModifiedLeafRelidsData *) hash_seq_search(&scan)) != NULL) + { + switch (r->key.cmd) + { + case CMD_INSERT: + inserted = lappend_oid(inserted, r->key.relid); + break; + case CMD_UPDATE: + updated = lappend_oid(updated, r->key.relid); + break; + case CMD_DELETE: + deleted = lappend_oid(deleted, r->key.relid); + break; + default: + Assert(false); + break; + } + } + + if (inserted != NIL) + { + epd_add_subtag_data(EP_TAG_I, inserted); + pfree(inserted); + } + + if (updated != NIL) + { + epd_add_subtag_data(EP_TAG_U, updated); + pfree(updated); + } + + if (deleted != NIL) + { + epd_add_subtag_data(EP_TAG_D, deleted); + pfree(deleted); + } } /* @@ -3849,7 +3978,7 @@ notify_modified_relations_local(ModifyTableState *node) * are performed under the TopTransactionContext to ensure proper memory management. */ static void -epd_add_subtag_data(ExtendProtocolSubTag subtag, Bitmapset * relids) +epd_add_subtag_data(ExtendProtocolSubTag subtag, List *relids) { MemoryContext oldctx; StringInfo buf; @@ -3857,20 +3986,24 @@ epd_add_subtag_data(ExtendProtocolSubTag subtag, Bitmapset * relids) int rlen; char *ptr; int rcount; - int relid = -1; + Oid relid; + ListCell *lc; - rcount = bms_num_members(relids); - rlen = sizeof(int) /* count of relids */ + sizeof(int) * rcount; + rcount = list_length(relids); + rlen = sizeof(int) /* count of relids */ + sizeof(Oid) * rcount; res = palloc(rlen + VARHDRSZ); ptr = VARDATA(res); memcpy(ptr, &rcount, sizeof(int)); ptr += sizeof(int); - while ((relid = bms_next_member(relids, relid)) >= 0) + + foreach(lc, relids) { - memcpy(ptr, &relid, sizeof(int)); - ptr += sizeof(int); + relid = lfirst_oid(lc); + memcpy(ptr, &relid, sizeof(Oid)); + ptr += sizeof(Oid); } + SET_VARSIZE(res, rlen + VARHDRSZ); oldctx = MemoryContextSwitchTo(TopTransactionContext); diff --git a/src/include/cdb/cdbdisp_query.h b/src/include/cdb/cdbdisp_query.h index b62368dfdff..f8f8ed7b0e6 100644 --- a/src/include/cdb/cdbdisp_query.h +++ b/src/include/cdb/cdbdisp_query.h @@ -140,9 +140,9 @@ extern ExtendProtocolData epd; extern List* ConsumeExtendProtocolData(ExtendProtocolSubTag subtag); extern void -ConsumeAndProcessExtendProtocolData_IUD(Bitmapset **inserted, - Bitmapset **updated, - Bitmapset **deleted); +ConsumeAndProcessExtendProtocolData_IUD(List **inserted, + List **updated, + List **deleted); extern void AtEOXact_ExtendProtocolData(void); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index ede431f9ff3..63a4b212bfe 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1357,10 +1357,8 @@ typedef struct ModifyTableState struct TransitionCaptureState *mt_oc_transition_capture; /* Record modified leaf relation(s) */ - bool has_leaf_changed; - Bitmapset *mt_leaf_relids_inserted; - Bitmapset *mt_leaf_relids_updated; - Bitmapset *mt_leaf_relids_deleted; + HTAB *modified_leaf_relids; + } ModifyTableState; /* ---------------- diff --git a/src/test/regress/expected/matview_data.out b/src/test/regress/expected/matview_data.out index dc7aaa8aea5..14ffe538c55 100644 --- a/src/test/regress/expected/matview_data.out +++ b/src/test/regress/expected/matview_data.out @@ -893,6 +893,32 @@ select count(*) from gp_matview_aux where mvname = 'mv_name2'; (1 row) abort; +-- start_ignore +CREATE EXTENSION IF NOT EXISTS gp_inject_fault; +NOTICE: extension "gp_inject_fault" already exists, skipping +-- end_ignore +create table par1(a int, b int) partition by range(a) using ao_row distributed randomly; +select gp_inject_fault('bump_oid', 'skip', dbid) from gp_segment_configuration where role = 'p' and content = -1; + gp_inject_fault +----------------- + Success: +(1 row) + +create table sub_par1 partition of par1 for values from (1) to (2) using ao_row; +NOTICE: table has parent, setting distribution columns to match parent table +select 'sub_par1'::regclass::oid > x'7FFFFFFF'::bigint; + ?column? +---------- + t +(1 row) + +select gp_inject_fault('bump_oid', 'reset', dbid) from gp_segment_configuration where role = 'p' and content = -1; + gp_inject_fault +----------------- + Success: +(1 row) + +insert into par1 values(1, 2); --start_ignore drop schema matview_data_schema cascade; NOTICE: drop cascades to 3 other objects diff --git a/src/test/regress/sql/matview_data.sql b/src/test/regress/sql/matview_data.sql index 7c4e85ee582..7b6759477a2 100644 --- a/src/test/regress/sql/matview_data.sql +++ b/src/test/regress/sql/matview_data.sql @@ -344,6 +344,16 @@ select count(*) from gp_matview_aux where mvname = 'mv_name1'; select count(*) from gp_matview_aux where mvname = 'mv_name2'; abort; +-- start_ignore +CREATE EXTENSION IF NOT EXISTS gp_inject_fault; +-- end_ignore +create table par1(a int, b int) partition by range(a) using ao_row distributed randomly; +select gp_inject_fault('bump_oid', 'skip', dbid) from gp_segment_configuration where role = 'p' and content = -1; +create table sub_par1 partition of par1 for values from (1) to (2) using ao_row; +select 'sub_par1'::regclass::oid > x'7FFFFFFF'::bigint; +select gp_inject_fault('bump_oid', 'reset', dbid) from gp_segment_configuration where role = 'p' and content = -1; +insert into par1 values(1, 2); + --start_ignore drop schema matview_data_schema cascade; --end_ignore --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
