From e974f223abb473ba62577261378ca740c641814f Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Fri, 18 Jan 2019 14:40:13 -0300 Subject: [PATCH] Refactor duplicate code into DeconstructFkConstraintRow My commit 3de241dba86f introduced some code (in tablecmds.c) to obtain data from a pg_constraint row for a foreign key, that already existed in ri_triggers.c. Split it out into its own routine in pg_constraint.c, where it naturally belongs. No functional code changes, only code movement. Backpatch to pg11, because a future bugfix is simpler after this. --- src/backend/catalog/pg_constraint.c | 199 +++++++++++++++------------- src/backend/utils/adt/ri_triggers.c | 89 +------------ src/backend/utils/cache/relcache.c | 61 +-------- src/include/catalog/pg_constraint.h | 3 + 4 files changed, 124 insertions(+), 228 deletions(-) diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index f4057a9f15..6235729f69 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -450,14 +450,11 @@ static void clone_fk_constraints(Relation pg_constraint, Relation parentRel, Relation partRel, List *clone, List **cloned) { - TupleDesc tupdesc; AttrNumber *attmap; List *partFKs; List *subclone = NIL; ListCell *cell; - tupdesc = RelationGetDescr(pg_constraint); - /* * The constraint key may differ, if the columns in the partition are * different. This map is used to convert them. @@ -487,9 +484,6 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel, int nelem; ListCell *cell; int i; - ArrayType *arr; - Datum datum; - bool isnull; tuple = SearchSysCache1(CONSTROID, parentConstrOid); if (!tuple) @@ -506,93 +500,11 @@ clone_fk_constraints(Relation pg_constraint, Relation parentRel, ObjectAddressSet(parentAddr, ConstraintRelationId, parentConstrOid); - datum = fastgetattr(tuple, Anum_pg_constraint_conkey, - tupdesc, &isnull); - if (isnull) - elog(ERROR, "null conkey"); - arr = DatumGetArrayTypeP(datum); - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem < 1 || - nelem > INDEX_MAX_KEYS || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != INT2OID) - elog(ERROR, "conkey is not a 1-D smallint array"); - memcpy(conkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber)); - + DeconstructFkConstraintRow(tuple, &nelem, conkey, confkey, + conpfeqop, conppeqop, conffeqop); for (i = 0; i < nelem; i++) mapped_conkey[i] = attmap[conkey[i] - 1]; - datum = fastgetattr(tuple, Anum_pg_constraint_confkey, - tupdesc, &isnull); - if (isnull) - elog(ERROR, "null confkey"); - arr = DatumGetArrayTypeP(datum); - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem < 1 || - nelem > INDEX_MAX_KEYS || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != INT2OID) - elog(ERROR, "confkey is not a 1-D smallint array"); - memcpy(confkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber)); - - datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop, - tupdesc, &isnull); - if (isnull) - elog(ERROR, "null conpfeqop"); - arr = DatumGetArrayTypeP(datum); - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem < 1 || - nelem > INDEX_MAX_KEYS || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conpfeqop is not a 1-D OID array"); - memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); - - datum = fastgetattr(tuple, Anum_pg_constraint_conpfeqop, - tupdesc, &isnull); - if (isnull) - elog(ERROR, "null conpfeqop"); - arr = DatumGetArrayTypeP(datum); - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem < 1 || - nelem > INDEX_MAX_KEYS || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conpfeqop is not a 1-D OID array"); - memcpy(conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); - - datum = fastgetattr(tuple, Anum_pg_constraint_conppeqop, - tupdesc, &isnull); - if (isnull) - elog(ERROR, "null conppeqop"); - arr = DatumGetArrayTypeP(datum); - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem < 1 || - nelem > INDEX_MAX_KEYS || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conppeqop is not a 1-D OID array"); - memcpy(conppeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); - - datum = fastgetattr(tuple, Anum_pg_constraint_conffeqop, - tupdesc, &isnull); - if (isnull) - elog(ERROR, "null conffeqop"); - arr = DatumGetArrayTypeP(datum); - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem < 1 || - nelem > INDEX_MAX_KEYS || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conffeqop is not a 1-D OID array"); - memcpy(conffeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); - /* * Before creating a new constraint, see whether any existing FKs are * fit for the purpose. If one is, attach the parent constraint to it, @@ -1536,6 +1448,113 @@ get_primary_key_attnos(Oid relid, bool deferrableOk, Oid *constraintOid) return pkattnos; } +/* + * Extract data from the pg_constraint tuple of a foreign-key constraint. + * + * All arguments save the first are output arguments; the last three of them + * can be passed as NULL if caller doesn't need them. + */ +void +DeconstructFkConstraintRow(HeapTuple tuple, int *numfks, + AttrNumber *conkey, AttrNumber *confkey, + Oid *pf_eq_oprs, Oid *pp_eq_oprs, Oid *ff_eq_oprs) +{ + Oid constrId = HeapTupleGetOid(tuple); + Datum adatum; + bool isNull; + ArrayType *arr; + int numkeys; + + /* + * We expect the arrays to be 1-D arrays of the right types; verify that. + * We don't need to use deconstruct_array() since the array data is just + * going to look like a C array of values. + */ + adatum = SysCacheGetAttr(CONSTROID, tuple, + Anum_pg_constraint_conkey, &isNull); + if (isNull) + elog(ERROR, "null conkey for constraint %u", constrId); + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + if (ARR_NDIM(arr) != 1 || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != INT2OID) + elog(ERROR, "conkey is not a 1-D smallint array"); + numkeys = ARR_DIMS(arr)[0]; + if (numkeys <= 0 || numkeys > INDEX_MAX_KEYS) + elog(ERROR, "foreign key constraint cannot have %d columns", numkeys); + memcpy(conkey, ARR_DATA_PTR(arr), numkeys * sizeof(int16)); + if ((Pointer) arr != DatumGetPointer(adatum)) + pfree(arr); /* free de-toasted copy, if any */ + + adatum = SysCacheGetAttr(CONSTROID, tuple, + Anum_pg_constraint_confkey, &isNull); + if (isNull) + elog(ERROR, "null confkey for constraint %u", constrId); + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + if (ARR_NDIM(arr) != 1 || + ARR_DIMS(arr)[0] != numkeys || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != INT2OID) + elog(ERROR, "confkey is not a 1-D smallint array"); + memcpy(confkey, ARR_DATA_PTR(arr), numkeys * sizeof(int16)); + if ((Pointer) arr != DatumGetPointer(adatum)) + pfree(arr); /* free de-toasted copy, if any */ + + if (pf_eq_oprs) + { + adatum = SysCacheGetAttr(CONSTROID, tuple, + Anum_pg_constraint_conpfeqop, &isNull); + if (isNull) + elog(ERROR, "null conpfeqop for constraint %u", constrId); + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + /* see TryReuseForeignKey if you change the test below */ + if (ARR_NDIM(arr) != 1 || + ARR_DIMS(arr)[0] != numkeys || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != OIDOID) + elog(ERROR, "conpfeqop is not a 1-D Oid array"); + memcpy(pf_eq_oprs, ARR_DATA_PTR(arr), numkeys * sizeof(Oid)); + if ((Pointer) arr != DatumGetPointer(adatum)) + pfree(arr); /* free de-toasted copy, if any */ + } + + if (pp_eq_oprs) + { + adatum = SysCacheGetAttr(CONSTROID, tuple, + Anum_pg_constraint_conppeqop, &isNull); + if (isNull) + elog(ERROR, "null conppeqop for constraint %u", constrId); + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + if (ARR_NDIM(arr) != 1 || + ARR_DIMS(arr)[0] != numkeys || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != OIDOID) + elog(ERROR, "conppeqop is not a 1-D Oid array"); + memcpy(pp_eq_oprs, ARR_DATA_PTR(arr), numkeys * sizeof(Oid)); + if ((Pointer) arr != DatumGetPointer(adatum)) + pfree(arr); /* free de-toasted copy, if any */ + } + + if (ff_eq_oprs) + { + adatum = SysCacheGetAttr(CONSTROID, tuple, + Anum_pg_constraint_conffeqop, &isNull); + if (isNull) + elog(ERROR, "null conffeqop for constraint %u", constrId); + arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ + if (ARR_NDIM(arr) != 1 || + ARR_DIMS(arr)[0] != numkeys || + ARR_HASNULL(arr) || + ARR_ELEMTYPE(arr) != OIDOID) + elog(ERROR, "conffeqop is not a 1-D Oid array"); + memcpy(ff_eq_oprs, ARR_DATA_PTR(arr), numkeys * sizeof(Oid)); + if ((Pointer) arr != DatumGetPointer(adatum)) + pfree(arr); /* free de-toasted copy, if any */ + } + + *numfks = numkeys; +} + /* * Determine whether a relation can be proven functionally dependent on * a set of grouping columns. If so, return true and add the pg_constraint diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index fc034ce601..cfc25c55df 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -2334,10 +2334,6 @@ ri_LoadConstraintInfo(Oid constraintOid) bool found; HeapTuple tup; Form_pg_constraint conForm; - Datum adatum; - bool isNull; - ArrayType *arr; - int numkeys; /* * On the first call initialize the hashtable @@ -2379,84 +2375,13 @@ ri_LoadConstraintInfo(Oid constraintOid) riinfo->confdeltype = conForm->confdeltype; riinfo->confmatchtype = conForm->confmatchtype; - /* - * We expect the arrays to be 1-D arrays of the right types; verify that. - * We don't need to use deconstruct_array() since the array data is just - * going to look like a C array of values. - */ - adatum = SysCacheGetAttr(CONSTROID, tup, - Anum_pg_constraint_conkey, &isNull); - if (isNull) - elog(ERROR, "null conkey for constraint %u", constraintOid); - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - if (ARR_NDIM(arr) != 1 || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != INT2OID) - elog(ERROR, "conkey is not a 1-D smallint array"); - numkeys = ARR_DIMS(arr)[0]; - if (numkeys <= 0 || numkeys > RI_MAX_NUMKEYS) - elog(ERROR, "foreign key constraint cannot have %d columns", numkeys); - riinfo->nkeys = numkeys; - memcpy(riinfo->fk_attnums, ARR_DATA_PTR(arr), numkeys * sizeof(int16)); - if ((Pointer) arr != DatumGetPointer(adatum)) - pfree(arr); /* free de-toasted copy, if any */ - - adatum = SysCacheGetAttr(CONSTROID, tup, - Anum_pg_constraint_confkey, &isNull); - if (isNull) - elog(ERROR, "null confkey for constraint %u", constraintOid); - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - if (ARR_NDIM(arr) != 1 || - ARR_DIMS(arr)[0] != numkeys || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != INT2OID) - elog(ERROR, "confkey is not a 1-D smallint array"); - memcpy(riinfo->pk_attnums, ARR_DATA_PTR(arr), numkeys * sizeof(int16)); - if ((Pointer) arr != DatumGetPointer(adatum)) - pfree(arr); /* free de-toasted copy, if any */ - - adatum = SysCacheGetAttr(CONSTROID, tup, - Anum_pg_constraint_conpfeqop, &isNull); - if (isNull) - elog(ERROR, "null conpfeqop for constraint %u", constraintOid); - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - /* see TryReuseForeignKey if you change the test below */ - if (ARR_NDIM(arr) != 1 || - ARR_DIMS(arr)[0] != numkeys || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conpfeqop is not a 1-D Oid array"); - memcpy(riinfo->pf_eq_oprs, ARR_DATA_PTR(arr), numkeys * sizeof(Oid)); - if ((Pointer) arr != DatumGetPointer(adatum)) - pfree(arr); /* free de-toasted copy, if any */ - - adatum = SysCacheGetAttr(CONSTROID, tup, - Anum_pg_constraint_conppeqop, &isNull); - if (isNull) - elog(ERROR, "null conppeqop for constraint %u", constraintOid); - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - if (ARR_NDIM(arr) != 1 || - ARR_DIMS(arr)[0] != numkeys || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conppeqop is not a 1-D Oid array"); - memcpy(riinfo->pp_eq_oprs, ARR_DATA_PTR(arr), numkeys * sizeof(Oid)); - if ((Pointer) arr != DatumGetPointer(adatum)) - pfree(arr); /* free de-toasted copy, if any */ - - adatum = SysCacheGetAttr(CONSTROID, tup, - Anum_pg_constraint_conffeqop, &isNull); - if (isNull) - elog(ERROR, "null conffeqop for constraint %u", constraintOid); - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - if (ARR_NDIM(arr) != 1 || - ARR_DIMS(arr)[0] != numkeys || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conffeqop is not a 1-D Oid array"); - memcpy(riinfo->ff_eq_oprs, ARR_DATA_PTR(arr), numkeys * sizeof(Oid)); - if ((Pointer) arr != DatumGetPointer(adatum)) - pfree(arr); /* free de-toasted copy, if any */ + DeconstructFkConstraintRow(tup, + &riinfo->nkeys, + riinfo->fk_attnums, + riinfo->pk_attnums, + riinfo->pf_eq_oprs, + riinfo->pp_eq_oprs, + riinfo->ff_eq_oprs); ReleaseSysCache(tup); diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index aecbd4a943..e21ec620db 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -4135,10 +4135,6 @@ RelationGetFKeyList(Relation relation) { Form_pg_constraint constraint = (Form_pg_constraint) GETSTRUCT(htup); ForeignKeyCacheInfo *info; - Datum adatum; - bool isnull; - ArrayType *arr; - int nelem; /* consider only foreign keys */ if (constraint->contype != CONSTRAINT_FOREIGN) @@ -4149,58 +4145,11 @@ RelationGetFKeyList(Relation relation) info->conrelid = constraint->conrelid; info->confrelid = constraint->confrelid; - /* Extract data from conkey field */ - adatum = fastgetattr(htup, Anum_pg_constraint_conkey, - conrel->rd_att, &isnull); - if (isnull) - elog(ERROR, "null conkey for rel %s", - RelationGetRelationName(relation)); - - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem < 1 || - nelem > INDEX_MAX_KEYS || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != INT2OID) - elog(ERROR, "conkey is not a 1-D smallint array"); - - info->nkeys = nelem; - memcpy(info->conkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber)); - - /* Likewise for confkey */ - adatum = fastgetattr(htup, Anum_pg_constraint_confkey, - conrel->rd_att, &isnull); - if (isnull) - elog(ERROR, "null confkey for rel %s", - RelationGetRelationName(relation)); - - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem != info->nkeys || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != INT2OID) - elog(ERROR, "confkey is not a 1-D smallint array"); - - memcpy(info->confkey, ARR_DATA_PTR(arr), nelem * sizeof(AttrNumber)); - - /* Likewise for conpfeqop */ - adatum = fastgetattr(htup, Anum_pg_constraint_conpfeqop, - conrel->rd_att, &isnull); - if (isnull) - elog(ERROR, "null conpfeqop for rel %s", - RelationGetRelationName(relation)); - - arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */ - nelem = ARR_DIMS(arr)[0]; - if (ARR_NDIM(arr) != 1 || - nelem != info->nkeys || - ARR_HASNULL(arr) || - ARR_ELEMTYPE(arr) != OIDOID) - elog(ERROR, "conpfeqop is not a 1-D OID array"); - - memcpy(info->conpfeqop, ARR_DATA_PTR(arr), nelem * sizeof(Oid)); + DeconstructFkConstraintRow(htup, &info->nkeys, + info->conkey, + info->confkey, + info->conpfeqop, + NULL, NULL); /* Add FK's node to the result list */ result = lappend(result, info); diff --git a/src/include/catalog/pg_constraint.h b/src/include/catalog/pg_constraint.h index 66b3f13f74..028e11a795 100644 --- a/src/include/catalog/pg_constraint.h +++ b/src/include/catalog/pg_constraint.h @@ -255,6 +255,9 @@ extern Oid get_relation_idx_constraint_oid(Oid relationId, Oid indexId); extern Bitmapset *get_primary_key_attnos(Oid relid, bool deferrableOk, Oid *constraintOid); +extern void DeconstructFkConstraintRow(HeapTuple tuple, int *numfks, + AttrNumber *conkey, AttrNumber *confkey, + Oid *pf_eq_oprs, Oid *pp_eq_oprs, Oid *ff_eq_oprs); extern bool check_functional_grouping(Oid relid, Index varno, Index varlevelsup, -- 2.40.0