Re: [PATCH] Cleanup: unify checks for catalog modification

Marti Raudsepp <marti@juffo.org> writes:
> I happened to notice that there are no less than 14 places in the code
> that check whether a relation is a system catalog and throwing the
> error "permission denied: "foo" is a system catalog"

> The attached patch factors all of those into a single
> ForbidSystemTableMods() function. Is this considered an improvement?

I'd argue not.  The code bulk savings is minimal, and this change
would degrade the usefulness of the file/line number reporting that's
built into ereport().  Admittedly it's a judgment call --- we've certainly
built error-reporting subroutines in cases where a significant amount of
complexity could be folded into the subroutine.  But I don't think this
case meets the threshold.
        regards, tom lane