Re: Unify drop-by-OID functions

Em ter., 5 de mai. de 2020 às 14:57, Robert Haas <robertmhaas(at)gmail(dot)com>
escreveu:

> On Tue, May 5, 2020 at 1:43 PM Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> > I see, the famous "cliché".
>
> By using the word cliché, and by putting it in quotes, you seem to
> suggest that you consider my argument dubious. However, I stand by it.
> Code shouldn't be changed without understanding the reasons behind the
> current coding. Doing so very often breaks things.
>
Sorry Robert, It was not my intention. I didn't know that using quotes
would change your understanding.
Of course, I find your arguments very valid and valuable.
And I understand that there are many interrelated things, which can break
if done in the wrong order.
Maybe I used the wrong word, in this case, the cliché.
What I mean is, the cliché, does some strange things, like leaving
variables to be declared, assigned in and not used.
And in that specific case, leaving resources blocked, which perhaps, in my
humble opinion, could be released quickly.
I think the expected behavior is being the same, with the changes I
proposed, IMHO.

+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))
+ elog(ERROR, "cache lookup failed for %s %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 %s %u",
+ get_object_class_descr(object->classId), object->objectId);
+
+ CatalogTupleDelete(rel, &tup->t_self);
+ systable_endscan(scan);
+
+ table_close(rel, RowExclusiveLock);
+ }
+}

And again, your opinion is very important to me.

best regards,
Ranier Vilela