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

pá 20. 1. 2023 v 21:35 odesílatel Dmitry Dolgov <9erthalion6(at)gmail(dot)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