Re: Global temporary tables

On Thu, 2 Jul 2026 at 10:11, Japin Li <japinli(at)hotmail(dot)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 local temporary tables")));
> if (!pkrel->rd_islocaltemp || !rel->rd_islocaltemp)
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
> - errmsg("constraints on temporary tables must involve temporary tables of this session")));
> + errmsg("constraints on local temporary tables must involve local temporary tables 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 global temporary 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