Re: Supporting MERGE on updatable views
От | Dean Rasheed |
---|---|
Тема | Re: Supporting MERGE on updatable views |
Дата | |
Msg-id | CAEZATCWGGmigGBzLHkJm5Ccv2mMxXmwi3+uq0yhwDHm-tsvSLg@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Supporting MERGE on updatable views (Alvaro Herrera <alvherre@alvh.no-ip.org>) |
Ответы |
Re: Supporting MERGE on updatable views
|
Список | pgsql-hackers |
On Tue, 30 Jan 2024 at 11:58, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > This looks quite nice, thanks. LGTM. > Going over this again, I spotted a bug -- the UPDATE path in ExecMergeMatched() wasn't calling ExecUpdateEpilogue() for trigger-updatable views, because it wasn't setting updateCxt.updated to true. (This matters if you have an auto-updatable view WITH CHECK OPTION on top of a trigger-updatable view, so I've added a new test there.) Rather than setting updateCxt.updated to true, making the trigger-invoking code in ExecMergeMatched() diverge from ExecUpdate(), a better fix is to simply remove the UpdateContext.updated flag entirely. The only place that reads it is this code in ExecMergeMatched(): if (result == TM_Ok && updateCxt.updated) { ExecUpdateEpilogue(context, &updateCxt, resultRelInfo, tupleid, NULL, newslot); where result is the result from ExecUpdateAct(). However, all paths through ExecUpdateAct() that return TM_Ok also set updateCxt.updated to true, so the flag is redundant. It looks like that has always been the case, ever since it was introduced. Getting rid of it is a useful simplification, and brings the UPDATE path in ExecMergeMatched() more into line with ExecUpdate(), which always calls ExecUpdateEpilogue() if ExecUpdateAct() returns TM_Ok. Aside from that, I've done a little more copy-editing and added a few more test cases, and I think this is pretty-much good to go, though I think I'll split this up into separate commits, since removing UpdateContext.updated isn't really related to the MERGE INTO view feature. Regards, Dean
Вложения
В списке pgsql-hackers по дате отправления: