On Wed, Oct 30, 2024 at 7:32 PM Rafia Sabih <rafia.pghack...@gmail.com> wrote:
> Good catch. I agree with this being an unwarranted behaviour. > A minor comment from my end is the wording of the error message. > Based on the Postgresql error message style huide, something like this could > be better, > "could not access temporary relations of other sessions". > -- > Regards, > Rafia Sabih > CYBERTEC PostgreSQL International GmbH > Thanks for your comment. I attach a patch with a fixed error message. Also you can find it in commit fest (https://commitfest.postgresql.org/51/5379/) -- Best Regards, Daniil Davydov
From 2c2e1b2fd75d38d43fbec5023691b93dceb18dbb Mon Sep 17 00:00:00 2001 From: Daniil Davidov <davydovdaniil...@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..1a8e8c8844 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("could not access temporary relations of other sessions"))); } + + 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