Downgrade errors in object_ownercheck() to internal

The "does not exist" errors in object_ownership() were written as
ereport(), suggesting that they are user-facing.  But no code path
except one can reach this function without first checking that the
object exists.  If this were actually a user-facing error message,
then there would be some problems: get_object_class_descr() is meant
to be for internal errors only and does not support translation.

The one case that can reach this without first checking the object
existence is from be_lo_unlink().  (This makes some sense since large
objects are referred to by their OID directly.)  In this one case, we
can add a line of code to check the object existence explicitly,
consistent with other LO code.

For the rest, downgrade the error messages to elog()s.  The new
message wordings are the same as in DropObjectById().

Reviewed-by: Alvaro Herrera <[email protected]>
Discussion: https://www.postgresql.org/message-id/flat/da2f8942-be6d-48d0-ac1c-a053370a6b1f@eisentraut.org

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index b196294fb2906620bcda0ddd1b6c885a5d7c1da9..bd006931938ff4a53119fd32cfb752101e0a1247 100644 (file)
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -4082,9 +4082,8 @@ object_ownercheck(Oid classid, Oid objectid, Oid roleid)
 
                tuple = SearchSysCache1(cacheid, ObjectIdGetDatum(objectid));
                if (!HeapTupleIsValid(tuple))
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_UNDEFINED_OBJECT),
-                                        errmsg("%s with OID %u does not exist", get_object_class_descr(classid), objectid)));
+                       elog(ERROR, "cache lookup failed for %s %u",
+                                get_object_class_descr(classid), objectid);
 
                ownerId = DatumGetObjectId(SysCacheGetAttrNotNull(cacheid,
                                                                                                                  tuple,
@@ -4113,9 +4112,8 @@ object_ownercheck(Oid classid, Oid objectid, Oid roleid)
 
                tuple = systable_getnext(scan);
                if (!HeapTupleIsValid(tuple))
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_UNDEFINED_OBJECT),
-                                        errmsg("%s with OID %u does not exist", get_object_class_descr(classid), objectid)));
+                       elog(ERROR, "could not find tuple for %s %u",
+                                get_object_class_descr(classid), objectid);
 
                ownerId = DatumGetObjectId(heap_getattr(tuple,
                                                                                                get_object_attnum_owner(classid),

diff --git a/src/backend/libpq/be-fsstubs.c b/src/backend/libpq/be-fsstubs.c
index a272e82b85078c507b8c6c0f5c2a62cd375c3c42..e5a34c6193165f609e0f9ce69b1c7fe8c5256c15 100644 (file)
--- a/src/backend/libpq/be-fsstubs.c
+++ b/src/backend/libpq/be-fsstubs.c
@@ -317,6 +317,11 @@ be_lo_unlink(PG_FUNCTION_ARGS)
 
        PreventCommandIfReadOnly("lo_unlink()");
 
+       if (!LargeObjectExists(lobjId))
+               ereport(ERROR,
+                               (errcode(ERRCODE_UNDEFINED_OBJECT),
+                                errmsg("large object %u does not exist", lobjId)));
+
        /*
         * Must be owner of the large object.  It would be cleaner to check this
         * in inv_drop(), but we want to throw the error before not after closing