From 465d7e1882bc1f316c7cb2a68e751c34b403e8d7 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 5 Nov 2014 11:44:06 -0500 Subject: [PATCH] Make CREATE TYPE print warnings if a datatype's I/O functions are volatile. This is a followup to commit 43ac12c6e6e397fd9142ed908447eba32d3785b2, which added regression tests checking that I/O functions of built-in types are not marked volatile. Complaining in CREATE TYPE should push developers of add-on types to fix any misdeclared functions in their types. It's just a warning not an error, to avoid creating upgrade problems for what might be just cosmetic mis-markings. Aside from adding the warning code, fix a number of types that were sloppily created in the regression tests. --- src/backend/commands/typecmds.c | 46 +++++++++++++++++++ src/test/regress/expected/create_cast.out | 4 +- src/test/regress/expected/create_type.out | 8 ++-- .../regress/input/create_function_1.source | 8 ++-- .../regress/output/create_function_1.source | 8 ++-- src/test/regress/sql/create_cast.sql | 4 +- src/test/regress/sql/create_type.sql | 8 ++-- 7 files changed, 66 insertions(+), 20 deletions(-) diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 55a68810f23..cbb65f8921a 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -547,6 +547,52 @@ DefineType(List *names, List *parameters) NameListToString(analyzeName)); #endif + /* + * Print warnings if any of the type's I/O functions are marked volatile. + * There is a general assumption that I/O functions are stable or + * immutable; this allows us for example to mark record_in/record_out + * stable rather than volatile. Ideally we would throw errors not just + * warnings here; but since this check is new as of 9.5, and since the + * volatility marking might be just an error-of-omission and not a true + * indication of how the function behaves, we'll let it pass as a warning + * for now. + */ + if (inputOid && func_volatile(inputOid) == PROVOLATILE_VOLATILE) + ereport(WARNING, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type input function %s should not be volatile", + NameListToString(inputName)))); + if (outputOid && func_volatile(outputOid) == PROVOLATILE_VOLATILE) + ereport(WARNING, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type output function %s should not be volatile", + NameListToString(outputName)))); + if (receiveOid && func_volatile(receiveOid) == PROVOLATILE_VOLATILE) + ereport(WARNING, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type receive function %s should not be volatile", + NameListToString(receiveName)))); + if (sendOid && func_volatile(sendOid) == PROVOLATILE_VOLATILE) + ereport(WARNING, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type send function %s should not be volatile", + NameListToString(sendName)))); + if (typmodinOid && func_volatile(typmodinOid) == PROVOLATILE_VOLATILE) + ereport(WARNING, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type modifier input function %s should not be volatile", + NameListToString(typmodinName)))); + if (typmodoutOid && func_volatile(typmodoutOid) == PROVOLATILE_VOLATILE) + ereport(WARNING, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type modifier output function %s should not be volatile", + NameListToString(typmodoutName)))); + + /* + * OK, we're done checking, time to make the type. We must assign the + * array type OID ahead of calling TypeCreate, since the base type and + * array type each refer to the other. + */ array_oid = AssignTypeArrayOid(); /* diff --git a/src/test/regress/expected/create_cast.out b/src/test/regress/expected/create_cast.out index 56cd86e00bf..88f94a63b48 100644 --- a/src/test/regress/expected/create_cast.out +++ b/src/test/regress/expected/create_cast.out @@ -6,12 +6,12 @@ CREATE TYPE casttesttype; CREATE FUNCTION casttesttype_in(cstring) RETURNS casttesttype AS 'textin' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; NOTICE: return type casttesttype is only a shell CREATE FUNCTION casttesttype_out(casttesttype) RETURNS cstring AS 'textout' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; NOTICE: argument type casttesttype is only a shell CREATE TYPE casttesttype ( internallength = variable, diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out index 6dfe9169854..35e8f5d6a25 100644 --- a/src/test/regress/expected/create_type.out +++ b/src/test/regress/expected/create_type.out @@ -41,22 +41,22 @@ CREATE TYPE text_w_default; CREATE FUNCTION int42_in(cstring) RETURNS int42 AS 'int4in' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; NOTICE: return type int42 is only a shell CREATE FUNCTION int42_out(int42) RETURNS cstring AS 'int4out' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; NOTICE: argument type int42 is only a shell CREATE FUNCTION text_w_default_in(cstring) RETURNS text_w_default AS 'textin' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; NOTICE: return type text_w_default is only a shell CREATE FUNCTION text_w_default_out(text_w_default) RETURNS cstring AS 'textout' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; NOTICE: argument type text_w_default is only a shell CREATE TYPE int42 ( internallength = 4, diff --git a/src/test/regress/input/create_function_1.source b/src/test/regress/input/create_function_1.source index 1fded846a05..f2b1561cc24 100644 --- a/src/test/regress/input/create_function_1.source +++ b/src/test/regress/input/create_function_1.source @@ -5,22 +5,22 @@ CREATE FUNCTION widget_in(cstring) RETURNS widget AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C STRICT; + LANGUAGE C STRICT IMMUTABLE; CREATE FUNCTION widget_out(widget) RETURNS cstring AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C STRICT; + LANGUAGE C STRICT IMMUTABLE; CREATE FUNCTION int44in(cstring) RETURNS city_budget AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C STRICT; + LANGUAGE C STRICT IMMUTABLE; CREATE FUNCTION int44out(city_budget) RETURNS cstring AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C STRICT; + LANGUAGE C STRICT IMMUTABLE; CREATE FUNCTION check_primary_key () RETURNS trigger diff --git a/src/test/regress/output/create_function_1.source b/src/test/regress/output/create_function_1.source index 9926c9073e7..30c2936f8d1 100644 --- a/src/test/regress/output/create_function_1.source +++ b/src/test/regress/output/create_function_1.source @@ -4,24 +4,24 @@ CREATE FUNCTION widget_in(cstring) RETURNS widget AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C STRICT; + LANGUAGE C STRICT IMMUTABLE; NOTICE: type "widget" is not yet defined DETAIL: Creating a shell type definition. CREATE FUNCTION widget_out(widget) RETURNS cstring AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C STRICT; + LANGUAGE C STRICT IMMUTABLE; NOTICE: argument type widget is only a shell CREATE FUNCTION int44in(cstring) RETURNS city_budget AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C STRICT; + LANGUAGE C STRICT IMMUTABLE; NOTICE: type "city_budget" is not yet defined DETAIL: Creating a shell type definition. CREATE FUNCTION int44out(city_budget) RETURNS cstring AS '@libdir@/regress@DLSUFFIX@' - LANGUAGE C STRICT; + LANGUAGE C STRICT IMMUTABLE; NOTICE: argument type city_budget is only a shell CREATE FUNCTION check_primary_key () RETURNS trigger diff --git a/src/test/regress/sql/create_cast.sql b/src/test/regress/sql/create_cast.sql index ad348daa097..b11cf88b064 100644 --- a/src/test/regress/sql/create_cast.sql +++ b/src/test/regress/sql/create_cast.sql @@ -8,11 +8,11 @@ CREATE TYPE casttesttype; CREATE FUNCTION casttesttype_in(cstring) RETURNS casttesttype AS 'textin' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; CREATE FUNCTION casttesttype_out(casttesttype) RETURNS cstring AS 'textout' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; CREATE TYPE casttesttype ( internallength = variable, diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql index a4906b64e10..96a075b0267 100644 --- a/src/test/regress/sql/create_type.sql +++ b/src/test/regress/sql/create_type.sql @@ -44,19 +44,19 @@ CREATE TYPE text_w_default; CREATE FUNCTION int42_in(cstring) RETURNS int42 AS 'int4in' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; CREATE FUNCTION int42_out(int42) RETURNS cstring AS 'int4out' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; CREATE FUNCTION text_w_default_in(cstring) RETURNS text_w_default AS 'textin' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; CREATE FUNCTION text_w_default_out(text_w_default) RETURNS cstring AS 'textout' - LANGUAGE internal STRICT; + LANGUAGE internal STRICT IMMUTABLE; CREATE TYPE int42 ( internallength = 4,