Re: Schema variables - new implementation for Postgres 15

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: Sergey Shinderuk <s(dot)shinderuk(at)postgrespro(dot)ru>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Julien Rouhaud <rjuju123(at)gmail(dot)com>, dean(dot)a(dot)rasheed(at)gmail(dot)com, er(at)xs4all(dot)nl, joel(at)compiler(dot)org, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Schema variables - new implementation for Postgres 15
Date: 2023-03-24 07:04:08
Message-ID: CAFj8pRCthUs4ZXdLzcKkNKpebJ3S=kGutgHBTK3PegKULYgrTQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

čt 23. 3. 2023 v 19:54 odesílatel Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
napsal:

> 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.
>

Maybe I can divide the patch 0002-session-variables to three sections -
related to memory management, planning and execution?

Regards

Pavel

> Regards
>
> Pavel
>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Junwang Zhao 2023-03-24 07:16:01 fix a typo in file src/backend/utils/adt/xid8funcs.c comment
Previous Message wangw.fnst@fujitsu.com 2023-03-24 06:43:32 RE: Data is copied twice when specifying both child and parent table in publication