Re: Unify drop-by-OID functions - Mailing list pgsql-hackers

From Ranier Vilela
Subject Re: Unify drop-by-OID functions
Date
Msg-id CAEudQAqC0+GWJ-Fiw0M1Nnjy=nCH=+N35U9jUdhsCJh1XzBMrA@mail.gmail.com
Whole thread Raw
In response to Re: Unify drop-by-OID functions  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: Unify drop-by-OID functions
List pgsql-hackers
Em ter., 5 de mai. de 2020 às 18:11, Alvaro Herrera <alvherre@2ndquadrant.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

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: PG 13 release notes, first draft
Next
From: Masahiko Sawada
Date:
Subject: Re: xid wraparound danger due to INDEX_CLEANUP false