Re: plpgsql versus domains

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2015-02-26 13:53:18 -0500, Tom Lane wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
>>> You're probably going to kill me because of the increased complexity,
>>> but how about making typecache.c smarter, more in the vein of
>>> relcache.c, and store the relevant info in there?

>> I thought that's what I was proposing in point #1.

> Sure, but my second point was to avoid duplicating that information into
> ->fcinfo and such and instead reference the typecache entry from
> there. So that if the typecache entry is being rebuilt (a new mechanism
> I'm afraid), whatever uses ->fcinfo gets the updated
> functions/definition.

Here's a draft patch for that. This fixes the delayed-domain-constraint
problem pretty thoroughly, in that any attempt to check a domain's
constraints will use fully up-to-date info.

This is the first attempt at weaponizing the memory context reset/delete
feature, and I'm fairly happy with it, except for one thing: I had to
#include utils/memnodes.h into typcache.h in order to preserve the
intended property that the callback structs could be included directly
into structs using them. Now, that's not awful in itself, because
typcache.h isn't used everywhere; but if this feature gets popular we'll
find memnodes.h being included pretty much everywhere, which is not so
great. I'm thinking about moving struct MemoryContextCallback and the
extern for MemoryContextRegisterResetCallback into palloc.h to avoid this.
Maybe that's too far in the other direction. Thoughts? Compromise ideas?

Also, instrumenting the code showed that TypeCacheConstrCallback gets
called quite a lot during the standard regression tests, which is why
I went out of my way to make it quick. Almost all of those cache flushes
are from non-domain-related updates on pg_type or pg_constraint, so
they're not really necessary from a logical perspective, and they're
surely going to hurt performance for heavy users of domains. I think to
fix this we'd have to split pg_constraint into two catalogs, one for table
constraints and one for domain constraints; which would be a good thing
anyway from a normal-form-theory perspective. And we'd have to get rid of
pg_type.typnotnull and instead store domain NOT NULL constraints in this
hypothetical domain constraint catalog. I don't plan to do anything about
that myself right now, because I'm not sure that production databases
would have the kind of traffic on pg_type and pg_constraint that the
regression tests exhibit. But at some point we might have to fix it.

regards, tom lane