Re: Schema variables - new implementation for Postgres 15

pá 14. 1. 2022 v 3:44 odesílatel Julien Rouhaud <rjuju123(at)gmail(dot)com> napsal:

> Hi,
>
> On Thu, Jan 13, 2022 at 07:32:26PM +0100, Pavel Stehule wrote:
> > čt 13. 1. 2022 v 19:23 odesílatel Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com
> >
> > napsal:
> >
> > > On Thu, 13 Jan 2022 at 17:42, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
> > > wrote:
> > > >
> > > > I like the idea of prioritizing tables over variables with warnings
> when
> > > collision is detected. It cannot break anything. And it allows to using
> > > short identifiers when there is not collision.
> > >
> > > Yeah, that seems OK, as long as it's clearly documented. I don't think
> > > a warning is necessary.
>
> What should be the behavior for a cached plan that uses a variable when a
> conflicting relation is later created? I think that it should be the same
> as a
> search_path change and the plan should be discarded.
>

This is a more generic problem - creating a new DDL object doesn't
invalidate plans.

https://www.postgresql.org/message-id/2589876.1641914327%40sss.pgh.pa.us

>
> > The warning can be disabled by default, but I think it should be there.
> > This is a signal, so some in the database schema should be renamed.
> Maybe -
> > session_variables_ambiguity_warning.
>
> I agree that having a way to know that a variable has been bypassed can be
> useful.
>
> > > (FWIW, testing with dbfiddle, that appears to match Db2's behaviour).
> > >
> >
> > Thank you for check
>
> Do you know what's oracle's behavior on that?
>
>
Oracle is very different, because package variables are not visible from
plain SQL. And change of interface invalidates dependent objects and
requires recompilation. So it is a little bit more sensitive. If I remember
well, the SQL identifiers have bigger priority than PL/SQL identifiers
(package variables), so proposed behavior is very similar to Oracle
behavior too. The risk of unwanted collision is reduced (on Oracle) by
local visibility of package variables, and availability of package
variables only in some environments.

>
> I've been looking at the various dependency handling, and I noticed that
> collation are ignored, while they're accepted syntax-wise:
>
> =# create collation mycollation (locale = 'fr-FR', provider = 'icu');
> CREATE COLLATION
>
> =# create variable myvariable text collate mycollation;
> CREATE VARIABLE
>
> =# select classid::regclass, objid, objsubid, refclassid::regclass,
> refobjid, refobjsubid from pg_depend where classid::regclass::text =
> 'pg_variable' or refclassid::regclass::text = 'pg_variable';
> classid | objid | objsubid | refclassid | refobjid | refobjsubid
> -------------+-------+----------+--------------+----------+-------------
> pg_variable | 16407 | 0 | pg_namespace | 2200 | 0
> (1 row)
>
> =# let myvariable = 'AA';
> LET
>
> =# select 'AA' collate "en-x-icu" < myvariable;
> ?column?
> ----------
> f
> (1 row)
>
> =# select 'AA' collate "en-x-icu" < myvariable collate mycollation;
> ERROR: 42P21: collation mismatch between explicit collations "en-x-icu"
> and "mycollation"
> LINE 1: select 'AA' collate "en-x-icu" < myvariable collate mycollat...
>
> So it's missing both dependency recording for variable's collation and also
> teaching various code that variables can have a collation.
>
> It's also missing some invalidation detection. For instance:
>
> =# create variable myval text;
> CREATE VARIABLE
>
> =# let myval = 'pg_class';
> LET
>
> =# prepare s(text) as select relname from pg_class where relname = $1 or
> relname = myval;
> PREPARE
>
> =# set plan_cache_mode = force_generic_plan ;
> SET
>
> =# execute s ('');
> relname
> ----------
> pg_class
> (1 row)
>
> =# drop variable myval ;
> DROP VARIABLE
>
> =# create variable myval int;
> CREATE VARIABLE
>
> =# execute s ('');
> ERROR: XX000: cache lookup failed for session variable 16408
>
> The plan should have been discarded and the new plan should fail for type
> problem.
>
> Strangely, subsequent calls don't error out:
>
> =# execute s('');
> relname
> ---------
> (0 rows)
>
> But doing an explain shows that there's a problem:
>
> =# explain execute s('');
> ERROR: XX000: cache lookup failed for variable 16408
>

looks like bug

Regards

Pavel