From 439870f3a95e2227525a8ba5687f1221f1cb7fc2 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. --- 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; }