Re: Schema variables - new implementation for Postgres 15

Hi

čt 23. 3. 2023 v 16:33 odesílatel Peter Eisentraut <
peter(dot)eisentraut(at)enterprisedb(dot)com> napsal:

> On 17.03.23 21:50, Pavel Stehule wrote:
> > Hi
> >
> > rebase + fix-update pg_dump tests
> >
> > Regards
> >
> > Pavel
> >
>
> I have spent several hours studying the code and the past discussions.
>
> The problem I see in general is that everyone who reviews and tests the
> patches finds more problems, behavioral, weird internal errors, crashes.
> These are then immediately fixed, and the cycle starts again. I don't
> have the sense that this process has arrived at a steady state yet.
>
> The other issue is that by its nature this patch adds a lot of code in a
> lot of places. Large patches are more likely to be successful if they
> add a lot of code in one place or smaller amounts of code in a lot of
> places. But this patch does both and it's just overwhelming. There is
> so much new internal functionality and terminology. Variables can be
> created, registered, initialized, stored, copied, prepared, set, freed,
> removed, released, synced, dropped, and more. I don't know if anyone
> has actually reviewed all that in detail.
>
> Has any effort been made to make this simpler, smaller, reduce scope,
> refactoring, find commonalities with other features, try to manage the
> complexity somehow?
>
> I'm not making a comment on the details of the functionality itself. I
> just think on the coding level it's not gotten to a satisfying situation
> yet.
>
>
I agree that this patch is large, but almost all code is simple. Complex
code is "only" in 0002-session-variables.patch (113KB/438KB).

Now, I have no idea how the functionality can be sensibly reduced or
divided (no without significant performance loss). I see two difficult
points in this code:

1. when to clean memory. The code implements cleaning very accurately - and
this is unique in Postgres. Partially I implement some functionality of
storage manager. Probably no code from Postgres can be reused, because
there is not any support for global temporary objects. Cleaning based on
sinval messages processing is difficult, but there is nothing else. The
code is a little bit more complex, because there are three types of session
variables: a) session variables, b) temp session variables, c) session
variables with transaction scope. Maybe @c can be removed, and maybe we
don't need to support not null default (this can simplify initialization).
What do you think about it?

2. how to pass a variable's value to the executor. The implementation is
based on extending the Param node, but it cannot reuse query params buffers
and implements own.
But it is hard to simplify code, because we want to support usage variables
in queries, and usage in PL/pgSQL expressions too. And both are processed
differently.

Regards

Pavel