Re: Schema variables - new implementation for Postgres 15 (typo)

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

> > It can be used only
> > from extensions, where is ensured so a) the value is not changed, b) and
> in
> > a lifetime of returned value is not called any query or any expression
> that
> > can change the value of this variable. I fixed this code and I enhanced
> > comments. I am not sure if this function should not be removed. It is not
> > used by backend, but it can be handy for extensions - it reduces possible
> > useless copy.
>
> Hmm, how safe is it for third-party code to access the stored data directly
> rather than a copy? If it makes extension fragile if they're not careful
> enough with cache invalidation, or even give them a way to mess up with the
> data directly, it's probably not a good idea to provide such an API.
>

ok, I removed it

>
>
> > > I'm also wondering if there should be additional tests based on the
> last
> > > scenario reported by Dmitry? (I don't see any new or similar test, but
> I
> > > may
> > > have missed it).
> > >
> >
> > The scenario reported by Dmitry is in tests already.
>
> Oh, so I missed it sorry about that. I did some testing using
> debug_discard_cache in the past and didn't run into this issue, so it's
> probably due to a more recent changes or before such a test was added.
>
> > I am not sure if I
> > have to repeat it with active debug_discard_cache. I expect this mode
> will
> > be activated in some testing environments.
>
> Yes, some buildfarm animal are configured to run with various
> debug_discard_caches setting so it's not needed to override it in some
> tests
> (it makes testing time really slow, which will be annoying for everyone
> including old/slow buildfarm animals).
>
> > I have no idea how to simply emulate this issue without
> > debug_discard_caches on 1. It is necessary to raise the sinval message
> > exactly when the variable is checked against system catalogue.
>
> Manually testing while setting locally debug_discard_cache should be
> enough.
>
> > updated patches attached
>
> Thanks!
>

I thank you

Pavel

Attachment Content-Type Size
v20230107-1-0007-possibility-to-dump-session-variables-by-pg_dump.patch text/x-patch 19.5 KB
v20230107-1-0010-documentation.patch text/x-patch 43.7 KB
v20230107-1-0009-this-patch-changes-error-message-column-doesn-t-exis.patch text/x-patch 26.0 KB
v20230107-1-0008-regress-tests-for-session-variables.patch text/x-patch 60.4 KB
v20230107-1-0006-enhancing-psql-for-session-variables.patch text/x-patch 14.1 KB
v20230107-1-0005-DISCARD-VARIABLES-command.patch text/x-patch 3.2 KB
v20230107-1-0004-support-of-LET-command-in-PLpgSQL.patch text/x-patch 11.9 KB
v20230107-1-0003-LET-command.patch text/x-patch 44.9 KB
v20230107-1-0002-session-variables.patch text/x-patch 110.7 KB
v20230107-1-0001-catalog-support-for-session-variables.patch text/x-patch 89.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-01-07 07:21:26 Re: [PATCH] Const'ify the arguments of ilist.c/ilist.h functions
Previous Message Peter Eisentraut 2023-01-07 06:37:49 Re: Generating code for query jumbling through gen_node_support.pl