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

Pavel Stehule <pavel.stehule@gmail.com>

From: Pavel Stehule <pavel.stehule@gmail.com>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>, Erik Rijkers <er@xs4all.nl>, Sergey Shinderuk <s.shinderuk@postgrespro.ru>, Tomas Vondra <tomas.vondra@enterprisedb.com>, dean.a.rasheed@gmail.com, joel@compiler.org, pgsql-hackers@lists.postgresql.org
Date: 2023-01-22T18:47:07Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Allow underscores in integer and numeric constants.

  2. Remove special outfuncs/readfuncs handling of RangeVar.catalogname.

  3. Remove extra space from dumped ALTER DEFAULT PRIVILEGES.

  4. Create FKs properly when attaching table as partition

  5. psql: improve tab-complete's handling of variant SQL names.

Attachments

pá 20. 1. 2023 v 21:35 odesílatel Dmitry Dolgov <9erthalion6@gmail.com>
napsal:

> I've accumulated another collection of various questions and comments. As a
> side note I'm getting a good feeling about this patch, those part I've
> read so
> far looks good to me.
>
> * I've suddenly realized one could use pseudo types for variables, and
>   it not always works. E.g.:
>
>     =# create variable pseudo_array anyarray;
>     =# select pseudo_array;
>      pseudo_array
>     --------------
>      NULL
>
>     =# let pseudo_array = ARRAY[1, 2, 3];
>     ERROR:  42804: target session variable is of type anyarray but
> expression is of type integer[]
>     LOCATION:  svariableStartupReceiver, svariableReceiver.c:79
>
>     =# create variable pseudo_unknown unknown;
>     =# select pseudo_unknown;
>     ERROR:  XX000: failed to find conversion function from unknown to text
>     LOCATION:  coerce_type, parse_coerce.c:542
>
>   Is it supposed to be like this, or something is missing?
>

it is my oversight - it should be disallowed

done



>
> * I think it was already mentioned in the thread, there seems to be not a
>   single usage of CHECK_FOR_INTERRUPTS in session_variable.c . But some
> number
>   of loops over the sessionvars are implemented, are they always going to
> be
>   small enough to not make any troubles?
>

The longest cycle is a cycle that rechecks actively used variables against
system catalog. No cycle depends on the content of variables.


>
> * sync_sessionvars_all explains why is it necessary to copy
> xact_recheck_varids:
>
>                  When we check the variables, the system cache can be
> invalidated,
>                  and xact_recheck_varids can be enhanced.
>
>   I'm not quite following what the "enhancement" part is about? Is
>   xact_recheck_varids could be somehow updated concurrently with the loop?
>

Yes. pg_variable_cache_callback can be called when
is_session_variable_valid is executed.

Maybe "extended" can be a better word instead of "enhanced"? I reformulated
this comment



>
> * A small typo
>
>         diff --git a/src/backend/commands/session_variable.c
> b/src/backend/commands/session_variable.c
>         --- a/src/backend/commands/session_variable.c
>         +++ b/src/backend/commands/session_variable.c
>         @@ -485,7 +485,7 @@ sync_sessionvars_all(bool filter_lxid)
>
>                         /*
>                          * When we check the variables, the system cache
> can be invalidated,
>         -         * and xac_recheck_varids can be enhanced. We want to
> iterate
>         +        * and xact_recheck_varids can be enhanced. We want to
> iterate
>
>
fixed


> NOTE: The commentaries above were made based on the previous patch
> version, but
> it looks like those aspects were not changed.
>

Thank you for comments, updated rebased patch assigned

Regards

Pavel