Re: Unify drop-by-OID functions

Em sex., 1 de mai. de 2020 às 11:39, Peter Eisentraut <
peter(dot)eisentraut(at)2ndquadrant(dot)com> escreveu:

> [proposal for PG 14]
>
> There are a number of Remove${Something}ById() functions that are
> essentially identical in structure and only different in which catalog
> they are working on. This patch refactors this to be one generic
> function. The information about which oid column, index, etc. to use
> was already available in ObjectProperty for most catalogs, in a few
> cases it was easily added.
>
> Conceivably, this could be taken further by categorizing more special
> cases as ObjectProperty fields or something like that, but this seemed
> like a good balance.
>
Very good.

I can suggest improvements?

1. In case Object is cached, delay open_table until the last moment, for
the row to be blocked as little as possible and close the table as quickly
as possible.
2. In case Object is cached and the tuple is invalid, do not open table.
3. Otherwise, is it possible to call systable_endscan, after table_close?

I think that lock resources, for as little time as possible, it is an
advantage..

+static void
+DropGenericById(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))
+ elog(ERROR, "cache lookup failed for %s entry %u",
+ get_object_class_descr(object->classId), object->objectId);
+
+ rel = table_open(object->classId, RowExclusiveLock);
+ CatalogTupleDelete(rel, &tup->t_self);
+ table_close(rel, RowExclusiveLock);
+
+ ReleaseSysCache(tup);
+ }
+ 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))
+ elog(ERROR, "could not find tuple for class %u entry %u",
+ object->classId, object->objectId);
+
+ CatalogTupleDelete(rel, &tup->t_self);
+ systable_endscan(scan);
+ table_close(rel, RowExclusiveLock);
+ }
+}
+

regards,
Ranier Vilela