Re: [PATCH] PREPARE TRANSACTION unexpected behavior with TEMP TABLE

On Tue, Apr 6, 2021 at 8:18 PM Himanshu Upadhyaya
<upadhyaya(dot)himanshu(at)gmail(dot)com> wrote:
>
> Hi,
>
> While working on one of the issue, I have noticed below unexpected behavior with "PREPARE TRANSACTION".
>
> We are getting this unexpected behavior with PREPARE TRANSACTION when it is mixed with Temporary Objects. Please consider the below setup and SQL block.
>
> set max_prepared_transactions to 1 (or any non zero value), this is to enable the “prepare transaction”.
>
> Now please try to run the below set of statements.
> [BLOCK-1:]
> postgres=# create temp table fullname (first text, last text);
> CREATE TABLE
> postgres=# BEGIN;
> BEGIN
> postgres=*# create function longname(fullname) returns text language sql
> postgres-*# as $$select $1.first || ' ' || $1.last$$;
> CREATE FUNCTION
> postgres=*# prepare transaction 'mytran';
> ERROR: cannot PREPARE a transaction that has operated on temporary objects
>
> Above error is expected.
>
> The problem is if we again try to create the same function in the “PREPARE TRANSACTION” as below.
>
> [BLOCK-2:]
> postgres=# BEGIN;
> BEGIN
> postgres=*# create function longname(fullname) returns text language sql
> as $$select $1.first || ' ' || $1.last$$;
> CREATE FUNCTION
> postgres=*# PREPARE transaction 'mytran';
> PREPARE TRANSACTION
>
> Now, this time we succeed and not getting the above error (”ERROR: cannot PREPARE a transaction that has operated on temporary objects), like the way we were getting with BLOCK-1
>
> This is happening because we set MyXactFlags in relation_open function call, and here relation_open is getting called from load_typcache_tupdesc, but in the second run of “create function…” in the above #2 block will not call load_typcache_tupdesc because of the below condition(typentry->tupDesc == NULL) in lookup_type_cache().
>
> /*
> * If it's a composite type (row type), get tupdesc if requested
> */
> if ((flags & TYPECACHE_TUPDESC) &&
> typentry->tupDesc == NULL &&
> typentry->typtype == TYPTYPE_COMPOSITE)
> {
> load_typcache_tupdesc(typentry);
> }
>
> We set typentry->tupDesc to non NULL(and populates it with proper tuple descriptor in the cache) value during our first call to “create function…” in BLOCK-1.
> We have logic in file xact.c::PrepareTransaction() to simply error out if we have accessed any temporary object in the current transaction but because of the above-described issue of not setting XACT_FLAGS_ACCESSEDTEMPNAMESPACE in MyXactFlags second run of “create function..” Works and PREPARE TRANSACTION succeeds(but it should fail).
>
> Please find attached the proposed patch to FIX this issue.

I was able to reproduce the issue with your patch and your patch fixes
the issue.

Few comments:
1) We can drop the table after this test.
+CREATE TEMP TABLE temp_tbl (first TEXT, last TEXT);
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';
+
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';

2) +-- Test for accessing Temporary table
+-- in prepare transaction.
can be changed to
-- Test for accessing cached temporary table in a prepared transaction.

3) +-- These cases must fail and generate errors about Temporary objects.
can be changed to
-- These cases should fail with cannot access temporary object error.

Regards,
Vignesh