Re: Supporting MERGE on updatable views

Thanks for working on this. The patch looks well finished. I didn't
try to run it, though. I gave it a read and found nothing to complain
about, just these two pretty minor comments:

On 2024-Jan-26, Dean Rasheed wrote:

> diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
> new file mode 100644
> index 13a9b7d..5a99e4a
> --- a/src/backend/executor/execMain.c
> +++ b/src/backend/executor/execMain.c
> @@ -1021,11 +1021,13 @@ InitPlan(QueryDesc *queryDesc, int eflag
> * CheckValidRowMarkRel.
> */
> void
> -CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation)
> +CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation,
> + List *mergeActions)
> {

I suggest to add commentary explaining the new argument of this function.

> @@ -1080,6 +1083,51 @@ CheckValidResultRel(ResultRelInfo *resul
> RelationGetRelationName(resultRel)),
> errhint("To enable deleting from the view, provide an INSTEAD OF DELETE trigger or an unconditional ON DELETE DO INSTEAD rule.")));
> break;
> + case CMD_MERGE:
> +
> + /*
> + * Must have a suitable INSTEAD OF trigger for each MERGE
> + * action. Note that the error hints here differ from
> + * above, since MERGE doesn't support rules.
> + */
> + foreach(lc, mergeActions)
> + {
> + MergeAction *action = (MergeAction *) lfirst(lc);
> +
> + switch (action->commandType)
> + {
> + case CMD_INSERT:
> + if (!trigDesc || !trigDesc->trig_insert_instead_row)
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("cannot insert into view \"%s\"",
> + RelationGetRelationName(resultRel)),
> + errhint("To enable inserting into the view using MERGE, provide an INSTEAD OF INSERT trigger.")));

Looking at the comments in ereport_view_not_updatable and the code
coverate reports, it appears that these checks here are dead code? If
so, maybe it would be better to turn them into elog() calls? I don't
mean to touch the existing code, just that I don't see that it makes
sense to introduce the new ones as ereport().

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"La persona que no quería pecar / estaba obligada a sentarse
en duras y empinadas sillas / desprovistas, por cierto
de blandos atenuantes" (Patricio Vogel)