Re: Schema variables - new implementation for Postgres 15 - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: Schema variables - new implementation for Postgres 15
Date
Msg-id CAFj8pRAwWZ5CA+QZHzC+n2mfsoSeQ6XNoubV6C9kLPHw6Q2vag@mail.gmail.com
Whole thread Raw
In response to Re: Schema variables - new implementation for Postgres 15  (Peter Eisentraut <peter.eisentraut@enterprisedb.com>)
Responses Re: Schema variables - new implementation for Postgres 15  (Pavel Stehule <pavel.stehule@gmail.com>)
List pgsql-hackers
Hi


čt 23. 3. 2023 v 16:33 odesílatel Peter Eisentraut <peter.eisentraut@enterprisedb.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

pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: Memory leak from ExecutorState context?
Next
From: Robert Haas
Date:
Subject: Re: HOT chain validation in verify_heapam()