Hi
On Wed, May 21, 2025 at 09:12:54AM +0200, Pavel Stehule wrote:
> Last discussion is related to reducing the size of the session variable patch
> set.
>
> I have an idea to use variable's fencing more aggressively from the start, and
> then we can reduce it in future. This should not break issues with
> compatibility and doesn't need some like version flags.
>
> The real problem of proposed session variables is possible collisions between
> session variables identifiers and table or columns identifiers. I designed some
> tools to minimize the risk of unwanted collisions, but these tools increase the
> size of code and don't reduce the complexity of the patch and tests. The
> proposed change probably doesn't reduce a lot of code, but can reduce some
> tests, and mainly possible risk of some unwanted impact - at the end it can be
> less work for reviewers and less stress for committers - and the implementation
> can be divided to allone workable following steps.
Yes, I remember the discussions about how the creation of server
variables could break existing queries. Our scoping rules are already
complex, so adding another scope would add a lot of complexity.
> Step 1
> =====
>
> So the main change is the hard requirement for usage variable's fence
> everywhere where collisions are possible - and then in the first step, the
> collisions will not be possible, and then we don't need it to solve, and we
> don't need to test it.
>
> CREATE VARIABLE public.foo AS int;
> LET foo = 10;
> SELECT VARIABLE(foo);
Yes, I can see how adding fencing like VARIABLE() would simplify things.
> Step 2
> =====
> Necessity of usage variable fencing in PL/pgSQL can be a problem for migration
> from PL/SQL. But this can be solved separately by using SPI params hooks -
> similar to how PL/pgSQL works with PL/pgSQL variables. In this step we can push
> optimization for fast execution of the LET statement or optimization of usage
> variables in queries.
Yes, there is already going to be migration requirements in moving from
PL/SQL to PL/pgSQL, so the requirement to add VARIABLE() seems minimal.
> After this step will be possible:
>
> DO $$
> BEGIN
> RAISE NOTICE '% %', foo, VARIABLE(public.foo);
> END;
> $$;
>
> SELECT VARIABLE(foo);
>
> No other visible change in this step. WIth this step the people who do
> migration form Oracle and PL/pgSQL developers will be very happy. They don't
> need more. There can be collisions, but the collisions can be limited just to
> PL/pgSQL scope, and we can use already implemented mechanisms.
>
> Step 3
> =====
> We can talk in future about less requirement of usage variable fencing in
> queries. This needs to introduce some form of detection collisions and how they
> should be solved (outside PL/pgSQL).
> We can talk about other features like temporal, default values, transactional,
> etc ...
I feel that if we haven't found a good solution to this in 13 years, we
should assume it is unsolvable and just accept an imperfect solution.
I am sending an reorganized and modified patch set with implementation of session variables
Important changes:
1. session variables fence is required everywhere except target of LET command. Then there
are no possible collisions of session variables with column or plpgsql variables identifiers. The code
related to collision detection or collision solving is removed.
2. I little bit reduced the size of patch because I didn't introduce EXPR_KIND_LET_TARGET and
EXPR_KIND_ASSING_TARGET contextes.
3. There are not other changes in the code
4. First two patches (originally 170KB and 160KB) are divided to some few patches with max lengths (81KB and 53KB). The patches
are incremental still (and possibly tested incrementally) with more cleaner coverage
0001-introduce-new-class-catalog-pg_variable.patch - new catalog
0002-CREATE-DROP-ALTER-VARIABLE.patch - DDL commands
0003-GRANT-REVOKE-variable.patch - ACL SELECT, UPDATE
0004-support-of-session-variables-for-psql.patch - psql tab complete and \dV
0005-support-of-session-variables-for-pg_dump.patch - pg_dump can backup session variables
0006-session-variable-fences-parsing.patch - parser support for syntax VARIABLE(varname)
0007-local-HASHTAB-for-currently-used-session-variables-a.patch - local memory based storage for values of session variables
0008-collect-session-variables-used-in-plan-and-assign-pa.patch - planner support for reading session variables from query
0009-fill-an-auxiliary-buffer-with-values-of-session-vari.patch - executor support for reading session variables from query
0010-svariableReceiver.patch - DestReceiver used by LET command
0011-LET-command-assign-a-result-of-expression-to-the-ses.patch - LET command implementation, parsing, execution
0012-function-pg_session_variables-for-cleaning-tests.patch - debug function - returns a list of session variables from local memory storage
0013-DISCARD-VARIABLES.patch - throw memory used by session variables
0014-memory-cleaning-after-DROP-VARIABLE.patch - clean related memory after DROP VARIABLE
0015-plpgsql-tests.patch - special tests for plpgsql, plan cache, identifier collision, validity of memory used by session variables
The new total size of patches is 377KB, the size of previous patches was 400KB. The code is less than 1/3 of this (I think less than 2000 lines).
Almost all code is very simple without impact on other parts of Postgres. Maybe an exception is memory-cleaning-after-DROP-VARIABLE
patch - but still there is no impact to other parts of Postgres.
Now there is a little bit overhead of git format-patch format. I refactorized tests to separate files acl, ddl, dml. U use longer unique identifiers,
so the execution of the tests should be more stable and less sensitive for parallel tests execution.
I removed a few explicitly redundant tests.
This is the first step of implementing session variables in Postgres. It contains only basic implementation of session variables. The lot of
proposed features (in this thread) can be implemented in the future without compatibility issues. Their implementation is postponed now,
because the patch set is large enough now.
Although this is just a subset of proposed (discussed) functionality, I think it can be valuable for people that need session variables.
This patchset doesn't contains:
1. Using session variables without variable fences inside plpgsql (then the usage will be the same as PL/pgSQL or PL/SQL variables)
2. Performance optimization of usage of session variables (mostly for plpgsql)
3. Implementation of EXPLAIN LET, PREPARE LET commands
4. Implementation of temporary session variables
5. Possibility to set default value for session variable (now it is NULL)
6. Immutable session variables support
7. Implementation of transactional behave of content of session variables
8. Special option --variable for pg_dump
Please, do a review if you can.
Comments, notes?
Regards
Pavel