Re: Unify drop-by-OID functions

Em ter., 5 de mai. de 2020 às 18:11, Alvaro Herrera <
alvherre(at)2ndquadrant(dot)com> escreveu:

> 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