Re: Shouldn't current_schema() be at least PARALLEL RESTRICTED?

[Michael Paquier]

On Fri, Jan 18, 2019 at 03:30:16PM -0500, Robert Haas wrote:
> It seems like, as currently implemented, the function is
> parallel-unsafe, because any inserts, updates, or deletes are
> currently parallel-unsafe, including insertions into catalogs.  But I
> am a bit confused why a function that is called current_schema() ends
> up creating a temps schema.

This is documented in namespace.c which needs tricks related to
search_path if referring directly to 'pg_temp', especially if the
namespace creation is marked as pending because its creation cannot
happen outside a transaction context, and the initialization
processing of search_path happens out of that.

Please let me suggest the attached patch then to switch the function
as parallel unsafe, for HEAD.  That still seems like the best way of
course for now after sleeping on it.

Thoughts?
--
Michael

diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 3ecc2e12c3..4aec656099 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -2785,11 +2785,11 @@
   prosrc => 'name_text' },
 
 { oid => '1402', descr => 'current schema name',
-  proname => 'current_schema', provolatile => 's', prorettype => 'name',
-  proargtypes => '', prosrc => 'current_schema' },
+  proname => 'current_schema', provolatile => 's', proparallel => 'u',
+  prorettype => 'name', proargtypes => '', prosrc => 'current_schema' },
 { oid => '1403', descr => 'current schema search list',
-  proname => 'current_schemas', provolatile => 's', prorettype => '_name',
-  proargtypes => 'bool', prosrc => 'current_schemas' },
+  proname => 'current_schemas', provolatile => 's', proparallel => 'u',
+  prorettype => '_name', proargtypes => 'bool', prosrc => 'current_schemas' },
 
 { oid => '1404', descr => 'substitute portion of string',
   proname => 'overlay', prorettype => 'text',
diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out
index d6d8f25141..860f58a3bf 100644
--- a/src/test/regress/expected/temp.out
+++ b/src/test/regress/expected/temp.out
@@ -360,11 +360,8 @@ prepare transaction 'twophase_tab';
 ERROR:  cannot PREPARE a transaction that has operated on temporary tables
 -- Corner case: current_schema may create a temporary schema if namespace
 -- creation is pending, so check after that.  First reset the connection
--- to remove the temporary namespace, and make sure that non-parallel plans
--- are used.
+-- to remove the temporary namespace.
 \c -
-SET max_parallel_workers = 0;
-SET max_parallel_workers_per_gather = 0;
 SET search_path TO 'pg_temp';
 BEGIN;
 SELECT current_schema() ~ 'pg_temp' AS is_temp_schema;
diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql
index 17366a33a9..e634ddb9ca 100644
--- a/src/test/regress/sql/temp.sql
+++ b/src/test/regress/sql/temp.sql
@@ -274,11 +274,8 @@ prepare transaction 'twophase_tab';
 
 -- Corner case: current_schema may create a temporary schema if namespace
 -- creation is pending, so check after that.  First reset the connection
--- to remove the temporary namespace, and make sure that non-parallel plans
--- are used.
+-- to remove the temporary namespace.
 \c -
-SET max_parallel_workers = 0;
-SET max_parallel_workers_per_gather = 0;
 SET search_path TO 'pg_temp';
 BEGIN;
 SELECT current_schema() ~ 'pg_temp' AS is_temp_schema;

signature.asc
Description: PGP signature