Re: PG upgrade 14->15 fails - database contains our own extension - Mailing list pgsql-hackers

From Tom Lane
Subject Re: PG upgrade 14->15 fails - database contains our own extension
Date
Msg-id 1061568.1665687090@sss.pgh.pa.us
Whole thread Raw
In response to Re: PG upgrade 14->15 fails - database contains our own extension  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
I wrote:
> We might be able to put in some kluge in pg_dump to make it less
> likely to fail with existing DBs, but I think the true fix lies
> in adding that dependency.

Here's a draft patch for that.  I'm unsure whether it's worth
back-patching; even if we did, we couldn't guarantee that existing
databases would have the additional pg_depend entries.

If we do only put it in HEAD, maybe we should break compatibility
to the extent of changing IsBinaryCoercible's API rather than
inventing a separate call.  I'm still not excited about recording
additional dependencies elsewhere, but that path would leave us
with cleaner code if we eventually do that.

            regards, tom lane

diff --git a/src/backend/catalog/pg_cast.c b/src/backend/catalog/pg_cast.c
index 1812bb7fcc..b50a56954d 100644
--- a/src/backend/catalog/pg_cast.c
+++ b/src/backend/catalog/pg_cast.c
@@ -35,13 +35,20 @@
  * Caller must have already checked privileges, and done consistency
  * checks on the given datatypes and cast function (if applicable).
  *
+ * Since we allow binary coercibility of the datatypes to the cast
+ * function's input and result, there could be one or two WITHOUT FUNCTION
+ * casts that this one depends on.  We don't record that explicitly
+ * in pg_cast, but we still need to make dependencies on those casts.
+ *
  * 'behavior' indicates the types of the dependencies that the new
- * cast will have on its input and output types and the cast function.
+ * cast will have on its input and output types, the cast function,
+ * and the other casts if any.
  * ----------------------------------------------------------------
  */
 ObjectAddress
-CastCreate(Oid sourcetypeid, Oid targettypeid, Oid funcid, char castcontext,
-           char castmethod, DependencyType behavior)
+CastCreate(Oid sourcetypeid, Oid targettypeid,
+           Oid funcid, Oid incastid, Oid outcastid,
+           char castcontext, char castmethod, DependencyType behavior)
 {
     Relation    relation;
     HeapTuple    tuple;
@@ -102,6 +109,18 @@ CastCreate(Oid sourcetypeid, Oid targettypeid, Oid funcid, char castcontext,
         add_exact_object_address(&referenced, addrs);
     }

+    /* dependencies on casts required for function */
+    if (OidIsValid(incastid))
+    {
+        ObjectAddressSet(referenced, CastRelationId, incastid);
+        add_exact_object_address(&referenced, addrs);
+    }
+    if (OidIsValid(outcastid))
+    {
+        ObjectAddressSet(referenced, CastRelationId, outcastid);
+        add_exact_object_address(&referenced, addrs);
+    }
+
     record_object_address_dependencies(&myself, addrs, behavior);
     free_object_addresses(addrs);

diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index e6fcfc23b9..1f820c93e9 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -1526,6 +1526,8 @@ CreateCast(CreateCastStmt *stmt)
     char        sourcetyptype;
     char        targettyptype;
     Oid            funcid;
+    Oid            incastid = InvalidOid;
+    Oid            outcastid = InvalidOid;
     int            nargs;
     char        castcontext;
     char        castmethod;
@@ -1603,7 +1605,9 @@ CreateCast(CreateCastStmt *stmt)
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                      errmsg("cast function must take one to three arguments")));
-        if (!IsBinaryCoercible(sourcetypeid, procstruct->proargtypes.values[0]))
+        if (!IsBinaryCoercibleWithCast(sourcetypeid,
+                                       procstruct->proargtypes.values[0],
+                                       &incastid))
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                      errmsg("argument of cast function must match or be binary-coercible from source data type")));
@@ -1617,7 +1621,9 @@ CreateCast(CreateCastStmt *stmt)
                     (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                      errmsg("third argument of cast function must be type %s",
                             "boolean")));
-        if (!IsBinaryCoercible(procstruct->prorettype, targettypeid))
+        if (!IsBinaryCoercibleWithCast(procstruct->prorettype,
+                                       targettypeid,
+                                       &outcastid))
             ereport(ERROR,
                     (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
                      errmsg("return data type of cast function must match or be binary-coercible to target data
type")));
@@ -1756,8 +1762,8 @@ CreateCast(CreateCastStmt *stmt)
             break;
     }

-    myself = CastCreate(sourcetypeid, targettypeid, funcid, castcontext,
-                        castmethod, DEPENDENCY_NORMAL);
+    myself = CastCreate(sourcetypeid, targettypeid, funcid, incastid, outcastid,
+                        castcontext, castmethod, DEPENDENCY_NORMAL);
     return myself;
 }

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 33b64fd279..b7c3dded17 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1705,7 +1705,9 @@ DefineRange(ParseState *pstate, CreateRangeStmt *stmt)
                                &castFuncOid);

     /* Create cast from the range type to its multirange type */
-    CastCreate(typoid, multirangeOid, castFuncOid, 'e', 'f', DEPENDENCY_INTERNAL);
+    CastCreate(typoid, multirangeOid, castFuncOid, InvalidOid, InvalidOid,
+               COERCION_CODE_EXPLICIT, COERCION_METHOD_FUNCTION,
+               DEPENDENCY_INTERNAL);

     pfree(multirangeArrayName);

diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index c4e958e4aa..60908111c8 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -2993,11 +2993,29 @@ IsPreferredType(TYPCATEGORY category, Oid type)
  */
 bool
 IsBinaryCoercible(Oid srctype, Oid targettype)
+{
+    Oid            castoid;
+
+    return IsBinaryCoercibleWithCast(srctype, targettype, &castoid);
+}
+
+/* IsBinaryCoercibleWithCast()
+ *        Check if srctype is binary-coercible to targettype.
+ *
+ * This variant also returns the OID of the pg_cast entry if one is involved.
+ * *castoid is set to InvalidOid if no binary-coercible cast exists, or if
+ * there is a hard-wired rule for it rather than a pg_cast entry.
+ */
+bool
+IsBinaryCoercibleWithCast(Oid srctype, Oid targettype,
+                          Oid *castoid)
 {
     HeapTuple    tuple;
     Form_pg_cast castForm;
     bool        result;

+    *castoid = InvalidOid;
+
     /* Fast path if same type */
     if (srctype == targettype)
         return true;
@@ -3061,6 +3079,9 @@ IsBinaryCoercible(Oid srctype, Oid targettype)
     result = (castForm->castmethod == COERCION_METHOD_BINARY &&
               castForm->castcontext == COERCION_CODE_IMPLICIT);

+    if (result)
+        *castoid = castForm->oid;
+
     ReleaseSysCache(tuple);

     return result;
diff --git a/src/include/catalog/pg_cast.h b/src/include/catalog/pg_cast.h
index 3c15df0053..67b08a4663 100644
--- a/src/include/catalog/pg_cast.h
+++ b/src/include/catalog/pg_cast.h
@@ -95,6 +95,8 @@ typedef enum CoercionMethod
 extern ObjectAddress CastCreate(Oid sourcetypeid,
                                 Oid targettypeid,
                                 Oid funcid,
+                                Oid incastid,
+                                Oid outcastid,
                                 char castcontext,
                                 char castmethod,
                                 DependencyType behavior);
diff --git a/src/include/parser/parse_coerce.h b/src/include/parser/parse_coerce.h
index b105c7da90..ddbc995077 100644
--- a/src/include/parser/parse_coerce.h
+++ b/src/include/parser/parse_coerce.h
@@ -32,6 +32,8 @@ typedef enum CoercionPathType


 extern bool IsBinaryCoercible(Oid srctype, Oid targettype);
+extern bool IsBinaryCoercibleWithCast(Oid srctype, Oid targettype,
+                                      Oid *castoid);
 extern bool IsPreferredType(TYPCATEGORY category, Oid type);
 extern TYPCATEGORY TypeCategory(Oid type);

diff --git a/src/test/regress/expected/create_cast.out b/src/test/regress/expected/create_cast.out
index 88f94a63b4..9a56fe3f0f 100644
--- a/src/test/regress/expected/create_cast.out
+++ b/src/test/regress/expected/create_cast.out
@@ -72,3 +72,32 @@ SELECT 1234::int4::casttesttype; -- Should work now
  foo1234
 (1 row)

+DROP FUNCTION int4_casttesttype(int4) CASCADE;
+NOTICE:  drop cascades to cast from integer to casttesttype
+-- Try it with a function that requires an implicit cast
+CREATE FUNCTION bar_int4_text(int4) RETURNS text LANGUAGE SQL AS
+$$ SELECT ('bar'::text || $1::text); $$;
+CREATE CAST (int4 AS casttesttype) WITH FUNCTION bar_int4_text(int4) AS IMPLICIT;
+SELECT 1234::int4::casttesttype; -- Should work now
+ casttesttype
+--------------
+ bar1234
+(1 row)
+
+-- check dependencies generated for that
+SELECT pg_describe_object(classid, objid, objsubid) as obj,
+       pg_describe_object(refclassid, refobjid, refobjsubid) as objref,
+       deptype
+FROM pg_depend
+WHERE classid = 'pg_cast'::regclass AND
+      objid = (SELECT oid FROM pg_cast
+               WHERE castsource = 'int4'::regtype
+                 AND casttarget = 'casttesttype'::regtype)
+ORDER BY refclassid;
+                obj                |             objref              | deptype
+-----------------------------------+---------------------------------+---------
+ cast from integer to casttesttype | type casttesttype               | n
+ cast from integer to casttesttype | function bar_int4_text(integer) | n
+ cast from integer to casttesttype | cast from text to casttesttype  | n
+(3 rows)
+
diff --git a/src/test/regress/sql/create_cast.sql b/src/test/regress/sql/create_cast.sql
index b11cf88b06..32187853cc 100644
--- a/src/test/regress/sql/create_cast.sql
+++ b/src/test/regress/sql/create_cast.sql
@@ -52,3 +52,24 @@ $$ SELECT ('foo'::text || $1::text)::casttesttype; $$;

 CREATE CAST (int4 AS casttesttype) WITH FUNCTION int4_casttesttype(int4) AS IMPLICIT;
 SELECT 1234::int4::casttesttype; -- Should work now
+
+DROP FUNCTION int4_casttesttype(int4) CASCADE;
+
+-- Try it with a function that requires an implicit cast
+
+CREATE FUNCTION bar_int4_text(int4) RETURNS text LANGUAGE SQL AS
+$$ SELECT ('bar'::text || $1::text); $$;
+
+CREATE CAST (int4 AS casttesttype) WITH FUNCTION bar_int4_text(int4) AS IMPLICIT;
+SELECT 1234::int4::casttesttype; -- Should work now
+
+-- check dependencies generated for that
+SELECT pg_describe_object(classid, objid, objsubid) as obj,
+       pg_describe_object(refclassid, refobjid, refobjsubid) as objref,
+       deptype
+FROM pg_depend
+WHERE classid = 'pg_cast'::regclass AND
+      objid = (SELECT oid FROM pg_cast
+               WHERE castsource = 'int4'::regtype
+                 AND casttarget = 'casttesttype'::regtype)
+ORDER BY refclassid;
diff --git a/src/tools/valgrind.supp b/src/tools/valgrind.supp
index 4e8c482757..7ea464c809 100644
--- a/src/tools/valgrind.supp
+++ b/src/tools/valgrind.supp
@@ -113,7 +113,7 @@
     overread_tuplestruct_pg_cast
     Memcheck:Addr4

-    fun:IsBinaryCoercible
+    fun:IsBinaryCoercibleWithCast
 }

 # Python's allocator does some low-level tricks for efficiency. Those

pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: proposal: possibility to read dumped table's name from file
Next
From: Tom Lane
Date:
Subject: Re: archive modules