Skip to content

Commit

Permalink
Record dependencies of a cast on other casts that it requires.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tglsfdc committed Oct 17, 2022
1 parent 797e313 commit 8272749
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 9 deletions.
25 changes: 22 additions & 3 deletions src/backend/catalog/pg_cast.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
14 changes: 10 additions & 4 deletions src/backend/commands/functioncmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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")));
Expand All @@ -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")));
Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 3 additions & 1 deletion src/backend/commands/typecmds.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
21 changes: 21 additions & 0 deletions src/backend/parser/parse_coerce.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions src/include/catalog/pg_cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/include/parser/parse_coerce.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
29 changes: 29 additions & 0 deletions src/test/regress/expected/create_cast.out
Original file line number Diff line number Diff line change
Expand Up @@ -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)

21 changes: 21 additions & 0 deletions src/test/regress/sql/create_cast.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
2 changes: 1 addition & 1 deletion src/tools/valgrind.supp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 8272749

Please sign in to comment.