Skip to content

Commit 2a10fdc

Browse files
michaelpqmichaelpq
committed
Eliminate cache lookup errors in SQL functions for object addresses
When using the following functions, users could see various types of errors of the type "cache lookup failed for OID XXX" with elog(), that can only be used for internal errors: * pg_describe_object() * pg_identify_object() * pg_identify_object_as_address() The set of APIs managing object addresses for all object types are made smarter by gaining a new argument "missing_ok" that allows any caller to control if an error is raised or not on an undefined object. The SQL functions listed above are changed to handle the case where an object is missing. Regression tests are added for all object types for the cases where these are undefined. Before this commit, these cases failed with cache lookup errors, and now they basically return NULL (minus the name of the object type requested). Author: Michael Paquier Reviewed-by: Aleksander Alekseev, Dmitry Dolgov, Daniel Gustafsson, Álvaro Herrera, Kyotaro Horiguchi Discussion: https://postgr.es/m/CAB7nPqSZxrSmdHK-rny7z8mi=EAFXJ5J-0RbzDw6aus=wB5azQ@mail.gmail.com
1 parent 689696c commit 2a10fdc

File tree

19 files changed

+996
-332
lines changed

19 files changed

+996
-332
lines changed

‎contrib/sepgsql/database.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ sepgsql_database_drop(Oid databaseId)
142142
object.classId = DatabaseRelationId;
143143
object.objectId = databaseId;
144144
object.objectSubId = 0;
145-
audit_name = getObjectIdentity(&object);
145+
audit_name = getObjectIdentity(&object, false);
146146

147147
sepgsql_avc_check_perms(&object,
148148
SEPG_CLASS_DB_DATABASE,
@@ -169,7 +169,7 @@ sepgsql_database_setattr(Oid databaseId)
169169
object.classId = DatabaseRelationId;
170170
object.objectId = databaseId;
171171
object.objectSubId = 0;
172-
audit_name = getObjectIdentity(&object);
172+
audit_name = getObjectIdentity(&object, false);
173173

174174
sepgsql_avc_check_perms(&object,
175175
SEPG_CLASS_DB_DATABASE,
@@ -193,7 +193,7 @@ sepgsql_database_relabel(Oid databaseId, const char *seclabel)
193193
object.classId = DatabaseRelationId;
194194
object.objectId = databaseId;
195195
object.objectSubId = 0;
196-
audit_name = getObjectIdentity(&object);
196+
audit_name = getObjectIdentity(&object, false);
197197

198198
/*
199199
* check db_database:{setattr relabelfrom} permission

‎contrib/sepgsql/dml.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ check_relation_privileges(Oid relOid,
179179
object.classId = RelationRelationId;
180180
object.objectId = relOid;
181181
object.objectSubId = 0;
182-
audit_name = getObjectIdentity(&object);
182+
audit_name = getObjectIdentity(&object, false);
183183
switch (relkind)
184184
{
185185
case RELKIND_RELATION:
@@ -256,7 +256,7 @@ check_relation_privileges(Oid relOid,
256256
object.classId = RelationRelationId;
257257
object.objectId = relOid;
258258
object.objectSubId = attnum;
259-
audit_name = getObjectDescription(&object);
259+
audit_name = getObjectDescription(&object, false);
260260

261261
result = sepgsql_avc_check_perms(&object,
262262
SEPG_CLASS_DB_COLUMN,

‎contrib/sepgsql/label.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ sepgsql_fmgr_hook(FmgrHookEventType event,
355355
sepgsql_avc_check_perms(&object,
356356
SEPG_CLASS_DB_PROCEDURE,
357357
SEPG_DB_PROCEDURE__ENTRYPOINT,
358-
getObjectDescription(&object),
358+
getObjectDescription(&object, false),
359359
true);
360360

361361
sepgsql_avc_check_perms_label(stack->new_label,
@@ -523,7 +523,7 @@ sepgsql_object_relabel(const ObjectAddress *object, const char *seclabel)
523523
ereport(ERROR,
524524
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
525525
errmsg("sepgsql provider does not support labels on %s",
526-
getObjectTypeDescription(object))));
526+
getObjectTypeDescription(object, false))));
527527
break;
528528
}
529529
}

‎contrib/sepgsql/proc.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ sepgsql_proc_post_create(Oid functionId)
8080
sepgsql_avc_check_perms(&object,
8181
SEPG_CLASS_DB_SCHEMA,
8282
SEPG_DB_SCHEMA__ADD_NAME,
83-
getObjectIdentity(&object),
83+
getObjectIdentity(&object, false),
8484
true);
8585

8686
/*
@@ -114,7 +114,7 @@ sepgsql_proc_post_create(Oid functionId)
114114
object.classId = TypeRelationId;
115115
object.objectId = proForm->proargtypes.values[i];
116116
object.objectSubId = 0;
117-
appendStringInfoString(&audit_name, getObjectIdentity(&object));
117+
appendStringInfoString(&audit_name, getObjectIdentity(&object, false));
118118
}
119119
appendStringInfoChar(&audit_name, ')');
120120

@@ -164,7 +164,7 @@ sepgsql_proc_drop(Oid functionId)
164164
object.classId = NamespaceRelationId;
165165
object.objectId = get_func_namespace(functionId);
166166
object.objectSubId = 0;
167-
audit_name = getObjectIdentity(&object);
167+
audit_name = getObjectIdentity(&object, false);
168168

169169
sepgsql_avc_check_perms(&object,
170170
SEPG_CLASS_DB_SCHEMA,
@@ -179,7 +179,7 @@ sepgsql_proc_drop(Oid functionId)
179179
object.classId = ProcedureRelationId;
180180
object.objectId = functionId;
181181
object.objectSubId = 0;
182-
audit_name = getObjectIdentity(&object);
182+
audit_name = getObjectIdentity(&object, false);
183183

184184
sepgsql_avc_check_perms(&object,
185185
SEPG_CLASS_DB_PROCEDURE,
@@ -204,7 +204,7 @@ sepgsql_proc_relabel(Oid functionId, const char *seclabel)
204204
object.classId = ProcedureRelationId;
205205
object.objectId = functionId;
206206
object.objectSubId = 0;
207-
audit_name = getObjectIdentity(&object);
207+
audit_name = getObjectIdentity(&object, false);
208208

209209
/*
210210
* check db_procedure:{setattr relabelfrom} permission
@@ -292,7 +292,7 @@ sepgsql_proc_setattr(Oid functionId)
292292
object.classId = ProcedureRelationId;
293293
object.objectId = functionId;
294294
object.objectSubId = 0;
295-
audit_name = getObjectIdentity(&object);
295+
audit_name = getObjectIdentity(&object, false);
296296

297297
sepgsql_avc_check_perms(&object,
298298
SEPG_CLASS_DB_PROCEDURE,
@@ -324,7 +324,7 @@ sepgsql_proc_execute(Oid functionId)
324324
object.classId = ProcedureRelationId;
325325
object.objectId = functionId;
326326
object.objectSubId = 0;
327-
audit_name = getObjectIdentity(&object);
327+
audit_name = getObjectIdentity(&object, false);
328328
sepgsql_avc_check_perms(&object,
329329
SEPG_CLASS_DB_PROCEDURE,
330330
SEPG_DB_PROCEDURE__EXECUTE,

‎contrib/sepgsql/relation.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ sepgsql_attribute_post_create(Oid relOid, AttrNumber attnum)
102102

103103
initStringInfo(&audit_name);
104104
appendStringInfo(&audit_name, "%s.%s",
105-
getObjectIdentity(&object),
105+
getObjectIdentity(&object, false),
106106
quote_identifier(NameStr(attForm->attname)));
107107
sepgsql_avc_check_perms_label(ncontext,
108108
SEPG_CLASS_DB_COLUMN,
@@ -146,7 +146,7 @@ sepgsql_attribute_drop(Oid relOid, AttrNumber attnum)
146146
object.classId = RelationRelationId;
147147
object.objectId = relOid;
148148
object.objectSubId = attnum;
149-
audit_name = getObjectIdentity(&object);
149+
audit_name = getObjectIdentity(&object, false);
150150

151151
sepgsql_avc_check_perms(&object,
152152
SEPG_CLASS_DB_COLUMN,
@@ -178,7 +178,7 @@ sepgsql_attribute_relabel(Oid relOid, AttrNumber attnum,
178178
object.classId = RelationRelationId;
179179
object.objectId = relOid;
180180
object.objectSubId = attnum;
181-
audit_name = getObjectIdentity(&object);
181+
audit_name = getObjectIdentity(&object, false);
182182

183183
/*
184184
* check db_column:{setattr relabelfrom} permission
@@ -222,7 +222,7 @@ sepgsql_attribute_setattr(Oid relOid, AttrNumber attnum)
222222
object.classId = RelationRelationId;
223223
object.objectId = relOid;
224224
object.objectSubId = attnum;
225-
audit_name = getObjectIdentity(&object);
225+
audit_name = getObjectIdentity(&object, false);
226226

227227
sepgsql_avc_check_perms(&object,
228228
SEPG_CLASS_DB_COLUMN,
@@ -288,7 +288,7 @@ sepgsql_relation_post_create(Oid relOid)
288288
sepgsql_avc_check_perms(&object,
289289
SEPG_CLASS_DB_SCHEMA,
290290
SEPG_DB_SCHEMA__ADD_NAME,
291-
getObjectIdentity(&object),
291+
getObjectIdentity(&object, false),
292292
true);
293293

294294
switch (classForm->relkind)
@@ -450,7 +450,7 @@ sepgsql_relation_drop(Oid relOid)
450450
object.classId = NamespaceRelationId;
451451
object.objectId = get_rel_namespace(relOid);
452452
object.objectSubId = 0;
453-
audit_name = getObjectIdentity(&object);
453+
audit_name = getObjectIdentity(&object, false);
454454

455455
sepgsql_avc_check_perms(&object,
456456
SEPG_CLASS_DB_SCHEMA,
@@ -472,7 +472,7 @@ sepgsql_relation_drop(Oid relOid)
472472
object.classId = RelationRelationId;
473473
object.objectId = relOid;
474474
object.objectSubId = 0;
475-
audit_name = getObjectIdentity(&object);
475+
audit_name = getObjectIdentity(&object, false);
476476

477477
sepgsql_avc_check_perms(&object,
478478
tclass,
@@ -503,7 +503,7 @@ sepgsql_relation_drop(Oid relOid)
503503
object.classId = RelationRelationId;
504504
object.objectId = relOid;
505505
object.objectSubId = attForm->attnum;
506-
audit_name = getObjectIdentity(&object);
506+
audit_name = getObjectIdentity(&object, false);
507507

508508
sepgsql_avc_check_perms(&object,
509509
SEPG_CLASS_DB_COLUMN,
@@ -584,7 +584,7 @@ sepgsql_relation_relabel(Oid relOid, const char *seclabel)
584584
object.classId = RelationRelationId;
585585
object.objectId = relOid;
586586
object.objectSubId = 0;
587-
audit_name = getObjectIdentity(&object);
587+
audit_name = getObjectIdentity(&object, false);
588588

589589
/*
590590
* check db_xxx:{setattr relabelfrom} permission
@@ -695,7 +695,7 @@ sepgsql_relation_setattr(Oid relOid)
695695
object.classId = RelationRelationId;
696696
object.objectId = relOid;
697697
object.objectSubId = 0;
698-
audit_name = getObjectIdentity(&object);
698+
audit_name = getObjectIdentity(&object, false);
699699

700700
sepgsql_avc_check_perms(&object,
701701
tclass,

‎contrib/sepgsql/schema.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ sepgsql_schema_drop(Oid namespaceId)
123123
object.classId = NamespaceRelationId;
124124
object.objectId = namespaceId;
125125
object.objectSubId = 0;
126-
audit_name = getObjectIdentity(&object);
126+
audit_name = getObjectIdentity(&object, false);
127127

128128
sepgsql_avc_check_perms(&object,
129129
SEPG_CLASS_DB_SCHEMA,
@@ -148,7 +148,7 @@ sepgsql_schema_relabel(Oid namespaceId, const char *seclabel)
148148
object.classId = NamespaceRelationId;
149149
object.objectId = namespaceId;
150150
object.objectSubId = 0;
151-
audit_name = getObjectIdentity(&object);
151+
audit_name = getObjectIdentity(&object, false);
152152

153153
/*
154154
* check db_schema:{setattr relabelfrom} permission
@@ -186,7 +186,7 @@ check_schema_perms(Oid namespaceId, uint32 required, bool abort_on_violation)
186186
object.classId = NamespaceRelationId;
187187
object.objectId = namespaceId;
188188
object.objectSubId = 0;
189-
audit_name = getObjectIdentity(&object);
189+
audit_name = getObjectIdentity(&object, false);
190190

191191
result = sepgsql_avc_check_perms(&object,
192192
SEPG_CLASS_DB_SCHEMA,

‎doc/src/sgml/func.sgml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22826,7 +22826,8 @@ SELECT collation for ('foo' COLLATE "de_DE");
2282622826
object). This description is intended to be human-readable, and might
2282722827
be translated, depending on server configuration. This is especially
2282822828
useful to determine the identity of an object referenced in the
22829-
<structname>pg_depend</structname> catalog.
22829+
<structname>pg_depend</structname> catalog. This function returns
22830+
<literal>NULL</literal> values for undefined objects.
2283022831
</para></entry>
2283122832
</row>
2283222833

@@ -22858,7 +22859,8 @@ SELECT collation for ('foo' COLLATE "de_DE");
2285822859
otherwise <literal>NULL</literal>;
2285922860
<parameter>identity</parameter> is the complete object identity, with
2286022861
the precise format depending on object type, and each name within the
22861-
format being schema-qualified and quoted as necessary.
22862+
format being schema-qualified and quoted as necessary. Undefined
22863+
objects are identified with <literal>NULL</literal> values.
2286222864
</para></entry>
2286322865
</row>
2286422866

@@ -22915,6 +22917,7 @@ SELECT collation for ('foo' COLLATE "de_DE");
2291522917
<parameter>objsubid</parameter> is the sub-object ID, or zero if none.
2291622918
This function is the inverse
2291722919
of <function>pg_identify_object_as_address</function>.
22920+
Undefined objects are identified with <literal>NULL</literal> values.
2291822921
</para></entry>
2291922922
</row>
2292022923
</tbody>

‎src/backend/catalog/dependency.c

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -743,8 +743,8 @@ findDependentObjects(const ObjectAddress *object,
743743
if (!object_address_present_add_flags(object, objflags,
744744
targetObjects))
745745
elog(ERROR, "deletion of owning object %s failed to delete %s",
746-
getObjectDescription(&otherObject),
747-
getObjectDescription(object));
746+
getObjectDescription(&otherObject, false),
747+
getObjectDescription(object, false));
748748

749749
/* And we're done here. */
750750
return;
@@ -790,11 +790,11 @@ findDependentObjects(const ObjectAddress *object,
790790
* the depender fields...
791791
*/
792792
elog(ERROR, "incorrect use of PIN dependency with %s",
793-
getObjectDescription(object));
793+
getObjectDescription(object, false));
794794
break;
795795
default:
796796
elog(ERROR, "unrecognized dependency type '%c' for %s",
797-
foundDep->deptype, getObjectDescription(object));
797+
foundDep->deptype, getObjectDescription(object, false));
798798
break;
799799
}
800800
}
@@ -812,14 +812,14 @@ findDependentObjects(const ObjectAddress *object,
812812
char *otherObjDesc;
813813

814814
if (OidIsValid(partitionObject.classId))
815-
otherObjDesc = getObjectDescription(&partitionObject);
815+
otherObjDesc = getObjectDescription(&partitionObject, false);
816816
else
817-
otherObjDesc = getObjectDescription(&owningObject);
817+
otherObjDesc = getObjectDescription(&owningObject, false);
818818

819819
ereport(ERROR,
820820
(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
821821
errmsg("cannot drop %s because %s requires it",
822-
getObjectDescription(object), otherObjDesc),
822+
getObjectDescription(object, false), otherObjDesc),
823823
errhint("You can drop %s instead.", otherObjDesc)));
824824
}
825825

@@ -929,12 +929,12 @@ findDependentObjects(const ObjectAddress *object,
929929
ereport(ERROR,
930930
(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
931931
errmsg("cannot drop %s because it is required by the database system",
932-
getObjectDescription(object))));
932+
getObjectDescription(object, false))));
933933
subflags = 0; /* keep compiler quiet */
934934
break;
935935
default:
936936
elog(ERROR, "unrecognized dependency type '%c' for %s",
937-
foundDep->deptype, getObjectDescription(object));
937+
foundDep->deptype, getObjectDescription(object, false));
938938
subflags = 0; /* keep compiler quiet */
939939
break;
940940
}
@@ -1052,12 +1052,13 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
10521052
!(extra->flags & DEPFLAG_PARTITION))
10531053
{
10541054
const ObjectAddress *object = &targetObjects->refs[i];
1055-
char *otherObjDesc = getObjectDescription(&extra->dependee);
1055+
char *otherObjDesc = getObjectDescription(&extra->dependee,
1056+
false);
10561057

10571058
ereport(ERROR,
10581059
(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
10591060
errmsg("cannot drop %s because %s requires it",
1060-
getObjectDescription(object), otherObjDesc),
1061+
getObjectDescription(object, false), otherObjDesc),
10611062
errhint("You can drop %s instead.", otherObjDesc)));
10621063
}
10631064
}
@@ -1105,7 +1106,7 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
11051106
if (extra->flags & DEPFLAG_SUBOBJECT)
11061107
continue;
11071108

1108-
objDesc = getObjectDescription(obj);
1109+
objDesc = getObjectDescription(obj, false);
11091110

11101111
/*
11111112
* If, at any stage of the recursive search, we reached the object via
@@ -1129,7 +1130,8 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
11291130
}
11301131
else if (behavior == DROP_RESTRICT)
11311132
{
1132-
char *otherDesc = getObjectDescription(&extra->dependee);
1133+
char *otherDesc = getObjectDescription(&extra->dependee,
1134+
false);
11331135

11341136
if (numReportedClient < MAX_REPORTED_DEPS)
11351137
{
@@ -1187,7 +1189,7 @@ reportDependentObjects(const ObjectAddresses *targetObjects,
11871189
ereport(ERROR,
11881190
(errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
11891191
errmsg("cannot drop %s because other objects depend on it",
1190-
getObjectDescription(origObject)),
1192+
getObjectDescription(origObject, false)),
11911193
errdetail("%s", clientdetail.data),
11921194
errdetail_log("%s", logdetail.data),
11931195
errhint("Use DROP ... CASCADE to drop the dependent objects too.")));

0 commit comments

Comments
 (0)