Functions Immutable but not parallel safe?
There's 11 functions which are marked immutable, but are marked as
parallel unsafe.
postgres=# select proname from pg_proc where provolatile = 'i' and
proparallel = 'u';
proname
-----------------------------
_pg_expandarray
_pg_keysequal
_pg_truetypid
_pg_truetypmod
_pg_char_max_length
_pg_char_octet_length
_pg_numeric_precision
_pg_numeric_precision_radix
_pg_numeric_scale
_pg_datetime_precision
_pg_interval_type
(11 rows)
I'm finding hard to imagine a reason why these might be unsafe, but
failed. I do notice they're all only used in information_schema.
Could it just perhaps be that these just missed the verification
process the other functions went through to determine their parallel
safety?
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Nov 24, 2016 at 5:29 AM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
There's 11 functions which are marked immutable, but are marked as
parallel unsafe.postgres=# select proname from pg_proc where provolatile = 'i' and
proparallel = 'u';
proname
-----------------------------
_pg_expandarray
_pg_keysequal
_pg_truetypid
_pg_truetypmod
_pg_char_max_length
_pg_char_octet_length
_pg_numeric_precision
_pg_numeric_precision_radix
_pg_numeric_scale
_pg_datetime_precision
_pg_interval_type
(11 rows)I'm finding hard to imagine a reason why these might be unsafe, but
failed. I do notice they're all only used in information_schema.Could it just perhaps be that these just missed the verification
process the other functions went through to determine their parallel
safety?
Yes, I think that's it. I went through pg_proc.h, but never looked at
information_schema.sql.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 11/24/16 18:13, Robert Haas wrote:
I'm finding hard to imagine a reason why these might be unsafe, but
failed. I do notice they're all only used in information_schema.Could it just perhaps be that these just missed the verification
process the other functions went through to determine their parallel
safety?Yes, I think that's it. I went through pg_proc.h, but never looked at
information_schema.sql.
This hasn't been fixed yet. It's easy to to, but taking a step back,
- Is there any reason an immutable function (that is not lying about it)
should be anything but parallel safe?
- If so, could CREATE FUNCTION default it that way?
- Maybe add a check to opr_sanity to make sure the default set of
functions is configured the way we want?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Apr 5, 2017 at 8:57 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 11/24/16 18:13, Robert Haas wrote:
I'm finding hard to imagine a reason why these might be unsafe, but
failed. I do notice they're all only used in information_schema.Could it just perhaps be that these just missed the verification
process the other functions went through to determine their parallel
safety?Yes, I think that's it. I went through pg_proc.h, but never looked at
information_schema.sql.This hasn't been fixed yet. It's easy to to, but taking a step back,
- Is there any reason an immutable function (that is not lying about it)
should be anything but parallel safe?
It certainly isn't very likely. It's not outright impossible. For
example, imagine a function that does a calculation which is
deterministic given the inputs but which creates and uses temporary
tables in the course of performing the calculation. Because the
function performs writes, it's parallel-unsafe.
- If so, could CREATE FUNCTION default it that way?
This could be done but I'm not sure whether it's wise to make the
default value for one parameter depend on another parameter.
- Maybe add a check to opr_sanity to make sure the default set of
functions is configured the way we want?
That seems like a good idea.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Apr 5, 2017 at 8:57 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:- Maybe add a check to opr_sanity to make sure the default set of
functions is configured the way we want?
That seems like a good idea.
+1 for that. We could adopt the strategy already used in opr_sanity of
searching for functions having an unexpected combination of these
attributes. If there are any legitimate exceptions, they could be
embedded in the expected output.
I concur that changing the behavior of CREATE FUNCTION seems a bit too
cute.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/5/17 09:58, Robert Haas wrote:
- Maybe add a check to opr_sanity to make sure the default set of
functions is configured the way we want?That seems like a good idea.
patch
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Mark-immutable-functions-in-information-schema-as-pa.patchinvalid/octet-stream; name=0001-Mark-immutable-functions-in-information-schema-as-pa.patchDownload
From 66d5791deb2ac0434e3e93eded628cd23e3edc33 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Wed, 5 Apr 2017 12:17:03 -0400
Subject: [PATCH] Mark immutable functions in information schema as parallel
safe
Also add opr_sanity check that all immutable functions are parallel
safe.
---
src/backend/catalog/information_schema.sql | 13 +++++++++++--
src/test/regress/expected/opr_sanity.out | 8 ++++++++
src/test/regress/sql/opr_sanity.sql | 5 +++++
3 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
index fa2a88fc5c..c8b2bef813 100644
--- a/src/backend/catalog/information_schema.sql
+++ b/src/backend/catalog/information_schema.sql
@@ -42,14 +42,14 @@ CREATE SCHEMA information_schema;
/* Expand any 1-D array into a set with integers 1..N */
CREATE FUNCTION _pg_expandarray(IN anyarray, OUT x anyelement, OUT n int)
RETURNS SETOF RECORD
- LANGUAGE sql STRICT IMMUTABLE
+ LANGUAGE sql STRICT IMMUTABLE PARALLEL SAFE
AS 'select $1[s], s - pg_catalog.array_lower($1,1) + 1
from pg_catalog.generate_series(pg_catalog.array_lower($1,1),
pg_catalog.array_upper($1,1),
1) as g(s)';
CREATE FUNCTION _pg_keysequal(smallint[], smallint[]) RETURNS boolean
- LANGUAGE sql IMMUTABLE -- intentionally not STRICT, to allow inlining
+ LANGUAGE sql IMMUTABLE PARALLEL SAFE -- intentionally not STRICT, to allow inlining
AS 'select $1 operator(pg_catalog.<@) $2 and $2 operator(pg_catalog.<@) $1';
/* Given an index's OID and an underlying-table column number, return the
@@ -66,6 +66,7 @@ CREATE FUNCTION _pg_index_position(oid, smallint) RETURNS int
CREATE FUNCTION _pg_truetypid(pg_attribute, pg_type) RETURNS oid
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT CASE WHEN $2.typtype = 'd' THEN $2.typbasetype ELSE $1.atttypid END$$;
@@ -73,6 +74,7 @@ CREATE FUNCTION _pg_truetypid(pg_attribute, pg_type) RETURNS oid
CREATE FUNCTION _pg_truetypmod(pg_attribute, pg_type) RETURNS int4
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT CASE WHEN $2.typtype = 'd' THEN $2.typtypmod ELSE $1.atttypmod END$$;
@@ -82,6 +84,7 @@ CREATE FUNCTION _pg_truetypmod(pg_attribute, pg_type) RETURNS int4
CREATE FUNCTION _pg_char_max_length(typid oid, typmod int4) RETURNS integer
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT
@@ -97,6 +100,7 @@ CREATE FUNCTION _pg_char_max_length(typid oid, typmod int4) RETURNS integer
CREATE FUNCTION _pg_char_octet_length(typid oid, typmod int4) RETURNS integer
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT
@@ -112,6 +116,7 @@ CREATE FUNCTION _pg_char_octet_length(typid oid, typmod int4) RETURNS integer
CREATE FUNCTION _pg_numeric_precision(typid oid, typmod int4) RETURNS integer
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT
@@ -132,6 +137,7 @@ CREATE FUNCTION _pg_numeric_precision(typid oid, typmod int4) RETURNS integer
CREATE FUNCTION _pg_numeric_precision_radix(typid oid, typmod int4) RETURNS integer
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT
@@ -143,6 +149,7 @@ CREATE FUNCTION _pg_numeric_precision_radix(typid oid, typmod int4) RETURNS inte
CREATE FUNCTION _pg_numeric_scale(typid oid, typmod int4) RETURNS integer
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT
@@ -158,6 +165,7 @@ CREATE FUNCTION _pg_numeric_scale(typid oid, typmod int4) RETURNS integer
CREATE FUNCTION _pg_datetime_precision(typid oid, typmod int4) RETURNS integer
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT
@@ -173,6 +181,7 @@ CREATE FUNCTION _pg_datetime_precision(typid oid, typmod int4) RETURNS integer
CREATE FUNCTION _pg_interval_type(typid oid, mod int4) RETURNS text
LANGUAGE sql
IMMUTABLE
+ PARALLEL SAFE
RETURNS NULL ON NULL INPUT
AS
$$SELECT
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 262036ac4f..1da5598dc4 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -733,6 +733,14 @@ order by 1;
lowrite | 955
(13 rows)
+-- Check that all immutable functions are marked parallel safe
+SELECT p1.oid, p1.proname
+FROM pg_proc AS p1
+WHERE provolatile = 'i' AND proparallel = 'u';
+ oid | proname
+-----+---------
+(0 rows)
+
-- **************** pg_cast ****************
-- Catch bogus values in pg_cast columns (other than cases detected by
-- oidjoins test).
diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql
index bb9b6bb3b8..123e3bb5c1 100644
--- a/src/test/regress/sql/opr_sanity.sql
+++ b/src/test/regress/sql/opr_sanity.sql
@@ -381,6 +381,11 @@
where nspname = 'pg_catalog')
order by 1;
+-- Check that all immutable functions are marked parallel safe
+SELECT p1.oid, p1.proname
+FROM pg_proc AS p1
+WHERE provolatile = 'i' AND proparallel = 'u';
+
-- **************** pg_cast ****************
--
2.12.2
On Wed, Apr 5, 2017 at 12:18 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
On 4/5/17 09:58, Robert Haas wrote:
- Maybe add a check to opr_sanity to make sure the default set of
functions is configured the way we want?That seems like a good idea.
patch
LGTM
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
On 4/5/17 09:58, Robert Haas wrote:
- Maybe add a check to opr_sanity to make sure the default set of
functions is configured the way we want?
That seems like a good idea.
patch
Looks sane to me, although I wonder if opr_sanity ought to be looking
for any other combinations.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 4/5/17 12:26, Robert Haas wrote:
On Wed, Apr 5, 2017 at 12:18 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:On 4/5/17 09:58, Robert Haas wrote:
- Maybe add a check to opr_sanity to make sure the default set of
functions is configured the way we want?That seems like a good idea.
patch
LGTM
committed
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers