From 8272749e8ca1dbbcb5f8cf5632ec26a573ac3111 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 17 Oct 2022 14:02:05 -0400 Subject: [PATCH] Record dependencies of a cast on other casts that it requires. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When creating a cast that uses a conversion function, we've historically allowed the input and result types to be binary-compatible with the function's input and result types, rather than necessarily being identical. This means that the new cast is logically dependent on the binary-compatible cast or casts that it references: if those are defined by pg_cast entries, and you try to restore the new cast without having defined them, it'll fail. Hence, we should make pg_depend entries to record these dependencies so that pg_dump knows that there is an ordering requirement. This is not the only place where we allow such shortcuts; aggregate functions for example are similarly lax, and in principle should gain similar dependencies. However, for now it seems sufficient to fix the cast-versus-cast case, as pg_dump's other ordering heuristics should keep it out of trouble for other object types. Per report from David TuroĊˆ; thanks also to Robert Haas for preliminary investigation. I considered back-patching, but seeing that this issue has existed for many years without previous reports, it's not clear it's worth the trouble. Moreover, back-patching wouldn't be enough to ensure that the new pg_depend entries exist in existing databases anyway. Discussion: https://postgr.es/m/OF0A160F3E.578B15D1-ONC12588DA.003E4857-C12588DA.0045A428@notes.linuxbox.cz --- src/backend/catalog/pg_cast.c | 25 ++++++++++++++++--- src/backend/commands/functioncmds.c | 14 +++++++---- src/backend/commands/typecmds.c | 4 +++- src/backend/parser/parse_coerce.c | 21 ++++++++++++++++ src/include/catalog/pg_cast.h | 2 ++ src/include/parser/parse_coerce.h | 2 ++ src/test/regress/expected/create_cast.out | 29 +++++++++++++++++++++++ src/test/regress/sql/create_cast.sql | 21 ++++++++++++++++ src/tools/valgrind.supp | 2 +- 9 files changed, 111 insertions(+), 9 deletions(-) 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