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

On Tue, Jan 10, 2023 at 08:35:16PM +0100, Pavel Stehule wrote:
> út 10. 1. 2023 v 3:20 odesílatel Julien Rouhaud <rjuju123@gmail.com> napsal:
> >
> > Another new behavior I see is the new rowtype_only parameter for
> > LookupVariable.  Has this been discussed?
> >
>
> I think so it was discussed about table shadowing
>
> without this filter, I lost the message "missing FROM-clause entry for ..."
>
>  -- should fail
>  SELECT varx.xxx;
> -ERROR:  missing FROM-clause entry for table "varx"
> -LINE 1: SELECT varx.xxx;
> -               ^
> +ERROR:  type text is not composite
>  -- don't allow multi column query
>  CREATE TYPE vartesttp AS (a1 int, b1 int, c1 int);
>  CREATE VARIABLE v1 AS vartesttp;
> @@ -1421,9 +1419,7 @@
>  DROP TYPE ab;
>  CREATE VARIABLE myvar AS int;
>  SELECT myvar.blabla;
> -ERROR:  missing FROM-clause entry for table "myvar"
> -LINE 1: SELECT myvar.blabla;
> -               ^
> +ERROR:  type integer is not composite
>  DROP VARIABLE myvar;
>  -- the result of view should be same in parallel mode too
>  CREATE VARIABLE v1 AS int;
>
> My original idea was to try to reduce possible conflicts (in old versions
> of this path, a conflict was disallowed). But it is true, so these "new"
> error messages are sensible too, and with eliminating rowtype_only I can
> reduce code.

Ok!  Another problem is that the error message as-is is highly unhelpful as
it's not clear at all that the problem is coming from an unsuitable variable.
Maybe change makeParamSessionVariable to use lookup_rowtype_tupdesc_noerror()
and emit a friendlier error message?  Something like

variable "X.Y" is of type Z, which is not composite

> I modified the IdentifyVariable function a little bit. With new argument
> noerror I am able to ensure so no error will be raised when this function
> is called just for shadowing detection.

I locally modified IdentifyVariable to emit WARNING reports when noerror is set
to quickly see how it was used and didn't get any regression test error.  This
definitely needs to be covered by regression tests.  Looking as
session_variables.sql, the session_variables_ambiguity_warning GUC doesn't have
a lot of tests in general.