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

Marti Raudsepp <marti(at)juffo(dot)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