Обсуждение: Trivial fix for comment of function table_tuple_lock

On Thu, Sep 11, 2025 at 2:40 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> Hi Hackers,
>
> Just fixed an one-word error in comment. Please take a look.

Thanks for the patch! LGTM.

It looks like this happened because heapam_tuple_lock() and heap_lock_tuple()
use the argument relation instead of rel, and the comments for
table_lock_tuple() were probably copied from heap_lock_tuple(),
leaving relation in place by mistake.

By the way, I noticed another typo in the comments for heap_lock_tuple():
the input parameter description for tid ("TID of tuple to lock")
should be removed,
since that function doesn’t take tid. How about fixing both issues in
the same patch,
since there’s no need to commit them separately?

Regards,

--
Fujii Masao

On Mon, Sep 22, 2025 at 2:54 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> Hi Fujii-san,
>
> Thank you very much for taking care of this patch.
>
> I just updated the patch, see attached v2. In v2, I removed "tid" from the comment of heap_lock_tuple(), and I found
acouple of more occurrences of "relation"=>"rel" in tableam.h. 

Thanks for updating the patch! LGTM.

While also checking the comments for table_tuple_delete() and
table_tuple_update(),
I noticed a few other issues. Wouldn’t it be better to fix these together?

* changingPart is listed as an output parameter for table_tuple_delete(),
    but it looks like an input.
* slot is listed as an input parameter for table_tuple_update(), but it seems
    to be an output.
* The comment for update_indexes in table_tuple_update() is mis-indented.
* Not an issue, but it might be clearer to add a blank line between the input
    and output parameter comments in table_tuple_delete() and
    table_tuple_update().

Regards,

--
Fujii Masao

On Tue, Sep 23, 2025 at 7:44 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
> On Mon, Sep 22, 2025 at 11:41 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>>
>>
>> I noticed a few other issues. Wouldn’t it be better to fix these together?
>>
>> * changingPart is listed as an output parameter for table_tuple_delete(),
>>     but it looks like an input.
>
>
> Fixed. changingPart is of type bool, it cannot pass out anything.
>
>>
>> * slot is listed as an input parameter for table_tuple_update(), but it seems
>>     to be an output.
>
>
> Fixed. Yes, slot will get some field assigned inside the function.
>
>>
>> * The comment for update_indexes in table_tuple_update() is mis-indented.
>
>
> Fixed. Other parameters use a tab in front, while update_indexes used two white-space in front.
>
>>
>> * Not an issue, but it might be clearer to add a blank line between the input
>>     and output parameter comments in table_tuple_delete() and
>>     table_tuple_update().
>>
>
> Added new lines.

Thanks for updating the patch! LGTM.
Barring any objections, I'll commit it.

I noticed the commit message in the patch lists you as "Author: Chao
Li <lic@highgo.com>",
but in this discussion you've been using "li.evan.chao@gmail.com",
which also seems to be the address used in past commit logs for you.
Which email address would you like me to use in the commit log?

Regards,

--
Fujii Masao

On Tue, Sep 23, 2025 at 9:51 AM Chao Li <li.evan.chao@gmail.com> wrote:
>
>
>
> On Sep 23, 2025, at 08:45, Fujii Masao <masao.fujii@gmail.com> wrote:
>
>
> I noticed the commit message in the patch lists you as "Author: Chao
> Li <lic@highgo.com>",
> but in this discussion you've been using "li.evan.chao@gmail.com",
> which also seems to be the address used in past commit logs for you.
> Which email address would you like me to use in the commit log?
>
>
> Please use lic@highgo.com that is my company email address. Because the company email address doesn’t work well with
MailArchive, so I use my Gmail for daily community communications. 

Got it. I've pushed the patch and used that email address in the commit log.
Thanks!

Regards,

--
Fujii Masao