Re: Global temporary tables - Mailing list pgsql-hackers

On Thu, 2 Jul 2026 at 10:11, Japin Li <japinli@hotmail.com> wrote:
>
> Here is my review on v5-0002.  Please take a look.
>

Thanks again for reviewing. I've addressed these comments in v9.

> 8.
> +               /* Register the ON COMMIT action for relation, if it's DELETE ROWS */
> +               Assert(relation->rd_rel->reloncommit == RELONCOMMIT_PRESERVE_ROWS ||
> +                          relation->rd_rel->reloncommit == RELONCOMMIT_DELETE_ROWS);
> +
> +               if (relation->rd_rel->reloncommit == RELONCOMMIT_DELETE_ROWS)
> +                       register_on_commit_action(relation->rd_id, ONCOMMIT_DELETE_ROWS);
>
> How about moving the comment to just before the if statement?

I left this comment where it was, but changed it to also explain the Assert().

> 12.
> +List *
> +AllGlobalTempRelationsInUse(Oid dbid)
>
> How about GetGlobalTempReplationsInUse()?

I went with GetAllGlobalTempRelationsInUse(), because I think the
"All" part is important to emphasise that it returns all GTRs in use
by all backends.

> 19.
>                         if (pkrel->rd_rel->relpersistence != RELPERSISTENCE_TEMP)
>                                 ereport(ERROR,
>                                                 (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
> -                                                errmsg("constraints on temporary tables may reference only temporary
tables")));
> +                                                errmsg("constraints on local temporary tables may reference only
localtemporary tables")));
 
>                         if (!pkrel->rd_islocaltemp || !rel->rd_islocaltemp)
>                                 ereport(ERROR,
>                                                 (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
> -                                                errmsg("constraints on temporary tables must involve temporary
tablesof this session")));
 
> +                                                errmsg("constraints on local temporary tables must involve local
temporarytables of this session")));
 
> +                       break;
> +               case RELPERSISTENCE_GLOBAL_TEMP:
> +                       if (!RELATION_IS_GLOBAL_TEMP(pkrel))
> +                               ereport(ERROR,
> +                                               (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
> +                                                errmsg("constraints on global temporary tables may reference only
globaltemporary tables")));
 
>
> s/global temporary table/global temporary relation/g
> s/local temporary table/local temporary relation/g

I kept these as they were to match the existing error messages. I
think "table" is probably more appropriate than "relation" for the
majority of the error messages in tablecmds.c, but I don't want to
change that as part of this patch.

Regards,
Dean