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:

Previous
From: David Steele
Date:
Subject: Cast to uint16 in pg_checksum_page()
Next
From: Tom Lane
Date:
Subject: Re: Back-patching -Wno-format-truncation.