Re: CREATE TABLE .. LIKE copies comments to an unrelated table

Hi,

While reviewing v1, I noticed a behavior change when a temporary schema
is renamed concurrently. Would it make sense to qualify temporary table
names with pg_temp rather than the actual schema name?

The following sequence reproduces it with two connections to the same
database:

-- A
CREATE TABLE public.src (a int);
CREATE TEMP TABLE sentinel (a int);
SELECT nspname FROM pg_namespace WHERE oid = pg_my_temp_schema();

-- B
BEGIN;
LOCK TABLE public.src IN ACCESS EXCLUSIVE MODE;

-- A: wait until this statement is blocked before continuing in B.
CREATE TEMP TABLE dst (LIKE public.src);

-- B: replace pg_temp_N with the name returned by A's SELECT.
ALTER SCHEMA pg_temp_N RENAME TO renamed_temp_for_review;
COMMIT;

With v1, A fails with "schema \"pg_temp_N\" does not exist", because the
schema name saved by transformCreateStmt() becomes stale while waiting
for the lock. The same sequence succeeds without the patch.

This requires a superuser, and permanent tables can already encounter
the same error when their destination schema is renamed concurrently.
Therefore, if the intention is to match the name-based behavior of permanent
tables, I think this is reasonable.

Or would it be preferable to use pg_temp for temporary tables, so that
the generated COMMENT commands are schema-qualified while preserving
the previous behavior (i.e., the behavior of current master) in this case?

If we choose pg_temp, the additional change on top of v1 would be:
Permanent tables would still be qualified with the actual schema name.

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -229,7 +229,9 @@
* COMMENTS.
*/
if (stmt->relation->schemaname == NULL)
- stmt->relation->schemaname = get_namespace_name(namespaceid);
+ stmt->relation->schemaname =
+ (stmt->relation->relpersistence == RELPERSISTENCE_TEMP) ?
+ pstrdup("pg_temp") : get_namespace_name(namespaceid);

/* Set up CreateStmtContext */

Regards,

Koshi Shibagaki
FUJITSU LIMITED
https://www.fujitsu.com/