From 2ba22104eb3b0a542abc4df2bde4c69a0c322b48 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Fri, 22 Aug 2025 14:46:06 +0300 Subject: [PATCH] Fix incorrect usage of TupleDescSize() in #7950, #8120, #8124, #8121 and #8114 (#8146) In #7950, #8120, #8124, #8121 and #8114, TupleDescSize() was used to check whether the tuple length is `Natts_`. However this was wrong because TupleDescSize() returns the size of the tupledesc, not the length of it (i.e., number of attributes). Actually `TupleDescSize(tupleDesc) == Natts_` was always returning false but this didn't cause any problems because using `tupleDesc->natts - 1` when `tupleDesc->natts == Natts_` too had the same effect as using `Anum_ - 1` in that case. So this also makes me thinking of always returning `tupleDesc->natts - 1` (or `tupleDesc->natts - 2` if it's the second to last attribute) but being more explicit seems more useful. Even more, in the future we should probably switch to a different implementation if / when we think of adding more columns to those tables. We should probably scan non-dropped attributes of the relation, enumerate them and return the attribute number of the one that we're looking for, but seems this is not needed right now. (cherry picked from commit 439870f3a95e2227525a8ba5687f1221f1cb7fc2) --- CHANGELOG.md | 2 +- src/backend/columnar/columnar_metadata.c | 2 +- src/backend/distributed/metadata/distobject.c | 2 +- src/backend/distributed/metadata/metadata_utility.c | 4 ++-- src/backend/distributed/transaction/transaction_recovery.c | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72e765f2d..a63d6542e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ * Fixes potential memory corruptions that could happen when accessing various catalog tables after a Citus downgrade is followed by a Citus - upgrade (#7950, #8120, #8124, #8121, #8114) + upgrade (#7950, #8120, #8124, #8121, #8114, #8146) * Fixes UPDATE statements with indirection and array/jsonb subscripting with more than one field (#7675) diff --git a/src/backend/columnar/columnar_metadata.c b/src/backend/columnar/columnar_metadata.c index ef895d3b2..e9f577690 100644 --- a/src/backend/columnar/columnar_metadata.c +++ b/src/backend/columnar/columnar_metadata.c @@ -2132,7 +2132,7 @@ GetHighestUsedRowNumber(uint64 storageId) static int GetFirstRowNumberAttrIndexInColumnarStripe(TupleDesc tupleDesc) { - return TupleDescSize(tupleDesc) == Natts_columnar_stripe + return tupleDesc->natts == Natts_columnar_stripe ? (Anum_columnar_stripe_first_row_number - 1) : tupleDesc->natts - 1; } diff --git a/src/backend/distributed/metadata/distobject.c b/src/backend/distributed/metadata/distobject.c index bc23e7163..8687a9919 100644 --- a/src/backend/distributed/metadata/distobject.c +++ b/src/backend/distributed/metadata/distobject.c @@ -801,7 +801,7 @@ DistributedSequenceList(void) int GetForceDelegationAttrIndexInPgDistObject(TupleDesc tupleDesc) { - return TupleDescSize(tupleDesc) == Natts_pg_dist_object + return tupleDesc->natts == Natts_pg_dist_object ? (Anum_pg_dist_object_force_delegation - 1) : tupleDesc->natts - 1; } diff --git a/src/backend/distributed/metadata/metadata_utility.c b/src/backend/distributed/metadata/metadata_utility.c index b84260f9e..1fb3d6fd0 100644 --- a/src/backend/distributed/metadata/metadata_utility.c +++ b/src/backend/distributed/metadata/metadata_utility.c @@ -4486,7 +4486,7 @@ UnblockDependingBackgroundTasks(BackgroundTask *task) int GetAutoConvertedAttrIndexInPgDistPartition(TupleDesc tupleDesc) { - return TupleDescSize(tupleDesc) == Natts_pg_dist_partition + return tupleDesc->natts == Natts_pg_dist_partition ? (Anum_pg_dist_partition_autoconverted - 1) : tupleDesc->natts - 1; } @@ -4506,7 +4506,7 @@ GetAutoConvertedAttrIndexInPgDistPartition(TupleDesc tupleDesc) int GetNodesInvolvedAttrIndexInPgDistBackgroundTask(TupleDesc tupleDesc) { - return TupleDescSize(tupleDesc) == Natts_pg_dist_background_task + return tupleDesc->natts == Natts_pg_dist_background_task ? (Anum_pg_dist_background_task_nodes_involved - 1) : tupleDesc->natts - 1; } diff --git a/src/backend/distributed/transaction/transaction_recovery.c b/src/backend/distributed/transaction/transaction_recovery.c index 1f8ba949e..260e91b56 100644 --- a/src/backend/distributed/transaction/transaction_recovery.c +++ b/src/backend/distributed/transaction/transaction_recovery.c @@ -685,7 +685,7 @@ DeleteWorkerTransactions(WorkerNode *workerNode) int GetOuterXidAttrIndexInPgDistTransaction(TupleDesc tupleDesc) { - return TupleDescSize(tupleDesc) == Natts_pg_dist_transaction + return tupleDesc->natts == Natts_pg_dist_transaction ? (Anum_pg_dist_transaction_outerxid - 1) : tupleDesc->natts - 1; }