Re: Is temporary functions feature official/supported? Found someissues with it. - Mailing list pgsql-bugs

On Wed, Jan 9, 2019 at 4:30 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Jan 09, 2019 at 04:00:51PM +0900, Masahiko Sawada wrote:
> >  /*
> >   * XACT_FLAGS_ACCESSEDTEMPREL - set when a temporary relation is accessed. We
> >   * don't allow PREPARE TRANSACTION in that case.
> >   */
> >  #define XACT_FLAGS_ACCESSEDTEMPREL              (1U << 0)
> > (snip)
> >  +/*
> > + * XACT_FLAGS_ACCESSEDTEMPNAMESPACE - set when a temporary namespace is
> > + * accessed.  We don't allow PREPARE TRANSACTION in that case.
> > + */
> > +#define XACT_FLAGS_ACCESSEDTEMPNAMESPACE        (1U << 2)
> >
> > The cases where we set XACT_FLAGS_ACCESSEDTEMPNAMESPACE seems to
> > include the cases setting XACT_FLAGS_ACCESSEDTEMPREL. Is there any
> > path where we access a temporary relation without accessing temporary
> > namespace?
>
> This case would set ACCESSEDTEMPREL but not ACCESSEDTEMPNAMESPACE:
> create temp table twophase_tab (a int);
> begin;
> select a from twophase_tab;
> prepare transaction 'twophase_tab';

Thank you.

>
> I kept the original flag mainly for compatibility with the past
> handling so as the error message remains constant and back-patchable
> for application relying on the existing behavior.

Understood. Sounds good.

>  I think that for
> HEAD we could consider moving on with an approach where we have only
> ACCESSEDTEMPNAMESPACE, still we may want to make a difference for
> transactions which have actually tried to take any kind of locks on
> the temporary relation.

Yes, I was confused that in spite of the relation is a kind of
database object we separate two cases, relation and database objects
other than relation. So I thought that we can merge these flags to
something like XACT_FLAGS_ACCESSEDTEMPOBJ tracking whether the
transaction created, locked or dropped objects such as tables,
functions and datatypes on a temoprary namespace.

I've found a corner-case issue with this patch; this issue still
happens when we create the extension on the temprary namespace,
because it can be done without accessing the namespace if the
temporary namespace is already created.

postgres(1:59232)=# create table pg_temp.e(c int);
CREATE TABLE
postgres(1:59232)=# begin;
BEGIN
postgres(1:59232)=# create extension pgcrypto schema pg_temp_3;
CREATE EXTENSION
postgres(1:59232)=# prepare transaction 'a';
PREPARE TRANSACTION
postgres(1:59232)=# create extension pgcrypto;
(hang..)

To fix we could have get_namepace_oid set
XACT_FLAGS_ACCESSEDTEMPNAMESPACE (not calling
InitTempTableNamespace()) if the nspname is pg_temp_NNN but it could
influence many places.

Also, since even the current_schame() and the current_scheames() could
create the temporary namespace when executing fetch_search_path()
while 'pg_temp' appears the head of seach_patch, for example, the
following transaction cannot prepre. But it possible coulld be
acceptable by users as this is very corner case and it could be
regards as accessing the temporary namespace actually.

postgres(1:59025)=# set search_path to pg_temp, "$user", public;
SET
postgres(1:59025)=# begin;
BEGIN
postgres(1:59025)=# select current_schema;
 current_schema
----------------
 pg_temp_3
(1 row)
postgres(1:59025)=# prepare transaction 'a';
ERROR:  cannot PREPARE a transaction that has operated on temporary namespace


Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center