On Tue, Jan 9, 2024 at 6:21 PM vignesh C <
vignesh21@gmail.com> wrote:
>
> > doc seems to still have an issue.
> >
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F45%2F4617> >
> > In the regress test, do we need to clean up the created object after we use it.
> > tested passed, looking at ExecIRInsertTriggers, your changes look sane.
>
> I have changed the status of the patch to "Waiting on Author" as the
> CFBot reported by jina he is not yet handled.
Hi
it took me a while to figure out why the doc build fails.
Currently your wording is:
For INSERT, UPDATE, and DELETE operations, INSTEAD OF triggers can modify the data returned by RETURNING. In the case of INSERT and UPDATE, triggers can modify the NEW row before returning it, while for DELETE, triggers can modify the OLD row before returning it. This feature is useful when the returned data needs to be adjusted to match the view or other requirements.
The doc is:
For INSERT and UPDATE operations only, the trigger may modify the NEW row before returning it. This will change the data returned by INSERT RETURNING or UPDATE RETURNING, and is useful when the view will not show exactly the same data that was provided.
to make it a minimum change compared to doc, i think the following make sense:
For INSERT and UPDATE operations only, the trigger may modify the NEW row before returning it. For DELETE operations, the trigger may modify the OLD row before returning it.
This will change the data returned by INSERT RETURNING, UPDATE RETURNING, DELETE RETURNING and is useful when the view will not show exactly the same data that was provided.
I am not sure the following changes in the function ExecIRDeleteTriggers
is right.+ else if (newtuple != oldtuple)
+ {
+ ExecForceStoreHeapTuple(newtuple, slot, false);
+
+ if (should_free)
+ heap_freetuple(oldtuple);
+
+ /* signal tuple should be re-fetched if used */
+ newtuple = NULL;
In the ExecIRDeleteTriggers function,
all we want is to return the slot,
so that, nodeModifyTable.c `if (processReturning && resultRelInfo->ri_projectReturning) {}` can further process it, materialize it.
if newtuple != oldtuple that means DELETE INSTEAD OF changed the value.
ExecForceStoreHeapTuple already put the new values into the slot, we should just free the newtuple, since we don't use it anymore?
Also maybe we don't need the variable should_free, if (newtuple != oldtuple) then we should free oldtuple and newtuple, because the content is already in the slot.
Anyway, based on your patch, I modified it, also added a slightly more complicated test.