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)
