From: | Japin Li <japinli(at)hotmail(dot)com> |
---|---|
To: | Aleksander Alekseev <aleksander(at)timescale(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Unnecessary smgropen in {heapam_relation,index}_copy_data? |
Date: | 2024-01-25 15:22:41 |
Message-ID: | ME3P282MB3166D290B759D59F62AD4B68B67A2@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, 25 Jan 2024 at 21:43, Aleksander Alekseev <aleksander(at)timescale(dot)com> wrote:
> Hi,
>
>> I find heapam_relation_copy_data() and index_copy_data() have the following code:
>>
>> dstrel = smgropen(*newrlocator, rel->rd_backend);
>>
>> ...
>>
>> RelationCreateStorage(*newrlocator, rel->rd_rel->relpersistence, true);
>>
>> The smgropen() is also called by RelationCreateStorage(), why should we call
>> smgropen() explicitly here?
>>
>> I try to remove the smgropen(), and all tests passed.
>
> That's a very good question. Note that the second argument of
> smgropen() used to create dstrel changes after applying your patch.
> I'm not 100% sure whether this is significant or not.
>
Thanks for the review.
According the comments of RelationData->rd_backend, it is the backend id, if
the relation is temporary. The differnece is RelationCreateStorage() uses
relpersistence to determinate the backend id.
> I added your patch to the nearest open commitfest so that we will not lose it:
>
> https://commitfest.postgresql.org/47/4794/
Thank you.
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2024-01-25 15:28:06 | Re: A compiling warning in jsonb_populate_record_valid |
Previous Message | Robert Haas | 2024-01-25 15:19:28 | Re: Emit fewer vacuum records by reaping removable tuples during pruning |