Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Make more use of RELATION_IS_OTHER_TEMP().

  1. Use RELATION_IS_OTHER_TEMP where possible

    Junwang Zhao <zhjwpku@gmail.com> — 2025-06-10T16:07:35Z

    Hi hackers,

While reviewing the Merge/Split partitions patch[0], I found
some places in tablecmds.c where RELATION_IS_OTHER_TEMP
can be used but not, such as:

/* If the parent is temp, it must belong to this session */
if (rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
!rel->rd_islocaltemp)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot attach as partition of temporary relation of another session")));

/* Ditto for the partition */
if (attachrel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
!attachrel->rd_islocaltemp)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot attach temporary relation of another session as partition")));

All other files perform this check using RELATION_IS_OTHER_TEMP.
Should we update tablecmds.c to do the same for consistency?

[0] https://www.postgresql.org/message-id/884bcf9e-d6e3-4b0c-8dcb-7f2110070ac9@postgrespro.ru

-- 
Regards
Junwang Zhao

  2. Re: Use RELATION_IS_OTHER_TEMP where possible

    Nathan Bossart <nathandbossart@gmail.com> — 2025-06-10T16:30:35Z

    On Wed, Jun 11, 2025 at 12:07:35AM +0800, Junwang Zhao wrote:
> All other files perform this check using RELATION_IS_OTHER_TEMP.
> Should we update tablecmds.c to do the same for consistency?

Seems like a good idea.

-- 
nathan

  3. Re: Use RELATION_IS_OTHER_TEMP where possible

    Junwang Zhao <zhjwpku@gmail.com> — 2025-06-10T23:41:54Z

    Hi Nathan,

On Wed, Jun 11, 2025 at 12:30 AM Nathan Bossart
<nathandbossart@gmail.com> wrote:
>
> On Wed, Jun 11, 2025 at 12:07:35AM +0800, Junwang Zhao wrote:
> > All other files perform this check using RELATION_IS_OTHER_TEMP.
> > Should we update tablecmds.c to do the same for consistency?
>
> Seems like a good idea.

Thanks for the comment.

I have attached a patch with the proposed changes.

>
> --
> nathan



-- 
Regards
Junwang Zhao

  4. Re: Use RELATION_IS_OTHER_TEMP where possible

    Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2025-06-11T11:10:49Z

    On Wed, Jun 11, 2025 at 5:12 AM Junwang Zhao <zhjwpku@gmail.com> wrote:

> Hi Nathan,
>
> On Wed, Jun 11, 2025 at 12:30 AM Nathan Bossart
> <nathandbossart@gmail.com> wrote:
> >
> > On Wed, Jun 11, 2025 at 12:07:35AM +0800, Junwang Zhao wrote:
> > > All other files perform this check using RELATION_IS_OTHER_TEMP.
> > > Should we update tablecmds.c to do the same for consistency?
> >
> > Seems like a good idea.
>
> Thanks for the comment.
>
> I have attached a patch with the proposed changes.
>

LGTM. I looked at other instances of is_localtemp, but none of them
have (relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP there. Your
patch has covered all the existing ones.

-- 
Best Wishes,
Ashutosh Bapat

  5. Re: Use RELATION_IS_OTHER_TEMP where possible

    Junwang Zhao <zhjwpku@gmail.com> — 2025-06-11T11:35:39Z

    Hi Ashutosh,

On Wed, Jun 11, 2025 at 7:11 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
>
>
> On Wed, Jun 11, 2025 at 5:12 AM Junwang Zhao <zhjwpku@gmail.com> wrote:
>>
>> Hi Nathan,
>>
>> On Wed, Jun 11, 2025 at 12:30 AM Nathan Bossart
>> <nathandbossart@gmail.com> wrote:
>> >
>> > On Wed, Jun 11, 2025 at 12:07:35AM +0800, Junwang Zhao wrote:
>> > > All other files perform this check using RELATION_IS_OTHER_TEMP.
>> > > Should we update tablecmds.c to do the same for consistency?
>> >
>> > Seems like a good idea.
>>
>> Thanks for the comment.
>>
>> I have attached a patch with the proposed changes.
>
>
> LGTM. I looked at other instances of is_localtemp, but none of them have (relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP there. Your patch has covered all the existing ones.

Thank you for the review and the additional check.

I have created a cf entry to track this.

https://commitfest.postgresql.org/patch/5815/

>
> --
> Best Wishes,
> Ashutosh Bapat



-- 
Regards
Junwang Zhao

  6. Re: Use RELATION_IS_OTHER_TEMP where possible

    Nathan Bossart <nathandbossart@gmail.com> — 2025-06-12T19:53:38Z

    On Wed, Jun 11, 2025 at 07:35:39PM +0800, Junwang Zhao wrote:
> I have created a cf entry to track this.
> 
> https://commitfest.postgresql.org/patch/5815/

I'll plan on committing this once v19 development begins.

-- 
nathan

  7. Re: Use RELATION_IS_OTHER_TEMP where possible

    Junwang Zhao <zhjwpku@gmail.com> — 2025-06-13T01:54:59Z

    On Fri, Jun 13, 2025 at 3:53 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Wed, Jun 11, 2025 at 07:35:39PM +0800, Junwang Zhao wrote:
> > I have created a cf entry to track this.
> >
> > https://commitfest.postgresql.org/patch/5815/
>
> I'll plan on committing this once v19 development begins.

Sure, I just checked the CF entry, thanks for taking care of it.

>
> --
> nathan



-- 
Regards
Junwang Zhao

  8. Re: Use RELATION_IS_OTHER_TEMP where possible

    Nathan Bossart <nathandbossart@gmail.com> — 2025-07-02T17:34:23Z

    Committed.

-- 
nathan