Re: Fix parameters order for relation_copy_for_cluster - Mailing list pgsql-hackers
| From | Pavel Borisov |
|---|---|
| Subject | Re: Fix parameters order for relation_copy_for_cluster |
| Date | |
| Msg-id | CALT9ZEGPcVjZNAii7QVuE_yZsADFLXPzvtZj-CTLiOdi2u-CHg@mail.gmail.com Whole thread Raw |
| In response to | [MASSMAIL]Fix parameters order for relation_copy_for_cluster (Japin Li <japinli@hotmail.com>) |
| Responses |
Re: Fix parameters order for relation_copy_for_cluster
|
| List | pgsql-hackers |
Hi, Japin!
On Mon, 1 Apr 2024 at 12:15, Japin Li <japinli@hotmail.com> wrote:
Hi,
When attempting to implement a new table access method, I discovered that
relation_copy_for_cluster() has the following declaration:
void (*relation_copy_for_cluster) (Relation NewTable,
Relation OldTable,
Relation OldIndex,
bool use_sort,
TransactionId OldestXmin,
TransactionId *xid_cutoff,
MultiXactId *multi_cutoff,
double *num_tuples,
double *tups_vacuumed,
double *tups_recently_dead);
It claims that the first parameter is a new table, and the second one is an
old table. However, the table_relation_copy_for_cluster() uses the first
parameter as the old table, and the second as a new table, see below:
static inline void
table_relation_copy_for_cluster(Relation OldTable, Relation NewTable,
Relation OldIndex,
bool use_sort,
TransactionId OldestXmin,
TransactionId *xid_cutoff,
MultiXactId *multi_cutoff,
double *num_tuples,
double *tups_vacuumed,
double *tups_recently_dead)
{
OldTable->rd_tableam->relation_copy_for_cluster(OldTable, NewTable, OldIndex,
use_sort, OldestXmin,
xid_cutoff, multi_cutoff,
num_tuples, tups_vacuumed,
tups_recently_dead);
}
It's a bit confusing, so attach a patch to fix this.
I've looked into your patch. All callers of *_relation_copy_for_cluster now use Relation OldTable, Relation NewTable order. It coincides to what is expected by the function, no now code is not broken. The only wrong thing is naming of arguments in declaration of this function in tableam.h I think this is a minor oversight from original commit d25f519107b
Provided all the above I'd recommend committing this catch. This is for clarity only, no changes in code behavior.
Thank you for finding this!
Best regards,
Pavel Borisov
Supabase
pgsql-hackers by date: