Hi,
On Fri, Sep 02, 2022 at 07:42:00AM +0200, Pavel Stehule wrote:
>
> rebased today
After some off-list discussion with Pavel, I'm sending some proposal patches
(in .txt extension) to apply to the last patchset.
To sum up, when a session issues a DROP VARIABLE, the session will receive an
sinval notification for its own drop and we don't want to process it
immediately as we need to hold the value in case the transaction is rollbacked.
The current patch avoided that by forcing a single processing of sinval per
transaction, and forcing it before dropping the variable. It works but it
seems to me that postponing all but the first VARIABLEOID sinval to the next
transaction is a big hammer, and the sooner we can free some memory the better.
For an alternative approach the attached patch store the lxid in the SVariable
itself when dropping a currently set variable, so we can process all sinval and
simply defer to the next transaction the memory cleanup of the variable(s) we
know we just dropped. What do you think of that approach?
As I was working on some changes I also made a pass on session_variable.c. I
tried to improve a bit some comments, and also got rid of the "first_time"
variable. The name wasn't really great, and AFAICS it can be replaced by
testing whether the memory context has been created yet or not.
But once that done I noticed the get_rowtype_value() function. I don't think
this function is necessary as the core code already knows how to deal with
stored datum when the underlying composite type was modified. I tried to
bypass that function and always simply return the stored value and all the
tests run fine. Is there really any cases when this extra code is needed?
FTR I tried to do a bunch of additional testing using relation as base type for
variable, as you can do more with those than plain composite types, but it
still always works just fine.
However, while doing so I noticed that find_composite_type_dependencies()
failed to properly handle dependencies on relation (plain tables, matviews and
partitioned tables). I'm also adding 2 additional patches to fix this corner
case and add an additional regression test for the plain table case.