Hi,
Thanks for your comments, I appreciate them.
As I continued to deal with the topic of working with temp tables of
other sessions, I noticed something like a bug. For example
(REL_17_STABLE):
Session 1:
=# CREATE TEMP TABLE test(id int);
Session 2:
=# INSERT INTO pg_temp_0.test VALUES (1);
=# INSERT INTO pg_temp_0.test VALUES (2);
Second INSERT command will end with an error "cannot access temporary
tables of other sessions". I checked why this is happening and found
errors in several places.
So, I attach two files to this email :
1) Isolation test, that shows an error in REL_17_STABLE (iso_1.patch)
2) Patch that fixes code that mistakenly considered temporary tables
to be permanent (I will be glad to receive feedback on these fixes) +
isolation test, which shows that now any action with temp table of
other session leads to error (temp_tbl_fix.patch)
Tests look kinda ugly, but I think it's inevitable, given that we
don't know exactly what the name of the temporary schema of other
session will be.
--
Best regards,
Daniil Davydov
From 995002793b0af57b660565e6027615c083d8ce5e Mon Sep 17 00:00:00 2001
From: Daniil Davidov <davydovdaniil501@gmail.com>
Date: Mon, 28 Oct 2024 17:19:07 +0700
Subject: [PATCH] Add isolaton test for work with other session's temp table
---
src/test/isolation/expected/temp-tables.out | 75 +++++++++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/temp-tables.spec | 100 ++++++++++++++++++++
3 files changed, 176 insertions(+)
create mode 100644 src/test/isolation/expected/temp-tables.out
create mode 100644 src/test/isolation/specs/temp-tables.spec
diff --git a/src/test/isolation/expected/temp-tables.out b/src/test/isolation/expected/temp-tables.out
new file mode 100644
index 0000000000..dbd6ffc2e5
--- /dev/null
+++ b/src/test/isolation/expected/temp-tables.out
@@ -0,0 +1,75 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_st1 s1_st2 s2_st1 s1_st3 s2_st2
+step s1_st1: CREATE TEMP TABLE test_tmp(id int);
+step s1_st2: INSERT INTO temp_schema_id SELECT pg_my_temp_schema();
+s2: NOTICE: cannot access temporary tables of other sessions
+s2: NOTICE: cannot access temporary tables of other sessions
+step s2_st1:
+ DO $$
+ DECLARE
+ schema_name text;
+ BEGIN
+ -- Find out name of temporary schema of first session
+ SELECT nspname INTO schema_name
+ FROM pg_namespace
+ WHERE oid = (SELECT oid FROM temp_schema_id LIMIT 1);
+
+ -- Execute few insert operations. We expect that behavior to be the
+ -- same (both will complete successfully or both will fail). Since
+ -- we have RELATION_IS_OTHER_TEMP() check in PrefetchBuffer and
+ -- ReadBufferExtended functions (src/backend/storage/buffer/bufmgr.c),
+ -- let's assume that both operations must fail (this is reflected
+ -- in expected file)
+ BEGIN
+ EXECUTE format('INSERT INTO %I.test_tmp VALUES (1);', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ BEGIN
+ EXECUTE format('INSERT INTO %I.test_tmp VALUES (2);', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+ END
+ $$;
+
+step s1_st3: INSERT INTO test_tmp VALUES (3);
+s2: NOTICE: (3)
+s2: NOTICE: cannot access temporary tables of other sessions
+s2: NOTICE: cannot access temporary tables of other sessions
+step s2_st2:
+ DO $$
+ DECLARE
+ schema_name text;
+ result RECORD;
+ BEGIN
+ -- Find out name of temporary schema of first session
+ SELECT nspname INTO schema_name
+ FROM pg_namespace
+ WHERE oid = (SELECT oid FROM temp_schema_id LIMIT 1);
+
+ -- Before this step call, first session inserted few tuples into
+ -- test_tmp table. Let's assume that SELECT result must contain all
+ -- of these tuples (based on current logic)
+ FOR result IN
+ EXECUTE format('SELECT * FROM %I.test_tmp;', schema_name)
+ LOOP
+ RAISE NOTICE '%', result;
+ END LOOP;
+
+ -- Now lets try to update or delete tuples from test_tmp. If we
+ -- cannot insert into this table, lets assume that both UPDATE and
+ -- DELETE operations must return same error as INSERT
+ BEGIN
+ EXECUTE format('UPDATE %I.test_tmp SET id = 100 WHERE id = 3;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+ END
+ $$;
+
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 143109aa4d..8ed3f821a4 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -115,3 +115,4 @@ test: serializable-parallel-2
test: serializable-parallel-3
test: matview-write-skew
test: lock-nowait
+test: temp-tables
\ No newline at end of file
diff --git a/src/test/isolation/specs/temp-tables.spec b/src/test/isolation/specs/temp-tables.spec
new file mode 100644
index 0000000000..e77c871c79
--- /dev/null
+++ b/src/test/isolation/specs/temp-tables.spec
@@ -0,0 +1,100 @@
+# Test access to temporary tables of other sessions
+
+# We need to know oid of first session's temporary schema
+setup { CREATE TABLE temp_schema_id(oid oid); }
+
+teardown { DROP TABLE temp_schema_id; }
+
+session s1
+
+# In first session we only need to create temporary table and "remember" its oid
+step s1_st1 { CREATE TEMP TABLE test_tmp(id int); }
+step s1_st2 { INSERT INTO temp_schema_id SELECT pg_my_temp_schema(); }
+step s1_st3 { INSERT INTO test_tmp VALUES (3); }
+
+session s2
+
+# In this DO block we will try to access to temp table created before via
+# INSERT operation
+step s2_st1
+{
+ DO $$
+ DECLARE
+ schema_name text;
+ BEGIN
+ -- Find out name of temporary schema of first session
+ SELECT nspname INTO schema_name
+ FROM pg_namespace
+ WHERE oid = (SELECT oid FROM temp_schema_id LIMIT 1);
+
+ -- Execute few insert operations. We expect that behavior to be the
+ -- same (both will complete successfully or both will fail). Since
+ -- we have RELATION_IS_OTHER_TEMP() check in PrefetchBuffer and
+ -- ReadBufferExtended functions (src/backend/storage/buffer/bufmgr.c),
+ -- let's assume that both operations must fail (this is reflected
+ -- in expected file)
+ BEGIN
+ EXECUTE format('INSERT INTO %I.test_tmp VALUES (1);', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ BEGIN
+ EXECUTE format('INSERT INTO %I.test_tmp VALUES (2);', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+ END
+ $$;
+}
+
+# In this DO block we will try to access to temp table created before via
+# SELECT/UPDATE/DELETE operations
+step s2_st2
+{
+ DO $$
+ DECLARE
+ schema_name text;
+ result RECORD;
+ BEGIN
+ -- Find out name of temporary schema of first session
+ SELECT nspname INTO schema_name
+ FROM pg_namespace
+ WHERE oid = (SELECT oid FROM temp_schema_id LIMIT 1);
+
+ -- Before this step call, first session inserted few tuples into
+ -- test_tmp table. Let's assume that SELECT result must contain all
+ -- of these tuples (based on current logic)
+ FOR result IN
+ EXECUTE format('SELECT * FROM %I.test_tmp;', schema_name) LOOP
+ RAISE NOTICE '%', result;
+ END LOOP;
+
+ -- Now lets try to update or delete tuples from test_tmp. If we
+ -- cannot insert into this table, lets assume that both UPDATE and
+ -- DELETE operations must return same error as INSERT
+ BEGIN
+ EXECUTE format('UPDATE %I.test_tmp SET id = 100 WHERE id = 3;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ BEGIN
+ EXECUTE format('DELETE FROM %I.test_tmp WHERE id = 3;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+ END
+ $$;
+}
+
+permutation
+ s1_st1
+ s1_st2
+ s2_st1
+ s1_st3
+ s2_st2
\ No newline at end of file
--
2.43.0
From abc0da111060b4e63f4e83835859e9322c397ee4 Mon Sep 17 00:00:00 2001
From: Daniil Davidov <davydovdaniil501@gmail.com>
Date: Sun, 27 Oct 2024 16:15:50 +0700
Subject: [PATCH] Add fixes so now we cannot access to temporary tables of
other sessions
---
src/backend/catalog/namespace.c | 52 ++++------
src/backend/nodes/makefuncs.c | 6 +-
src/backend/parser/gram.y | 11 ++-
src/test/isolation/expected/temp-tables.out | 92 +++++++++++++++++
src/test/isolation/isolation_schedule | 1 +
src/test/isolation/specs/temp-tables.spec | 103 ++++++++++++++++++++
6 files changed, 231 insertions(+), 34 deletions(-)
create mode 100644 src/test/isolation/expected/temp-tables.out
create mode 100644 src/test/isolation/specs/temp-tables.spec
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 30807f9190..c06d978afc 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -499,28 +499,19 @@ RangeVarGetRelidExtended(const RangeVar *relation, LOCKMODE lockmode,
*/
if (relation->relpersistence == RELPERSISTENCE_TEMP)
{
- if (!OidIsValid(myTempNamespace))
- relId = InvalidOid; /* this probably can't happen? */
- else
+ if (relation->schemaname)
{
- if (relation->schemaname)
- {
- Oid namespaceId;
+ Oid namespaceId;
- namespaceId = LookupExplicitNamespace(relation->schemaname, missing_ok);
-
- /*
- * For missing_ok, allow a non-existent schema name to
- * return InvalidOid.
- */
- if (namespaceId != myTempNamespace)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
- errmsg("temporary tables cannot specify a schema name")));
- }
-
- relId = get_relname_relid(relation->relname, myTempNamespace);
+ namespaceId = LookupExplicitNamespace(relation->schemaname,
+ missing_ok);
+ if (namespaceId != myTempNamespace)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("you cannot access to temporary relations of other session")));
}
+
+ relId = get_relname_relid(relation->relname, myTempNamespace);
}
else if (relation->schemaname)
{
@@ -3387,17 +3378,14 @@ LookupExplicitNamespace(const char *nspname, bool missing_ok)
Oid namespaceId;
AclResult aclresult;
- /* check for pg_temp alias */
if (strcmp(nspname, "pg_temp") == 0)
{
- if (OidIsValid(myTempNamespace))
- return myTempNamespace;
-
/*
- * Since this is used only for looking up existing objects, there is
- * no point in trying to initialize the temp namespace here; and doing
- * so might create problems for some callers --- just fall through.
+ * If namespace specified in 'pg_temp' format
+ * (without owner proc number specification), we assume that this is
+ * our temporary namespace
*/
+ return myTempNamespace;
}
namespaceId = get_namespace_oid(nspname, missing_ok);
@@ -3553,21 +3541,19 @@ get_namespace_oid(const char *nspname, bool missing_ok)
RangeVar *
makeRangeVarFromNameList(const List *names)
{
- RangeVar *rel = makeRangeVar(NULL, NULL, -1);
+ RangeVar *rel;
switch (list_length(names))
{
case 1:
- rel->relname = strVal(linitial(names));
+ rel = makeRangeVar(NULL, strVal(linitial(names)), -1);
break;
case 2:
- rel->schemaname = strVal(linitial(names));
- rel->relname = strVal(lsecond(names));
+ rel = makeRangeVar(strVal(linitial(names)), strVal(lsecond(names)), -1);
break;
case 3:
+ rel = makeRangeVar(strVal(lsecond(names)), strVal(lthird(names)), -1);
rel->catalogname = strVal(linitial(names));
- rel->schemaname = strVal(lsecond(names));
- rel->relname = strVal(lthird(names));
break;
default:
ereport(ERROR,
@@ -3774,6 +3760,8 @@ GetTempNamespaceProcNumber(Oid namespaceId)
return INVALID_PROC_NUMBER; /* no such namespace? */
if (strncmp(nspname, "pg_temp_", 8) == 0)
result = atoi(nspname + 8);
+ else if (strncmp(nspname, "pg_temp", 7) == 0)
+ result = MyProcNumber;
else if (strncmp(nspname, "pg_toast_temp_", 14) == 0)
result = atoi(nspname + 14);
else
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 61ac172a85..e60044c9d5 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -429,10 +429,14 @@ makeRangeVar(char *schemaname, char *relname, int location)
r->schemaname = schemaname;
r->relname = relname;
r->inh = true;
- r->relpersistence = RELPERSISTENCE_PERMANENT;
r->alias = NULL;
r->location = location;
+ if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0)
+ r->relpersistence = RELPERSISTENCE_TEMP;
+ else
+ r->relpersistence = RELPERSISTENCE_PERMANENT;
+
return r;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index bca627c546..02c13fdd16 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -19184,7 +19184,11 @@ makeRangeVarFromAnyName(List *names, int position, core_yyscan_t yyscanner)
break;
}
- r->relpersistence = RELPERSISTENCE_PERMANENT;
+ if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0)
+ r->relpersistence = RELPERSISTENCE_TEMP;
+ else
+ r->relpersistence = RELPERSISTENCE_PERMANENT;
+
r->location = position;
return r;
@@ -19224,6 +19228,11 @@ makeRangeVarFromQualifiedName(char *name, List *namelist, int location,
break;
}
+ if (r->schemaname && strncmp(r->schemaname, "pg_temp", 7) == 0)
+ r->relpersistence = RELPERSISTENCE_TEMP;
+ else
+ r->relpersistence = RELPERSISTENCE_PERMANENT;
+
return r;
}
diff --git a/src/test/isolation/expected/temp-tables.out b/src/test/isolation/expected/temp-tables.out
new file mode 100644
index 0000000000..328e7d34b9
--- /dev/null
+++ b/src/test/isolation/expected/temp-tables.out
@@ -0,0 +1,92 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1_st1 s1_st2 s2_st1
+step s1_st1: CREATE TEMP TABLE test_tmp(id int);
+step s1_st2: INSERT INTO temp_schema_id SELECT pg_my_temp_schema();
+s2: NOTICE: cannot access temporary tables of other sessions
+s2: NOTICE: cannot access temporary tables of other sessions
+s2: NOTICE: cannot access temporary tables of other sessions
+s2: NOTICE: cannot access temporary tables of other sessions
+s2: NOTICE: cannot access temporary tables of other sessions
+s2: NOTICE: cannot access temporary tables of other sessions
+s2: NOTICE: cannot access temporary tables of other sessions
+step s2_st1:
+ DO $$
+ DECLARE
+ schema_name text;
+ result RECORD;
+ BEGIN
+ -- Find out name of temporary schema of first session
+ SELECT nspname INTO schema_name
+ FROM pg_namespace
+ WHERE oid = (SELECT oid FROM temp_schema_id LIMIT 1);
+
+ -- Both INSERT operation must fail due to 'cannot access temporary
+ -- tables of other sessions' error
+ BEGIN
+ EXECUTE format('INSERT INTO %I.test_tmp VALUES (1);', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ BEGIN
+ EXECUTE format('INSERT INTO %I.test_tmp VALUES (2);', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- SELECT operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ FOR result IN
+ EXECUTE format('SELECT * FROM %I.test_tmp;', schema_name) LOOP
+ RAISE NOTICE '%', result;
+ END LOOP;
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- UPDATE operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ EXECUTE format('UPDATE %I.test_tmp SET id = 100 WHERE id = 3;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- DELETE operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ EXECUTE format('DELETE FROM %I.test_tmp WHERE id = 3;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- LOCK operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ EXECUTE format('BEGIN;
+ LOCK TABLE %I.test_tmp;
+ COMMIT;',
+ schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- DROP operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ EXECUTE format('DROP TABLE %I.test_tmp;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+ END
+ $$;
+
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 143109aa4d..8ed3f821a4 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -115,3 +115,4 @@ test: serializable-parallel-2
test: serializable-parallel-3
test: matview-write-skew
test: lock-nowait
+test: temp-tables
\ No newline at end of file
diff --git a/src/test/isolation/specs/temp-tables.spec b/src/test/isolation/specs/temp-tables.spec
new file mode 100644
index 0000000000..009bd9845b
--- /dev/null
+++ b/src/test/isolation/specs/temp-tables.spec
@@ -0,0 +1,103 @@
+# Test access to temporary tables of other sessions
+
+# We need to know oid of first session's temporary schema
+setup { CREATE TABLE temp_schema_id(oid oid); }
+
+teardown { DROP TABLE temp_schema_id; }
+
+session s1
+
+# In first session we only need to create temporary table and "remember" its oid
+step s1_st1 { CREATE TEMP TABLE test_tmp(id int); }
+step s1_st2 { INSERT INTO temp_schema_id SELECT pg_my_temp_schema(); }
+
+session s2
+
+# In this DO block we will try to access to temp table created before via
+# INSERT/UPDATE/DELETE/SELECT/TRUNCATE/LOCK operations
+step s2_st1
+{
+ DO $$
+ DECLARE
+ schema_name text;
+ result RECORD;
+ BEGIN
+ -- Find out name of temporary schema of first session
+ SELECT nspname INTO schema_name
+ FROM pg_namespace
+ WHERE oid = (SELECT oid FROM temp_schema_id LIMIT 1);
+
+ -- Both INSERT operation must fail due to 'cannot access temporary
+ -- tables of other sessions' error
+ BEGIN
+ EXECUTE format('INSERT INTO %I.test_tmp VALUES (1);', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ BEGIN
+ EXECUTE format('INSERT INTO %I.test_tmp VALUES (2);', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- SELECT operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ FOR result IN
+ EXECUTE format('SELECT * FROM %I.test_tmp;', schema_name) LOOP
+ RAISE NOTICE '%', result;
+ END LOOP;
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- UPDATE operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ EXECUTE format('UPDATE %I.test_tmp SET id = 100 WHERE id = 3;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- DELETE operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ EXECUTE format('DELETE FROM %I.test_tmp WHERE id = 3;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- LOCK operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ EXECUTE format('BEGIN;
+ LOCK TABLE %I.test_tmp;
+ COMMIT;',
+ schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+
+ -- DROP operation must fail due to 'cannot access temporary tables
+ -- of other sessions' error
+ BEGIN
+ EXECUTE format('DROP TABLE %I.test_tmp;', schema_name);
+ EXCEPTION
+ WHEN feature_not_supported
+ THEN RAISE NOTICE 'cannot access temporary tables of other sessions';
+ END;
+ END
+ $$;
+}
+
+permutation
+ s1_st1
+ s1_st2
+ s2_st1
\ No newline at end of file
--
2.43.0
