On 2020-May-05, Ranier Vilela wrote:
> And in that specific case, leaving resources blocked, which perhaps, in my
> humble opinion, could be released quickly.
I very much doubt that you can measure any difference at all between
these two codings of the function.
I agree with the principle you mention, of not holding resources for
long if they can be released earlier; but in this case the table_close
call occurs across a ReleaseSysCache() call, which is hardly of
significance. It's not like you have to wait for some other
transaction, or wait for I/O, or anything like that that would make it
measurable.
Locally it may not be as efficient, but it is a question, to follow the good programming practices, among them, not to break anything, not to block, and if it is not possible, release soon.
In at least one case, there will not even be a block, which is an improvement.
Another version, that could, be, without using ERROR.
+static void
+DropObjectById(const ObjectAddress *object)
+{
+ int cacheId;
+ Relation rel;
+ HeapTuple tup;
+
+ cacheId = get_object_catcache_oid(object->classId);
+
+ /*
+ * Use the system cache for the oid column, if one exists.
+ */
+ if (cacheId >= 0)
+ {
+ tup = SearchSysCache1(cacheId, ObjectIdGetDatum(object->objectId));
+ if (HeapTupleIsValid(tup)) {
+ rel = table_open(object->classId, RowExclusiveLock);
+ CatalogTupleDelete(rel, &tup->t_self);
+ table_close(rel, RowExclusiveLock);
+ ReleaseSysCache(tup);
+ }
+ else
+ elog(LOG, "cache lookup failed for %s %u",
+ get_object_class_descr(object->classId), object->objectId);
+ }
+ else
+ {
+ ScanKeyData skey[1];
+ SysScanDesc scan;
+
+ ScanKeyInit(&skey[0],
+ get_object_attnum_oid(object->classId),
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(object->objectId));
+
+ rel = table_open(object->classId, RowExclusiveLock);
+ scan = systable_beginscan(rel, get_object_oid_index(object->classId), true,
+ NULL, 1, skey);
+
+ /* we expect exactly one match */
+ tup = systable_getnext(scan);
+ if (HeapTupleIsValid(tup))
+ CatalogTupleDelete(rel, &tup->t_self);
+ else
+ elog(LOG, "could not find tuple for %s %u",
+ get_object_class_descr(object->classId), object->objectId);
+
+ systable_endscan(scan);
+ table_close(rel, RowExclusiveLock);
+ }
+}
+
regards,
Ranier Vilela