Joe Conway <mail@joeconway.com> writes:
> Items 1 and 2 from the proposal above are implemented in the attached
> patch.
Good first cut, but...
I don't like the way you did the dependencies. In a normal relation,
the type row is internally dependent on the class row. This causes the
type to automatically go away if the relation is dropped, and it also
prevents an attempt to manually drop the type directly (without dropping
the relation). For a composite type, of course we want exactly the
opposite behavior: the class row should internally depend on the type,
not vice versa.
If you did it that way then you'd not need that ugly kluge in
RemoveType. What you'd need instead is some smarts (a kluge!?) in
setting up the dependency. Currently that dependency is made in
TypeCreate which doesn't know what sort of relation it's creating
a type for. Probably the best answer is to pull that particular
dependency out of TypeCreate, and make it (in the proper direction)
in AddNewRelationType.
Also, I'm not following the point of the separation between
DefineCompositeType and DefineCompositeTypeRelation; nor do I see a need
for a CommandCounterIncrement call in there.
You have missed a number of places where this new relkind ought to
be special-cased the same way RELKIND_VIEW is --- for example
CheckAttributeNames and AddNewAttributeTuples, since a composite type
presumably shouldn't have system columns associated. I'd counsel
looking at all references to RELKIND_VIEW to see which places also need
to check for RELKIND_COMPOSITE_TYPE.
regards, tom lane