On Thu, 2 Jul 2026 at 10:11, Japin Li <[email protected]> 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
