Re: proposal: schema variables

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
Cc: Erik Rijkers <er(at)xs4all(dot)nl>, Michael Paquier <michael(at)paquier(dot)xyz>, Zhihong Yu <zyu(at)yugabyte(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, DUVAL REMI <REMI(dot)DUVAL(at)cheops(dot)fr>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: proposal: schema variables
Date: 2024-07-25 16:32:24
Message-ID: CAFj8pRAsjr0pzQpQeRrQ-wR-ew1oTc6qkW=4355KveN1jKQ+1Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

čt 25. 7. 2024 v 15:52 odesílatel Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
napsal:

> Thanks for the updated patch set.
>
> I found a problem in 0019-transactional-variables.patch:
>
> --- a/doc/src/sgml/catalogs.sgml
> +++ b/doc/src/sgml/catalogs.sgml
> @@ -9851,6 +9851,17 @@ SCRAM-SHA-256$<replaceable>&lt;iteration
> count&gt;</replaceable>:<replaceable>&l
> </para></entry>
> </row>
>
> + <row>
> + <entry><structfield>varistransact</structfield></entry>
> + <entry><type>boolean</type></entry>
> + <entry></entry>
> + <entry>
> + True, when the variable is "transactional". In case of transaction
> + rollback, transactional variables are reset to their content at the
> + transaction start. The default value is false.
> + </entry>
> + </row>
>
> That's messed up; it should be
>
> <row>
> <entry role="catalog_table_entry"><para role="column_definition">
> <structfield>varistransact</structfield> <type>boolean</type>
> </para>
> <para>
> True, when the variable is <quote>transactional</quote>. In the case
> of a transaction rollback, transactional variables are reset to the
> value they had when the transaction started. The default value is
> <literal>false</literal>.
> </para></entry>
> </row>
>
>
changed

> I have started reading through the first patch, and so far I have only
> found
> language problems.
>
> I wonder how I should go about this. At first, I intended to send an
> edited
> version of the first patch, but as later patches depend on earlier patches,
> that would mess up the patch set.
>
> I can send my suggested modifications in text, but then you have to copy
> and
> paste them all, which is cumbersome.
>
> What would be best for you?
>

send me it in any form - probably the diff of patches are best. I'll do
necessary fixes.

There are not good tool for maintaining chronologically incremental
patchset

>
> Thinking further, I wondered about the order of patches.
> If some committer eventually takes mercy on this patch set, I expect that
> only a part of the functionality will go in as a first step.
> Does the order of the patches in the patch set match such a process?
>

yes

we talk so first seven patches are mandatory - others are optional and
introduce new functionality or increase performance

>
> I'd guess that temporary session variables or ON TRANSACTION END RESET
> would be things that can be committed later on, but PL/pgSQL support
> should be in the first commit.
>

The plpgsql is supported by a second patch, because variables are
accessible by queries. But patch 16 introduces the
PLpgSQL LET command, and then allows us to evaluate an expression as a
simple expression, which is significantly faster.

> What is your approach to that?
>

The first six, seven patches are fundamental. For others we can talk about
priorities. I see performance related patches to be more important, but
they are a little bit more technically difficult or maybe related to a not
too cool area now (like PL/pgSQL - I like it, but almost all people's
stored procedures aren't used). Implementation of new features is almost
done by simple patches. And again for somebody temp variables are not
important (usage temp variables in PLpgSQL is +/- zero), but for others can
be very important (it can be very interesting for writing psql scripts,
because it allows parametrization of DO commands, and it is cheaper than
temp tables for passing result).

So others patches have some order, because there has to be some order, but
it is not final or immutable. I can imagine throwing these patches to
different patchset. On the second hand, the size of these patches is less
than the first two, and I think it is interesting, because from my
perspective, these patches cover this area completely. But again, the order
of these patches is important for some dependencies in files (regress
tests), but it doesn't draw the priority of implemented features. Current
state is the result of my work or work of other peoples that did review of
these patches. I am open to changing this order, if somebody would do it.

Regards

Pavel

> Yours,
> Laurenz Albe
>

Attachment Content-Type Size
v20240725-0001-Enhancing-catalog-for-support-session-variables-and-.patch text/x-patch 132.0 KB
v20240725-0003-function-pg_session_variables-for-cleaning-tests.patch text/x-patch 4.6 KB
v20240725-0005-memory-cleaning-after-DROP-VARIABLE.patch text/x-patch 22.5 KB
v20240725-0004-DISCARD-VARIABLES.patch text/x-patch 9.6 KB
v20240725-0002-Storage-for-session-variables-and-SQL-interface.patch text/x-patch 147.7 KB
v20240725-0006-plpgsql-tests.patch text/x-patch 16.9 KB
v20240725-0007-GUC-session_variables_ambiguity_warning.patch text/x-patch 14.3 KB
v20240725-0009-PREPARE-LET-support.patch text/x-patch 7.4 KB
v20240725-0008-EXPLAIN-LET-support.patch text/x-patch 8.3 KB
v20240725-0010-implementation-of-temporary-session-variables.patch text/x-patch 38.7 KB
v20240725-0011-Implementation-ON-TRANSACTION-END-RESET-clause.patch text/x-patch 14.5 KB
v20240725-0012-Implementation-of-DEFAULT-clause-default-expressions.patch text/x-patch 33.4 KB
v20240725-0013-Implementation-of-NOT-NULL-and-IMMUTABLE-clauses.patch text/x-patch 36.2 KB
v20240725-0014-allow-read-an-value-of-session-variable-directly-fro.patch text/x-patch 12.0 KB
v20240725-0015-allow-parallel-execution-queries-with-session-variab.patch text/x-patch 11.3 KB
v20240725-0016-plpgsql-implementation-for-LET-statement.patch text/x-patch 13.6 KB
v20240725-0017-expression-with-session-variables-can-be-inlined.patch text/x-patch 4.2 KB
v20240725-0018-this-patch-changes-error-message-column-doesn-t-exis.patch text/x-patch 29.1 KB
v20240725-0019-transactional-variables.patch text/x-patch 39.0 KB
v20240725-0020-pg_restore-A-variable.patch text/x-patch 2.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michał Kłeczek 2024-07-25 16:58:00 Re: DRAFT: Pass sk_attno to consistent function
Previous Message Andrew Dunstan 2024-07-25 16:25:49 Re: Sporadic connection-setup-related test failures on Cygwin in v15-