Re: Is it time to retire type "opaque"? - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: Is it time to retire type "opaque"? |
Date | |
Msg-id | 10245.1583278766@sss.pgh.pa.us Whole thread Raw |
In response to | Is it time to retire type "opaque"? (Tom Lane <tgl@sss.pgh.pa.us>) |
List | pgsql-hackers |
I wrote: > In short, I propose ripping out OPAQUE entirely. Like so... I separated out the changes in CREATE TYPE because that's a bit more complicated than the rest. The behavior around shell types gets somewhat simpler, and I moved the I/O function result type checks into the lookup functions to make them all consistent. regards, tom lane diff --git a/doc/src/sgml/ref/create_type.sgml b/doc/src/sgml/ref/create_type.sgml index 175315f..111f8e6 100644 --- a/doc/src/sgml/ref/create_type.sgml +++ b/doc/src/sgml/ref/create_type.sgml @@ -823,18 +823,6 @@ CREATE TYPE <replaceable class="parameter">name</replaceable> function is written in C. </para> - <para> - In <productname>PostgreSQL</productname> versions before 7.3, it - was customary to avoid creating a shell type at all, by replacing the - functions' forward references to the type name with the placeholder - pseudo-type <type>opaque</type>. The <type>cstring</type> arguments and - results also had to be declared as <type>opaque</type> before 7.3. To - support loading of old dump files, <command>CREATE TYPE</command> will - accept I/O functions declared using <type>opaque</type>, but it will issue - a notice and change the function declarations to use the correct - types. - </para> - </refsect1> <refsect1> diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 540044b..9279c05 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -1444,50 +1444,6 @@ SetFunctionReturnType(Oid funcOid, Oid newRetType) /* - * SetFunctionArgType - change declared argument type of a function - * - * As above, but change an argument's type. - */ -void -SetFunctionArgType(Oid funcOid, int argIndex, Oid newArgType) -{ - Relation pg_proc_rel; - HeapTuple tup; - Form_pg_proc procForm; - ObjectAddress func_address; - ObjectAddress type_address; - - pg_proc_rel = table_open(ProcedureRelationId, RowExclusiveLock); - - tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid)); - if (!HeapTupleIsValid(tup)) /* should not happen */ - elog(ERROR, "cache lookup failed for function %u", funcOid); - procForm = (Form_pg_proc) GETSTRUCT(tup); - - if (argIndex < 0 || argIndex >= procForm->pronargs || - procForm->proargtypes.values[argIndex] != OPAQUEOID) - elog(ERROR, "function %u doesn't take OPAQUE", funcOid); - - /* okay to overwrite copied tuple */ - procForm->proargtypes.values[argIndex] = newArgType; - - /* update the catalog and its indexes */ - CatalogTupleUpdate(pg_proc_rel, &tup->t_self, tup); - - table_close(pg_proc_rel, RowExclusiveLock); - - /* - * Also update the dependency to the new type. Opaque is a pinned type, so - * there is no old dependency record for it that we would need to remove. - */ - ObjectAddressSet(type_address, TypeRelationId, newArgType); - ObjectAddressSet(func_address, ProcedureRelationId, funcOid); - recordDependencyOn(&func_address, &type_address, DEPENDENCY_NORMAL); -} - - - -/* * CREATE CAST */ ObjectAddress diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 5209736..d6e694e 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -163,7 +163,6 @@ DefineType(ParseState *pstate, List *names, List *parameters) char *array_type; Oid array_oid; Oid typoid; - Oid resulttype; ListCell *pl; ObjectAddress address; @@ -196,8 +195,7 @@ DefineType(ParseState *pstate, List *names, List *parameters) #endif /* - * Look to see if type already exists (presumably as a shell; if not, - * TypeCreate will complain). + * Look to see if type already exists. */ typoid = GetSysCacheOid2(TYPENAMENSP, Anum_pg_type_oid, CStringGetDatum(typeName), @@ -211,35 +209,37 @@ DefineType(ParseState *pstate, List *names, List *parameters) { if (moveArrayTypeName(typoid, typeName, typeNamespace)) typoid = InvalidOid; + else + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("type \"%s\" already exists", typeName))); } /* - * If it doesn't exist, create it as a shell, so that the OID is known for - * use in the I/O function definitions. + * If this command is a parameterless CREATE TYPE, then we're just here to + * make a shell type, so do that (or fail if there already is a shell). */ - if (!OidIsValid(typoid)) + if (parameters == NIL) { - address = TypeShellMake(typeName, typeNamespace, GetUserId()); - typoid = address.objectId; - /* Make new shell type visible for modification below */ - CommandCounterIncrement(); - - /* - * If the command was a parameterless CREATE TYPE, we're done --- - * creating the shell type was all we're supposed to do. - */ - if (parameters == NIL) - return address; - } - else - { - /* Complain if dummy CREATE TYPE and entry already exists */ - if (parameters == NIL) + if (OidIsValid(typoid)) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), errmsg("type \"%s\" already exists", typeName))); + + address = TypeShellMake(typeName, typeNamespace, GetUserId()); + return address; } + /* + * Otherwise, we must already have a shell type, since there is no other + * way that the I/O functions could have been created. + */ + if (!OidIsValid(typoid)) + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("type \"%s\" does not exist", typeName), + errhint("Create the type as a shell type, then create its I/O functions, then do a full CREATE TYPE."))); + /* Extract the parameters from the parameter list */ foreach(pl, parameters) { @@ -445,63 +445,6 @@ DefineType(ParseState *pstate, List *names, List *parameters) sendOid = findTypeSendFunction(sendName, typoid); /* - * Verify that I/O procs return the expected thing. If we see OPAQUE, - * complain and change it to the correct type-safe choice. - */ - resulttype = get_func_rettype(inputOid); - if (resulttype != typoid) - { - if (resulttype == OPAQUEOID) - { - /* backwards-compatibility hack */ - ereport(WARNING, - (errmsg("changing return type of function %s from %s to %s", - NameListToString(inputName), "opaque", typeName))); - SetFunctionReturnType(inputOid, typoid); - } - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("type input function %s must return type %s", - NameListToString(inputName), typeName))); - } - resulttype = get_func_rettype(outputOid); - if (resulttype != CSTRINGOID) - { - if (resulttype == OPAQUEOID) - { - /* backwards-compatibility hack */ - ereport(WARNING, - (errmsg("changing return type of function %s from %s to %s", - NameListToString(outputName), "opaque", "cstring"))); - SetFunctionReturnType(outputOid, CSTRINGOID); - } - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("type output function %s must return type %s", - NameListToString(outputName), "cstring"))); - } - if (receiveOid) - { - resulttype = get_func_rettype(receiveOid); - if (resulttype != typoid) - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("type receive function %s must return type %s", - NameListToString(receiveName), typeName))); - } - if (sendOid) - { - resulttype = get_func_rettype(sendOid); - if (resulttype != BYTEAOID) - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("type send function %s must return type %s", - NameListToString(sendName), "bytea"))); - } - - /* * Convert typmodin/out function proc names to OIDs. */ if (typmodinName) @@ -1404,16 +1347,9 @@ DefineRange(CreateRangeStmt *stmt) } /* - * If it doesn't exist, create it as a shell, so that the OID is known for - * use in the range function definitions. + * Unlike DefineType(), we don't insist on a shell type existing first, as + * it's only needed if the user wants to specify a canonical function. */ - if (!OidIsValid(typoid)) - { - address = TypeShellMake(typeName, typeNamespace, GetUserId()); - typoid = address.objectId; - /* Make new shell type visible for modification below */ - CommandCounterIncrement(); - } /* Extract the parameters from the parameter list */ foreach(lc, stmt->params) @@ -1502,8 +1438,15 @@ DefineRange(CreateRangeStmt *stmt) /* Identify support functions, if provided */ if (rangeCanonicalName != NIL) + { + if (!OidIsValid(typoid)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("cannot specify a canonical function without a pre-created shell type"), + errhint("Create the type as a shell type, then create its canonicalization function, then do a fullCREATE TYPE."))); rangeCanonical = findRangeCanonicalFunction(rangeCanonicalName, typoid); + } else rangeCanonical = InvalidOid; @@ -1555,7 +1498,8 @@ DefineRange(CreateRangeStmt *stmt) 0, /* Array dimensions of typbasetype */ false, /* Type NOT NULL */ InvalidOid); /* type's collation (ranges never have one) */ - Assert(typoid == address.objectId); + Assert(typoid == InvalidOid || typoid == address.objectId); + typoid = address.objectId; /* Create the entry in pg_range */ RangeCreate(typoid, rangeSubtype, rangeCollation, rangeSubOpclass, @@ -1695,63 +1639,32 @@ findTypeInputFunction(List *procname, Oid typeOid) /* * Input functions can take a single argument of type CSTRING, or three - * arguments (string, typioparam OID, typmod). - * - * For backwards compatibility we allow OPAQUE in place of CSTRING; if we - * see this, we issue a warning and fix up the pg_proc entry. + * arguments (string, typioparam OID, typmod). They must return the + * target type. */ argList[0] = CSTRINGOID; procOid = LookupFuncName(procname, 1, argList, true); - if (OidIsValid(procOid)) - return procOid; - - argList[1] = OIDOID; - argList[2] = INT4OID; - - procOid = LookupFuncName(procname, 3, argList, true); - if (OidIsValid(procOid)) - return procOid; - - /* No luck, try it with OPAQUE */ - argList[0] = OPAQUEOID; - - procOid = LookupFuncName(procname, 1, argList, true); - if (!OidIsValid(procOid)) { argList[1] = OIDOID; argList[2] = INT4OID; procOid = LookupFuncName(procname, 3, argList, true); + if (!OidIsValid(procOid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("function %s does not exist", + func_signature_string(procname, 1, NIL, argList)))); } - if (OidIsValid(procOid)) - { - /* Found, but must complain and fix the pg_proc entry */ - ereport(WARNING, - (errmsg("changing argument type of function %s from \"opaque\" to \"cstring\"", - NameListToString(procname)))); - SetFunctionArgType(procOid, 0, CSTRINGOID); - - /* - * Need CommandCounterIncrement since DefineType will likely try to - * alter the pg_proc tuple again. - */ - CommandCounterIncrement(); - - return procOid; - } - - /* Use CSTRING (preferred) in the error message */ - argList[0] = CSTRINGOID; - - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_FUNCTION), - errmsg("function %s does not exist", - func_signature_string(procname, 1, NIL, argList)))); + if (get_func_rettype(procOid) != typeOid) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type input function %s must return type %s", + NameListToString(procname), format_type_be(typeOid)))); - return InvalidOid; /* keep compiler quiet */ + return procOid; } static Oid @@ -1761,48 +1674,25 @@ findTypeOutputFunction(List *procname, Oid typeOid) Oid procOid; /* - * Output functions can take a single argument of the type. - * - * For backwards compatibility we allow OPAQUE in place of the actual type - * name; if we see this, we issue a warning and fix up the pg_proc entry. + * Output functions always take a single argument of the type and return + * cstring. */ argList[0] = typeOid; procOid = LookupFuncName(procname, 1, argList, true); - if (OidIsValid(procOid)) - return procOid; - - /* No luck, try it with OPAQUE */ - argList[0] = OPAQUEOID; - - procOid = LookupFuncName(procname, 1, argList, true); - - if (OidIsValid(procOid)) - { - /* Found, but must complain and fix the pg_proc entry */ - ereport(WARNING, - (errmsg("changing argument type of function %s from \"opaque\" to %s", - NameListToString(procname), format_type_be(typeOid)))); - SetFunctionArgType(procOid, 0, typeOid); - - /* - * Need CommandCounterIncrement since DefineType will likely try to - * alter the pg_proc tuple again. - */ - CommandCounterIncrement(); - - return procOid; - } - - /* Use type name, not OPAQUE, in the failure message. */ - argList[0] = typeOid; + if (!OidIsValid(procOid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("function %s does not exist", + func_signature_string(procname, 1, NIL, argList)))); - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_FUNCTION), - errmsg("function %s does not exist", - func_signature_string(procname, 1, NIL, argList)))); + if (get_func_rettype(procOid) != CSTRINGOID) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type output function %s must return type %s", + NameListToString(procname), "cstring"))); - return InvalidOid; /* keep compiler quiet */ + return procOid; } static Oid @@ -1813,27 +1703,32 @@ findTypeReceiveFunction(List *procname, Oid typeOid) /* * Receive functions can take a single argument of type INTERNAL, or three - * arguments (internal, typioparam OID, typmod). + * arguments (internal, typioparam OID, typmod). They must return the + * target type. */ argList[0] = INTERNALOID; procOid = LookupFuncName(procname, 1, argList, true); - if (OidIsValid(procOid)) - return procOid; - - argList[1] = OIDOID; - argList[2] = INT4OID; + if (!OidIsValid(procOid)) + { + argList[1] = OIDOID; + argList[2] = INT4OID; - procOid = LookupFuncName(procname, 3, argList, true); - if (OidIsValid(procOid)) - return procOid; + procOid = LookupFuncName(procname, 3, argList, true); + if (!OidIsValid(procOid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("function %s does not exist", + func_signature_string(procname, 1, NIL, argList)))); + } - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_FUNCTION), - errmsg("function %s does not exist", - func_signature_string(procname, 1, NIL, argList)))); + if (get_func_rettype(procOid) != typeOid) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type receive function %s must return type %s", + NameListToString(procname), format_type_be(typeOid)))); - return InvalidOid; /* keep compiler quiet */ + return procOid; } static Oid @@ -1843,20 +1738,25 @@ findTypeSendFunction(List *procname, Oid typeOid) Oid procOid; /* - * Send functions can take a single argument of the type. + * Send functions always take a single argument of the type and return + * bytea. */ argList[0] = typeOid; procOid = LookupFuncName(procname, 1, argList, true); - if (OidIsValid(procOid)) - return procOid; + if (!OidIsValid(procOid)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("function %s does not exist", + func_signature_string(procname, 1, NIL, argList)))); - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_FUNCTION), - errmsg("function %s does not exist", - func_signature_string(procname, 1, NIL, argList)))); + if (get_func_rettype(procOid) != BYTEAOID) + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("type send function %s must return type %s", + NameListToString(procname), "bytea"))); - return InvalidOid; /* keep compiler quiet */ + return procOid; } static Oid diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index dede9d7..0992c23 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -55,7 +55,6 @@ extern Oid ResolveOpClass(List *opclass, Oid attrType, extern ObjectAddress CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt); extern void RemoveFunctionById(Oid funcOid); extern void SetFunctionReturnType(Oid funcOid, Oid newRetType); -extern void SetFunctionArgType(Oid funcOid, int argIndex, Oid newArgType); extern ObjectAddress AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt); extern ObjectAddress CreateCast(CreateCastStmt *stmt); extern void DropCastById(Oid castOid); diff --git a/src/test/regress/expected/create_type.out b/src/test/regress/expected/create_type.out index 8309756..eb55e25 100644 --- a/src/test/regress/expected/create_type.out +++ b/src/test/regress/expected/create_type.out @@ -83,8 +83,10 @@ SELECT * FROM default_test; zippo | 42 (1 row) +-- We need a shell type to test some CREATE TYPE failure cases with +CREATE TYPE bogus_type; -- invalid: non-lowercase quoted identifiers -CREATE TYPE case_int42 ( +CREATE TYPE bogus_type ( "Internallength" = 4, "Input" = int42_in, "Output" = int42_out, @@ -111,6 +113,20 @@ WARNING: type attribute "Passedbyvalue" not recognized LINE 7: "Passedbyvalue" ^ ERROR: type input function must be specified +-- invalid: input/output function incompatibility +CREATE TYPE bogus_type (INPUT = array_in, + OUTPUT = array_out, + ELEMENT = int, + INTERNALLENGTH = 32); +ERROR: type input function array_in must return type bogus_type +DROP TYPE bogus_type; +-- It no longer is possible to issue CREATE TYPE without making a shell first +CREATE TYPE bogus_type (INPUT = array_in, + OUTPUT = array_out, + ELEMENT = int, + INTERNALLENGTH = 32); +ERROR: type "bogus_type" does not exist +HINT: Create the type as a shell type, then create its I/O functions, then do a full CREATE TYPE. -- Test stand-alone composite type CREATE TYPE default_test_row AS (f1 text_w_default, f2 int42); CREATE FUNCTION get_default_test() RETURNS SETOF default_test_row AS ' @@ -137,28 +153,25 @@ ERROR: type "text_w_default" already exists DROP TYPE default_test_row CASCADE; NOTICE: drop cascades to function get_default_test() DROP TABLE default_test; --- Check type create with input/output incompatibility -CREATE TYPE not_existing_type (INPUT = array_in, - OUTPUT = array_out, - ELEMENT = int, - INTERNALLENGTH = 32); -ERROR: function array_out(not_existing_type) does not exist --- Check dependency transfer of opaque functions when creating a new type -CREATE FUNCTION base_fn_in(cstring) RETURNS opaque AS 'boolin' +-- Check dependencies are established when creating a new type +CREATE TYPE base_type; +CREATE FUNCTION base_fn_in(cstring) RETURNS base_type AS 'boolin' LANGUAGE internal IMMUTABLE STRICT; -CREATE FUNCTION base_fn_out(opaque) RETURNS opaque AS 'boolout' +NOTICE: return type base_type is only a shell +CREATE FUNCTION base_fn_out(base_type) RETURNS cstring AS 'boolout' LANGUAGE internal IMMUTABLE STRICT; +NOTICE: argument type base_type is only a shell CREATE TYPE base_type(INPUT = base_fn_in, OUTPUT = base_fn_out); -WARNING: changing argument type of function base_fn_out from "opaque" to base_type -WARNING: changing return type of function base_fn_in from opaque to base_type -WARNING: changing return type of function base_fn_out from opaque to cstring DROP FUNCTION base_fn_in(cstring); -- error ERROR: cannot drop function base_fn_in(cstring) because other objects depend on it DETAIL: type base_type depends on function base_fn_in(cstring) function base_fn_out(base_type) depends on type base_type HINT: Use DROP ... CASCADE to drop the dependent objects too. -DROP FUNCTION base_fn_out(opaque); -- error -ERROR: function base_fn_out(opaque) does not exist +DROP FUNCTION base_fn_out(base_type); -- error +ERROR: cannot drop function base_fn_out(base_type) because other objects depend on it +DETAIL: type base_type depends on function base_fn_out(base_type) +function base_fn_in(cstring) depends on type base_type +HINT: Use DROP ... CASCADE to drop the dependent objects too. DROP TYPE base_type; -- error ERROR: cannot drop type base_type because other objects depend on it DETAIL: function base_fn_in(cstring) depends on type base_type diff --git a/src/test/regress/sql/create_type.sql b/src/test/regress/sql/create_type.sql index 3d1deba..68b04fd 100644 --- a/src/test/regress/sql/create_type.sql +++ b/src/test/regress/sql/create_type.sql @@ -84,8 +84,11 @@ INSERT INTO default_test DEFAULT VALUES; SELECT * FROM default_test; +-- We need a shell type to test some CREATE TYPE failure cases with +CREATE TYPE bogus_type; + -- invalid: non-lowercase quoted identifiers -CREATE TYPE case_int42 ( +CREATE TYPE bogus_type ( "Internallength" = 4, "Input" = int42_in, "Output" = int42_out, @@ -94,6 +97,20 @@ CREATE TYPE case_int42 ( "Passedbyvalue" ); +-- invalid: input/output function incompatibility +CREATE TYPE bogus_type (INPUT = array_in, + OUTPUT = array_out, + ELEMENT = int, + INTERNALLENGTH = 32); + +DROP TYPE bogus_type; + +-- It no longer is possible to issue CREATE TYPE without making a shell first +CREATE TYPE bogus_type (INPUT = array_in, + OUTPUT = array_out, + ELEMENT = int, + INTERNALLENGTH = 32); + -- Test stand-alone composite type CREATE TYPE default_test_row AS (f1 text_w_default, f2 int42); @@ -119,20 +136,15 @@ DROP TYPE default_test_row CASCADE; DROP TABLE default_test; --- Check type create with input/output incompatibility -CREATE TYPE not_existing_type (INPUT = array_in, - OUTPUT = array_out, - ELEMENT = int, - INTERNALLENGTH = 32); - --- Check dependency transfer of opaque functions when creating a new type -CREATE FUNCTION base_fn_in(cstring) RETURNS opaque AS 'boolin' +-- Check dependencies are established when creating a new type +CREATE TYPE base_type; +CREATE FUNCTION base_fn_in(cstring) RETURNS base_type AS 'boolin' LANGUAGE internal IMMUTABLE STRICT; -CREATE FUNCTION base_fn_out(opaque) RETURNS opaque AS 'boolout' +CREATE FUNCTION base_fn_out(base_type) RETURNS cstring AS 'boolout' LANGUAGE internal IMMUTABLE STRICT; CREATE TYPE base_type(INPUT = base_fn_in, OUTPUT = base_fn_out); DROP FUNCTION base_fn_in(cstring); -- error -DROP FUNCTION base_fn_out(opaque); -- error +DROP FUNCTION base_fn_out(base_type); -- error DROP TYPE base_type; -- error DROP TYPE base_type CASCADE; diff --git a/doc/src/sgml/datatype.sgml b/doc/src/sgml/datatype.sgml index d1d0331..410eaed 100644 --- a/doc/src/sgml/datatype.sgml +++ b/doc/src/sgml/datatype.sgml @@ -4827,10 +4827,6 @@ SELECT * FROM pg_attribute <primary>unknown</primary> </indexterm> - <indexterm zone="datatype-pseudo"> - <primary>opaque</primary> - </indexterm> - <para> The <productname>PostgreSQL</productname> type system contains a number of special-purpose entries that are collectively called @@ -4953,12 +4949,6 @@ SELECT * FROM pg_attribute <entry>Identifies a not-yet-resolved type, e.g. of an undecorated string literal.</entry> </row> - - <row> - <entry><type>opaque</type></entry> - <entry>An obsolete type name that formerly served many of the above - purposes.</entry> - </row> </tbody> </tgroup> </table> diff --git a/doc/src/sgml/ref/create_language.sgml b/doc/src/sgml/ref/create_language.sgml index af9d115..2243ee6 100644 --- a/doc/src/sgml/ref/create_language.sgml +++ b/doc/src/sgml/ref/create_language.sgml @@ -211,16 +211,6 @@ CREATE [ OR REPLACE ] [ TRUSTED ] [ PROCEDURAL ] LANGUAGE <replaceable class="pa database, which will cause it to be available automatically in all subsequently-created databases. </para> - - <para> - In <productname>PostgreSQL</productname> versions before 7.3, it was - necessary to declare handler functions as returning the placeholder - type <type>opaque</type>, rather than <type>language_handler</type>. - To support loading - of old dump files, <command>CREATE LANGUAGE</command> will accept a function - declared as returning <type>opaque</type>, but it will issue a notice and - change the function's declared return type to <type>language_handler</type>. - </para> </refsect1> <refsect1 id="sql-createlanguage-examples"> diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml index 3339a4b..3b8f25e 100644 --- a/doc/src/sgml/ref/create_trigger.sgml +++ b/doc/src/sgml/ref/create_trigger.sgml @@ -543,15 +543,6 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</ row-level triggers with transition relations cannot be defined on partitions or inheritance child tables. </para> - - <para> - In <productname>PostgreSQL</productname> versions before 7.3, it was - necessary to declare trigger functions as returning the placeholder - type <type>opaque</type>, rather than <type>trigger</type>. To support loading - of old dump files, <command>CREATE TRIGGER</command> will accept a function - declared as returning <type>opaque</type>, but it will issue a notice and - change the function's declared return type to <type>trigger</type>. - </para> </refsect1> <refsect1 id="sql-createtrigger-examples"> diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 9279c05..e634ccf 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -1399,49 +1399,6 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt) return address; } -/* - * SetFunctionReturnType - change declared return type of a function - * - * This is presently only used for adjusting legacy functions that return - * OPAQUE to return whatever we find their correct definition should be. - * The caller should emit a suitable warning explaining what we did. - */ -void -SetFunctionReturnType(Oid funcOid, Oid newRetType) -{ - Relation pg_proc_rel; - HeapTuple tup; - Form_pg_proc procForm; - ObjectAddress func_address; - ObjectAddress type_address; - - pg_proc_rel = table_open(ProcedureRelationId, RowExclusiveLock); - - tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid)); - if (!HeapTupleIsValid(tup)) /* should not happen */ - elog(ERROR, "cache lookup failed for function %u", funcOid); - procForm = (Form_pg_proc) GETSTRUCT(tup); - - if (procForm->prorettype != OPAQUEOID) /* caller messed up */ - elog(ERROR, "function %u doesn't return OPAQUE", funcOid); - - /* okay to overwrite copied tuple */ - procForm->prorettype = newRetType; - - /* update the catalog and its indexes */ - CatalogTupleUpdate(pg_proc_rel, &tup->t_self, tup); - - table_close(pg_proc_rel, RowExclusiveLock); - - /* - * Also update the dependency to the new type. Opaque is a pinned type, so - * there is no old dependency record for it that we would need to remove. - */ - ObjectAddressSet(type_address, TypeRelationId, newRetType); - ObjectAddressSet(func_address, ProcedureRelationId, funcOid); - recordDependencyOn(&func_address, &type_address, DEPENDENCY_NORMAL); -} - /* * CREATE CAST diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c index 9d72edb..21ceeb7 100644 --- a/src/backend/commands/proclang.c +++ b/src/backend/commands/proclang.c @@ -74,27 +74,10 @@ CreateProceduralLanguage(CreatePLangStmt *stmt) handlerOid = LookupFuncName(stmt->plhandler, 0, NULL, false); funcrettype = get_func_rettype(handlerOid); if (funcrettype != LANGUAGE_HANDLEROID) - { - /* - * We allow OPAQUE just so we can load old dump files. When we see a - * handler function declared OPAQUE, change it to LANGUAGE_HANDLER. - * (This is probably obsolete and removable?) - */ - if (funcrettype == OPAQUEOID) - { - ereport(WARNING, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("changing return type of function %s from %s to %s", - NameListToString(stmt->plhandler), - "opaque", "language_handler"))); - SetFunctionReturnType(handlerOid, LANGUAGE_HANDLEROID); - } - else - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("function %s must return type %s", - NameListToString(stmt->plhandler), "language_handler"))); - } + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("function %s must return type %s", + NameListToString(stmt->plhandler), "language_handler"))); /* validate the inline function */ if (stmt->plinline) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 6e8b722..056a912 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -699,25 +699,10 @@ CreateTrigger(CreateTrigStmt *stmt, const char *queryString, } funcrettype = get_func_rettype(funcoid); if (funcrettype != TRIGGEROID) - { - /* - * We allow OPAQUE just so we can load old dump files. When we see a - * trigger function declared OPAQUE, change it to TRIGGER. - */ - if (funcrettype == OPAQUEOID) - { - ereport(WARNING, - (errmsg("changing return type of function %s from %s to %s", - NameListToString(stmt->funcname), - "opaque", "trigger"))); - SetFunctionReturnType(funcoid, TRIGGEROID); - } - else - ereport(ERROR, - (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), - errmsg("function %s must return type %s", - NameListToString(stmt->funcname), "trigger"))); - } + ereport(ERROR, + (errcode(ERRCODE_INVALID_OBJECT_DEFINITION), + errmsg("function %s must return type %s", + NameListToString(stmt->funcname), "trigger"))); /* * If the command is a user-entered CREATE CONSTRAINT TRIGGER command that diff --git a/src/backend/utils/adt/pseudotypes.c b/src/backend/utils/adt/pseudotypes.c index ad0e363..4653fc3 100644 --- a/src/backend/utils/adt/pseudotypes.c +++ b/src/backend/utils/adt/pseudotypes.c @@ -415,7 +415,6 @@ PSEUDOTYPE_DUMMY_IO_FUNCS(fdw_handler); PSEUDOTYPE_DUMMY_IO_FUNCS(index_am_handler); PSEUDOTYPE_DUMMY_IO_FUNCS(tsm_handler); PSEUDOTYPE_DUMMY_IO_FUNCS(internal); -PSEUDOTYPE_DUMMY_IO_FUNCS(opaque); PSEUDOTYPE_DUMMY_IO_FUNCS(anyelement); PSEUDOTYPE_DUMMY_IO_FUNCS(anynonarray); PSEUDOTYPE_DUMMY_IO_FUNCS(table_am_handler); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index ef15390..d3ab5bb 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -82,10 +82,9 @@ typedef struct typedef enum OidOptions { - zeroAsOpaque = 1, - zeroAsAny = 2, - zeroAsStar = 4, - zeroAsNone = 8 + zeroIsError = 1, + zeroAsStar = 2, + zeroAsNone = 4 } OidOptions; /* global decls */ @@ -122,8 +121,6 @@ static SimpleStringList tabledata_exclude_patterns = {NULL, NULL}; static SimpleOidList tabledata_exclude_oids = {NULL, NULL}; -char g_opaque_type[10]; /* name for the opaque type */ - /* placeholders for the delimiters for comments */ char g_comment_start[10]; char g_comment_end[10]; @@ -404,7 +401,6 @@ main(int argc, char **argv) strcpy(g_comment_start, "-- "); g_comment_end[0] = '\0'; - strcpy(g_opaque_type, "opaque"); progname = get_progname(argv[0]); @@ -10736,7 +10732,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo) { char *elemType; - elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroAsOpaque); + elemType = getFormattedTypeName(fout, tyinfo->typelem, zeroIsError); appendPQExpBuffer(q, ",\n ELEMENT = %s", elemType); free(elemType); } @@ -11547,7 +11543,7 @@ format_function_arguments_old(Archive *fout, const char *argname; typid = allargtypes ? atooid(allargtypes[j]) : finfo->argtypes[j]; - typname = getFormattedTypeName(fout, typid, zeroAsOpaque); + typname = getFormattedTypeName(fout, typid, zeroIsError); if (argmodes) { @@ -11616,7 +11612,7 @@ format_function_signature(Archive *fout, FuncInfo *finfo, bool honor_quotes) appendPQExpBufferStr(&fn, ", "); typname = getFormattedTypeName(fout, finfo->argtypes[j], - zeroAsOpaque); + zeroIsError); appendPQExpBufferStr(&fn, typname); free(typname); } @@ -12021,7 +12017,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo) else { rettypename = getFormattedTypeName(fout, finfo->prorettype, - zeroAsOpaque); + zeroIsError); appendPQExpBuffer(q, " RETURNS %s%s", (proretset[0] == 't') ? "SETOF " : "", rettypename); @@ -13740,7 +13736,7 @@ format_aggregate_signature(AggInfo *agginfo, Archive *fout, bool honor_quotes) char *typname; typname = getFormattedTypeName(fout, agginfo->aggfn.argtypes[j], - zeroAsOpaque); + zeroIsError); appendPQExpBuffer(&buf, "%s%s", (j > 0) ? ", " : "", @@ -18363,11 +18359,7 @@ getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts) if (oid == 0) { - if ((opts & zeroAsOpaque) != 0) - return pg_strdup(g_opaque_type); - else if ((opts & zeroAsAny) != 0) - return pg_strdup("'any'"); - else if ((opts & zeroAsStar) != 0) + if ((opts & zeroAsStar) != 0) return pg_strdup("*"); else if ((opts & zeroAsNone) != 0) return pg_strdup("NONE"); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 21004e5..852020b 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -640,8 +640,6 @@ typedef struct _extensionMemberId extern char g_comment_start[10]; extern char g_comment_end[10]; -extern char g_opaque_type[10]; /* name for the opaque type */ - /* * common utility functions */ diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 07a86c7..7fb574f 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -7054,12 +7054,6 @@ { oid => '2305', descr => 'I/O', proname => 'internal_out', prorettype => 'cstring', proargtypes => 'internal', prosrc => 'internal_out' }, -{ oid => '2306', descr => 'I/O', - proname => 'opaque_in', proisstrict => 'f', prorettype => 'opaque', - proargtypes => 'cstring', prosrc => 'opaque_in' }, -{ oid => '2307', descr => 'I/O', - proname => 'opaque_out', prorettype => 'cstring', proargtypes => 'opaque', - prosrc => 'opaque_out' }, { oid => '2312', descr => 'I/O', proname => 'anyelement_in', prorettype => 'anyelement', proargtypes => 'cstring', prosrc => 'anyelement_in' }, @@ -7067,10 +7061,10 @@ proname => 'anyelement_out', prorettype => 'cstring', proargtypes => 'anyelement', prosrc => 'anyelement_out' }, { oid => '2398', descr => 'I/O', - proname => 'shell_in', proisstrict => 'f', prorettype => 'opaque', + proname => 'shell_in', proisstrict => 'f', prorettype => 'void', proargtypes => 'cstring', prosrc => 'shell_in' }, { oid => '2399', descr => 'I/O', - proname => 'shell_out', prorettype => 'cstring', proargtypes => 'opaque', + proname => 'shell_out', prorettype => 'cstring', proargtypes => 'void', prosrc => 'shell_out' }, { oid => '2597', descr => 'I/O', proname => 'domain_in', proisstrict => 'f', provolatile => 's', diff --git a/src/include/catalog/pg_type.dat b/src/include/catalog/pg_type.dat index 4cf2b9d..b00597d 100644 --- a/src/include/catalog/pg_type.dat +++ b/src/include/catalog/pg_type.dat @@ -546,10 +546,6 @@ typtype => 'p', typcategory => 'P', typinput => 'internal_in', typoutput => 'internal_out', typreceive => '-', typsend => '-', typalign => 'ALIGNOF_POINTER' }, -{ oid => '2282', descr => 'obsolete, deprecated pseudo-type', - typname => 'opaque', typlen => '4', typbyval => 't', typtype => 'p', - typcategory => 'P', typinput => 'opaque_in', typoutput => 'opaque_out', - typreceive => '-', typsend => '-', typalign => 'i' }, { oid => '2283', descr => 'pseudo-type representing a polymorphic base type', typname => 'anyelement', typlen => '4', typbyval => 't', typtype => 'p', typcategory => 'P', typinput => 'anyelement_in', diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index 0992c23..5cd6975 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -54,7 +54,6 @@ extern Oid ResolveOpClass(List *opclass, Oid attrType, /* commands/functioncmds.c */ extern ObjectAddress CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt); extern void RemoveFunctionById(Oid funcOid); -extern void SetFunctionReturnType(Oid funcOid, Oid newRetType); extern ObjectAddress AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt); extern ObjectAddress CreateCast(CreateCastStmt *stmt); extern void DropCastById(Oid castOid); diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c index a65bce0..5fdf303 100644 --- a/src/pl/plperl/plperl.c +++ b/src/pl/plperl/plperl.c @@ -2000,9 +2000,7 @@ plperl_validator(PG_FUNCTION_ARGS) /* except for TRIGGER, EVTTRIGGER, RECORD, or VOID */ if (functyptype == TYPTYPE_PSEUDO) { - /* we assume OPAQUE with no arguments means a trigger */ - if (proc->prorettype == TRIGGEROID || - (proc->prorettype == OPAQUEOID && proc->pronargs == 0)) + if (proc->prorettype == TRIGGEROID) is_trigger = true; else if (proc->prorettype == EVTTRIGGEROID) is_event_trigger = true; diff --git a/src/pl/plpgsql/src/pl_handler.c b/src/pl/plpgsql/src/pl_handler.c index b83087e..b434818 100644 --- a/src/pl/plpgsql/src/pl_handler.c +++ b/src/pl/plpgsql/src/pl_handler.c @@ -421,12 +421,10 @@ plpgsql_validator(PG_FUNCTION_ARGS) functyptype = get_typtype(proc->prorettype); /* Disallow pseudotype result */ - /* except for TRIGGER, RECORD, VOID, or polymorphic */ + /* except for TRIGGER, EVTTRIGGER, RECORD, VOID, or polymorphic */ if (functyptype == TYPTYPE_PSEUDO) { - /* we assume OPAQUE with no arguments means a trigger */ - if (proc->prorettype == TRIGGEROID || - (proc->prorettype == OPAQUEOID && proc->pronargs == 0)) + if (proc->prorettype == TRIGGEROID) is_dml_trigger = true; else if (proc->prorettype == EVTTRIGGEROID) is_event_trigger = true; diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c index 882d69e..3eedaa8 100644 --- a/src/pl/plpython/plpy_main.c +++ b/src/pl/plpython/plpy_main.c @@ -379,9 +379,7 @@ plpython2_inline_handler(PG_FUNCTION_ARGS) static bool PLy_procedure_is_trigger(Form_pg_proc procStruct) { - return (procStruct->prorettype == TRIGGEROID || - (procStruct->prorettype == OPAQUEOID && - procStruct->pronargs == 0)); + return (procStruct->prorettype == TRIGGEROID); } static void diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out index fb6c029..40468e8 100644 --- a/src/test/regress/expected/opr_sanity.out +++ b/src/test/regress/expected/opr_sanity.out @@ -384,7 +384,7 @@ FROM pg_proc as p1 WHERE p1.prorettype = 'cstring'::regtype AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typoutput = p1.oid) AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typmodout = p1.oid) - AND p1.oid != 'shell_out(opaque)'::regprocedure + AND p1.oid != 'shell_out(void)'::regprocedure ORDER BY 1; oid | proname ------+-------------- diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql index 8351b64..f06f245 100644 --- a/src/test/regress/sql/opr_sanity.sql +++ b/src/test/regress/sql/opr_sanity.sql @@ -309,7 +309,7 @@ FROM pg_proc as p1 WHERE p1.prorettype = 'cstring'::regtype AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typoutput = p1.oid) AND NOT EXISTS(SELECT 1 FROM pg_type WHERE typmodout = p1.oid) - AND p1.oid != 'shell_out(opaque)'::regprocedure + AND p1.oid != 'shell_out(void)'::regprocedure ORDER BY 1; -- Check for length inconsistencies between the various argument-info arrays.
pgsql-hackers by date: