Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit d435f15

Browse files
danielgustafssondanielgustafsson
committed
Add SysCacheGetAttrNotNull for guaranteed not-null attrs
When extracting an attr from a cached tuple in the syscache with SysCacheGetAttr the isnull parameter must be checked in case the attr cannot be NULL. For cases when this is known beforehand, a wrapper is introduced which perform the errorhandling internally on behalf of the caller, invoking an elog in case of a NULL attr. Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/AD76405E-DB45-46B6-941F-17B1EB3A9076@yesql.se
1 parent e33967b commit d435f15

38 files changed

+232
-452
lines changed

‎src/backend/access/brin/brin_inclusion.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -630,7 +630,6 @@ inclusion_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
630630
HeapTuple tuple;
631631
Oid opfamily,
632632
oprid;
633-
bool isNull;
634633

635634
opfamily = bdesc->bd_index->rd_opfamily[attno - 1];
636635
attr = TupleDescAttr(bdesc->bd_tupdesc, attno - 1);
@@ -643,10 +642,10 @@ inclusion_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
643642
elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
644643
strategynum, attr->atttypid, subtype, opfamily);
645644

646-
oprid = DatumGetObjectId(SysCacheGetAttr(AMOPSTRATEGY, tuple,
647-
Anum_pg_amop_amopopr, &isNull));
645+
oprid = DatumGetObjectId(SysCacheGetAttrNotNull(AMOPSTRATEGY, tuple,
646+
Anum_pg_amop_amopopr));
648647
ReleaseSysCache(tuple);
649-
Assert(!isNull && RegProcedureIsValid(oprid));
648+
Assert(RegProcedureIsValid(oprid));
650649

651650
fmgr_info_cxt(get_opcode(oprid),
652651
&opaque->strategy_procinfos[strategynum - 1],

‎src/backend/access/brin/brin_minmax.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,6 @@ minmax_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
290290
HeapTuple tuple;
291291
Oid opfamily,
292292
oprid;
293-
bool isNull;
294293

295294
opfamily = bdesc->bd_index->rd_opfamily[attno - 1];
296295
attr = TupleDescAttr(bdesc->bd_tupdesc, attno - 1);
@@ -303,10 +302,10 @@ minmax_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
303302
elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
304303
strategynum, attr->atttypid, subtype, opfamily);
305304

306-
oprid = DatumGetObjectId(SysCacheGetAttr(AMOPSTRATEGY, tuple,
307-
Anum_pg_amop_amopopr, &isNull));
305+
oprid = DatumGetObjectId(SysCacheGetAttrNotNull(AMOPSTRATEGY, tuple,
306+
Anum_pg_amop_amopopr));
308307
ReleaseSysCache(tuple);
309-
Assert(!isNull && RegProcedureIsValid(oprid));
308+
Assert(RegProcedureIsValid(oprid));
310309

311310
fmgr_info_cxt(get_opcode(oprid),
312311
&opaque->strategy_procinfos[strategynum - 1],

‎src/backend/access/brin/brin_minmax_multi.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2953,7 +2953,6 @@ minmax_multi_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
29532953
HeapTuple tuple;
29542954
Oid opfamily,
29552955
oprid;
2956-
bool isNull;
29572956

29582957
opfamily = bdesc->bd_index->rd_opfamily[attno - 1];
29592958
attr = TupleDescAttr(bdesc->bd_tupdesc, attno - 1);
@@ -2965,10 +2964,10 @@ minmax_multi_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
29652964
elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
29662965
strategynum, attr->atttypid, subtype, opfamily);
29672966

2968-
oprid = DatumGetObjectId(SysCacheGetAttr(AMOPSTRATEGY, tuple,
2969-
Anum_pg_amop_amopopr, &isNull));
2967+
oprid = DatumGetObjectId(SysCacheGetAttrNotNull(AMOPSTRATEGY, tuple,
2968+
Anum_pg_amop_amopopr));
29702969
ReleaseSysCache(tuple);
2971-
Assert(!isNull && RegProcedureIsValid(oprid));
2970+
Assert(RegProcedureIsValid(oprid));
29722971

29732972
fmgr_info_cxt(get_opcode(oprid),
29742973
&opaque->strategy_procinfos[strategynum - 1],

‎src/backend/access/index/indexam.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,6 @@ index_opclass_options(Relation indrel, AttrNumber attnum, Datum attoptions,
961961
Oid opclass;
962962
Datum indclassDatum;
963963
oidvector *indclass;
964-
bool isnull;
965964

966965
if (!DatumGetPointer(attoptions))
967966
return NULL; /* ok, no options, no procedure */
@@ -970,9 +969,8 @@ index_opclass_options(Relation indrel, AttrNumber attnum, Datum attoptions,
970969
* Report an error if the opclass's options-parsing procedure does not
971970
* exist but the opclass options are specified.
972971
*/
973-
indclassDatum = SysCacheGetAttr(INDEXRELID, indrel->rd_indextuple,
974-
Anum_pg_index_indclass, &isnull);
975-
Assert(!isnull);
972+
indclassDatum = SysCacheGetAttrNotNull(INDEXRELID, indrel->rd_indextuple,
973+
Anum_pg_index_indclass);
976974
indclass = (oidvector *) DatumGetPointer(indclassDatum);
977975
opclass = indclass->values[attnum - 1];
978976

‎src/backend/catalog/aclchk.c

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2178,11 +2178,9 @@ ExecGrant_common(InternalGrant *istmt, Oid classid, AclMode default_privs,
21782178
* Get owner ID and working copy of existing ACL. If there's no ACL,
21792179
* substitute the proper default.
21802180
*/
2181-
ownerId = DatumGetObjectId(SysCacheGetAttr(cacheid,
2182-
tuple,
2183-
get_object_attnum_owner(classid),
2184-
&isNull));
2185-
Assert(!isNull);
2181+
ownerId = DatumGetObjectId(SysCacheGetAttrNotNull(cacheid,
2182+
tuple,
2183+
get_object_attnum_owner(classid)));
21862184
aclDatum = SysCacheGetAttr(cacheid,
21872185
tuple,
21882186
get_object_attnum_acl(classid),
@@ -2206,10 +2204,8 @@ ExecGrant_common(InternalGrant *istmt, Oid classid, AclMode default_privs,
22062204
old_acl, ownerId,
22072205
&grantorId, &avail_goptions);
22082206

2209-
nameDatum = SysCacheGetAttr(cacheid, tuple,
2210-
get_object_attnum_name(classid),
2211-
&isNull);
2212-
Assert(!isNull);
2207+
nameDatum = SysCacheGetAttrNotNull(cacheid, tuple,
2208+
get_object_attnum_name(classid));
22132209

22142210
/*
22152211
* Restrict the privileges to what we can actually grant, and emit the
@@ -2476,10 +2472,8 @@ ExecGrant_Parameter(InternalGrant *istmt)
24762472
parameterId);
24772473

24782474
/* We'll need the GUC's name */
2479-
nameDatum = SysCacheGetAttr(PARAMETERACLOID, tuple,
2480-
Anum_pg_parameter_acl_parname,
2481-
&isNull);
2482-
Assert(!isNull);
2475+
nameDatum = SysCacheGetAttrNotNull(PARAMETERACLOID, tuple,
2476+
Anum_pg_parameter_acl_parname);
24832477
parname = TextDatumGetCString(nameDatum);
24842478

24852479
/* Treat all parameters as belonging to the bootstrap superuser. */
@@ -3113,11 +3107,9 @@ object_aclmask(Oid classid, Oid objectid, Oid roleid,
31133107
(errcode(ERRCODE_UNDEFINED_DATABASE),
31143108
errmsg("%s with OID %u does not exist", get_object_class_descr(classid), objectid)));
31153109

3116-
ownerId = DatumGetObjectId(SysCacheGetAttr(cacheid,
3117-
tuple,
3118-
get_object_attnum_owner(classid),
3119-
&isNull));
3120-
Assert(!isNull);
3110+
ownerId = DatumGetObjectId(SysCacheGetAttrNotNull(cacheid,
3111+
tuple,
3112+
get_object_attnum_owner(classid)));
31213113

31223114
aclDatum = SysCacheGetAttr(cacheid, tuple, get_object_attnum_acl(classid),
31233115
&isNull);
@@ -3994,20 +3986,16 @@ object_ownercheck(Oid classid, Oid objectid, Oid roleid)
39943986
if (cacheid != -1)
39953987
{
39963988
HeapTuple tuple;
3997-
bool isnull;
39983989

39993990
tuple = SearchSysCache1(cacheid, ObjectIdGetDatum(objectid));
40003991
if (!HeapTupleIsValid(tuple))
40013992
ereport(ERROR,
40023993
(errcode(ERRCODE_UNDEFINED_OBJECT),
40033994
errmsg("%s with OID %u does not exist", get_object_class_descr(classid), objectid)));
40043995

4005-
ownerId = DatumGetObjectId(SysCacheGetAttr(cacheid,
4006-
tuple,
4007-
get_object_attnum_owner(classid),
4008-
&isnull));
4009-
Assert(!isnull);
4010-
3996+
ownerId = DatumGetObjectId(SysCacheGetAttrNotNull(cacheid,
3997+
tuple,
3998+
get_object_attnum_owner(classid)));
40113999
ReleaseSysCache(tuple);
40124000
}
40134001
else

‎src/backend/catalog/index.c

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,14 +1320,12 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
13201320
indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(oldIndexId));
13211321
if (!HeapTupleIsValid(indexTuple))
13221322
elog(ERROR, "cache lookup failed for index %u", oldIndexId);
1323-
indclassDatum = SysCacheGetAttr(INDEXRELID, indexTuple,
1324-
Anum_pg_index_indclass, &isnull);
1325-
Assert(!isnull);
1323+
indclassDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple,
1324+
Anum_pg_index_indclass);
13261325
indclass = (oidvector *) DatumGetPointer(indclassDatum);
13271326

1328-
colOptionDatum = SysCacheGetAttr(INDEXRELID, indexTuple,
1329-
Anum_pg_index_indoption, &isnull);
1330-
Assert(!isnull);
1327+
colOptionDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple,
1328+
Anum_pg_index_indoption);
13311329
indcoloptions = (int2vector *) DatumGetPointer(colOptionDatum);
13321330

13331331
/* Fetch options of index if any */
@@ -1347,9 +1345,8 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
13471345
Datum exprDatum;
13481346
char *exprString;
13491347

1350-
exprDatum = SysCacheGetAttr(INDEXRELID, indexTuple,
1351-
Anum_pg_index_indexprs, &isnull);
1352-
Assert(!isnull);
1348+
exprDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple,
1349+
Anum_pg_index_indexprs);
13531350
exprString = TextDatumGetCString(exprDatum);
13541351
indexExprs = (List *) stringToNode(exprString);
13551352
pfree(exprString);
@@ -1359,9 +1356,8 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
13591356
Datum predDatum;
13601357
char *predString;
13611358

1362-
predDatum = SysCacheGetAttr(INDEXRELID, indexTuple,
1363-
Anum_pg_index_indpred, &isnull);
1364-
Assert(!isnull);
1359+
predDatum = SysCacheGetAttrNotNull(INDEXRELID, indexTuple,
1360+
Anum_pg_index_indpred);
13651361
predString = TextDatumGetCString(predDatum);
13661362
indexPreds = (List *) stringToNode(predString);
13671363

‎src/backend/catalog/objectaddress.c

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2603,7 +2603,6 @@ get_object_namespace(const ObjectAddress *address)
26032603
{
26042604
int cache;
26052605
HeapTuple tuple;
2606-
bool isnull;
26072606
Oid oid;
26082607
const ObjectPropertyType *property;
26092608

@@ -2621,11 +2620,9 @@ get_object_namespace(const ObjectAddress *address)
26212620
if (!HeapTupleIsValid(tuple))
26222621
elog(ERROR, "cache lookup failed for cache %d oid %u",
26232622
cache, address->objectId);
2624-
oid = DatumGetObjectId(SysCacheGetAttr(cache,
2625-
tuple,
2626-
property->attnum_namespace,
2627-
&isnull));
2628-
Assert(!isnull);
2623+
oid = DatumGetObjectId(SysCacheGetAttrNotNull(cache,
2624+
tuple,
2625+
property->attnum_namespace));
26292626
ReleaseSysCache(tuple);
26302627

26312628
return oid;
@@ -3896,7 +3893,6 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
38963893
{
38973894
HeapTuple tup;
38983895
Datum nameDatum;
3899-
bool isNull;
39003896
char *parname;
39013897

39023898
tup = SearchSysCache1(PARAMETERACLOID,
@@ -3908,10 +3904,8 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok)
39083904
object->objectId);
39093905
break;
39103906
}
3911-
nameDatum = SysCacheGetAttr(PARAMETERACLOID, tup,
3912-
Anum_pg_parameter_acl_parname,
3913-
&isNull);
3914-
Assert(!isNull);
3907+
nameDatum = SysCacheGetAttrNotNull(PARAMETERACLOID, tup,
3908+
Anum_pg_parameter_acl_parname);
39153909
parname = TextDatumGetCString(nameDatum);
39163910
appendStringInfo(&buffer, _("parameter %s"), parname);
39173911
ReleaseSysCache(tup);
@@ -5759,7 +5753,6 @@ getObjectIdentityParts(const ObjectAddress *object,
57595753
{
57605754
HeapTuple tup;
57615755
Datum nameDatum;
5762-
bool isNull;
57635756
char *parname;
57645757

57655758
tup = SearchSysCache1(PARAMETERACLOID,
@@ -5771,10 +5764,8 @@ getObjectIdentityParts(const ObjectAddress *object,
57715764
object->objectId);
57725765
break;
57735766
}
5774-
nameDatum = SysCacheGetAttr(PARAMETERACLOID, tup,
5775-
Anum_pg_parameter_acl_parname,
5776-
&isNull);
5777-
Assert(!isNull);
5767+
nameDatum = SysCacheGetAttrNotNull(PARAMETERACLOID, tup,
5768+
Anum_pg_parameter_acl_parname);
57785769
parname = TextDatumGetCString(nameDatum);
57795770
appendStringInfoString(&buffer, parname);
57805771
if (objname)

‎src/backend/catalog/pg_constraint.c

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,23 +1190,18 @@ DeconstructFkConstraintRow(HeapTuple tuple, int *numfks,
11901190
Oid *pf_eq_oprs, Oid *pp_eq_oprs, Oid *ff_eq_oprs,
11911191
int *num_fk_del_set_cols, AttrNumber *fk_del_set_cols)
11921192
{
1193-
Oid constrId;
11941193
Datum adatum;
11951194
bool isNull;
11961195
ArrayType *arr;
11971196
int numkeys;
11981197

1199-
constrId = ((Form_pg_constraint) GETSTRUCT(tuple))->oid;
1200-
12011198
/*
12021199
* We expect the arrays to be 1-D arrays of the right types; verify that.
12031200
* We don't need to use deconstruct_array() since the array data is just
12041201
* going to look like a C array of values.
12051202
*/
1206-
adatum = SysCacheGetAttr(CONSTROID, tuple,
1207-
Anum_pg_constraint_conkey, &isNull);
1208-
if (isNull)
1209-
elog(ERROR, "null conkey for constraint %u", constrId);
1203+
adatum = SysCacheGetAttrNotNull(CONSTROID, tuple,
1204+
Anum_pg_constraint_conkey);
12101205
arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */
12111206
if (ARR_NDIM(arr) != 1 ||
12121207
ARR_HASNULL(arr) ||
@@ -1219,10 +1214,8 @@ DeconstructFkConstraintRow(HeapTuple tuple, int *numfks,
12191214
if ((Pointer) arr != DatumGetPointer(adatum))
12201215
pfree(arr); /* free de-toasted copy, if any */
12211216

1222-
adatum = SysCacheGetAttr(CONSTROID, tuple,
1223-
Anum_pg_constraint_confkey, &isNull);
1224-
if (isNull)
1225-
elog(ERROR, "null confkey for constraint %u", constrId);
1217+
adatum = SysCacheGetAttrNotNull(CONSTROID, tuple,
1218+
Anum_pg_constraint_confkey);
12261219
arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */
12271220
if (ARR_NDIM(arr) != 1 ||
12281221
ARR_DIMS(arr)[0] != numkeys ||
@@ -1235,10 +1228,8 @@ DeconstructFkConstraintRow(HeapTuple tuple, int *numfks,
12351228

12361229
if (pf_eq_oprs)
12371230
{
1238-
adatum = SysCacheGetAttr(CONSTROID, tuple,
1239-
Anum_pg_constraint_conpfeqop, &isNull);
1240-
if (isNull)
1241-
elog(ERROR, "null conpfeqop for constraint %u", constrId);
1231+
adatum = SysCacheGetAttrNotNull(CONSTROID, tuple,
1232+
Anum_pg_constraint_conpfeqop);
12421233
arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */
12431234
/* see TryReuseForeignKey if you change the test below */
12441235
if (ARR_NDIM(arr) != 1 ||
@@ -1253,10 +1244,8 @@ DeconstructFkConstraintRow(HeapTuple tuple, int *numfks,
12531244

12541245
if (pp_eq_oprs)
12551246
{
1256-
adatum = SysCacheGetAttr(CONSTROID, tuple,
1257-
Anum_pg_constraint_conppeqop, &isNull);
1258-
if (isNull)
1259-
elog(ERROR, "null conppeqop for constraint %u", constrId);
1247+
adatum = SysCacheGetAttrNotNull(CONSTROID, tuple,
1248+
Anum_pg_constraint_conppeqop);
12601249
arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */
12611250
if (ARR_NDIM(arr) != 1 ||
12621251
ARR_DIMS(arr)[0] != numkeys ||
@@ -1270,10 +1259,8 @@ DeconstructFkConstraintRow(HeapTuple tuple, int *numfks,
12701259

12711260
if (ff_eq_oprs)
12721261
{
1273-
adatum = SysCacheGetAttr(CONSTROID, tuple,
1274-
Anum_pg_constraint_conffeqop, &isNull);
1275-
if (isNull)
1276-
elog(ERROR, "null conffeqop for constraint %u", constrId);
1262+
adatum = SysCacheGetAttrNotNull(CONSTROID, tuple,
1263+
Anum_pg_constraint_conffeqop);
12771264
arr = DatumGetArrayTypeP(adatum); /* ensure not toasted */
12781265
if (ARR_NDIM(arr) != 1 ||
12791266
ARR_DIMS(arr)[0] != numkeys ||

0 commit comments

Comments
 (0)