pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Julien Rouhaud <rjuju123@gmail.com>
To: Pavel Stehule <pavel.stehule@gmail.com>
Cc: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Joel Jacobson <joel@compiler.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Schema variables - new implementation for Postgres 15
Date: Fri, 14 Jan 2022 10:44:02 +0800
Message-ID: <20220114024402.btyn7vrpkhnso2bq@jrouhaud> (raw)
In-Reply-To: <CAFj8pRDT4RW_ZiAbsbRGjTDZ+7on3y09dV04KVAEKya_HTo=1A@mail.gmail.com>
References: <CAFj8pRDN+B4OWkcanLa82BpqJqePEFcTzvu=pJFu=z+aTb9EXg@mail.gmail.com>
	<CAFj8pRAPcF7u_rV_2p=vaGFf8UkW1-Z7kef4rTFpsgy8JROH_w@mail.gmail.com>
	<68eedf25-79ca-d0e7-1af6-bad225fdcd97@enterprisedb.com>
	<CAEZATCW4PEWNC_tkXx5wzJij9at4ctpWWTr7Sa1bthwPRTm+fg@mail.gmail.com>
	<b846763e-1f9a-40a8-ac67-a3a4585826ef@www.fastmail.com>
	<CAFj8pRBT-bRQJBqkzon7tHcoFZ1byng06peZfZa0G72z46YFxg@mail.gmail.com>
	<1ebeaf57-da83-42e1-9302-fb126281814d@www.fastmail.com>
	<CAFj8pRD9iEpPN=qX1FKtNxYha9B48sZVqAjFgck7Ah0DpFckOg@mail.gmail.com>
	<CAEZATCUdkmVBKCy8WcLinMP9rjFD8hHrZ0Qy4tMEor_P_GHv2g@mail.gmail.com>
	<CAFj8pRDT4RW_ZiAbsbRGjTDZ+7on3y09dV04KVAEKya_HTo=1A@mail.gmail.com>

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.a.rasheed@gmail.com>
> napsal:
> 
> > On Thu, 13 Jan 2022 at 17:42, Pavel Stehule <pavel.stehule@gmail.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.

> 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?


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





view thread (365+ messages)  latest in thread

Message-ID: <20220114024402.btyn7vrpkhnso2bq@jrouhaud>
Permalink:  ../20220114024402.btyn7vrpkhnso2bq@jrouhaud/
Also on:    postgresql.org/message-id/20220114024402.btyn7vrpkhnso2bq@jrouhaud

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: rjuju123@gmail.com, pavel.stehule@gmail.com, dean.a.rasheed@gmail.com, joel@compiler.org, pgsql-hackers@lists.postgresql.org
  Subject: Re: Schema variables - new implementation for Postgres 15
  In-Reply-To: <20220114024402.btyn7vrpkhnso2bq@jrouhaud>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox