On Sun, Feb 06, 2011 at 07:54:52AM -0500, Robert Haas wrote: > On Sun, Feb 6, 2011 at 4:15 AM, Noah Misch <n...@leadboat.com> wrote: > >> That didn't quite work, because there's a caller in typecmds.c that > >> doesn't have the relation handy. ?So I made it take a relkind and a > >> name, which works fine. > > > > Hmm, indeed. ?In get_rels_with_domain(), it's a scalar type. > > Yeeeeeeah, that's actually a little ugly. It's actually a domain > over a composite type, not a composite type proper, IIUC. Better > ideas?
There are no domains over composite types. get_rels_with_domain() finds all relations having columns of the (scalar) domain type. It then calls find_composite_type_dependencies to identify uses of the composite types discovered in the previous step. Honestly, RELKIND_COMPOSITE_TYPE is a reasonable choice despite the technical mismatch. One more-correct approach would be to have two arguments, a catalog OID (pg_type or pg_class, currently) and a relkind, 0 when the catalog OID != pg_class. Might be an improvement, albeit a minor one. > >> The attached patch takes this approach. ?It's very slightly more code, > >> but it reduces the amount of spooky action at a distance. > > > >> Comments? > > > > Your patch improves the code. ?My standard for commending a refactor-only > > patch > > is rather high, though, and this patch doesn't reach it. ?The ancestral code > > placement wasn't obviously correct, but neither is this. ?So I'd vote -0. > > Well, what's your suggestion, then? Your patch pops the test up from > ATRewriteTable() up to ATRewriteTables(), but that's not obviously > correct either, and it's an awkward place to do it because we don't > have the Relation object handy at the point where the check needs to > be done. Agreed. I couldn't think of any grand improvements, so my patch bore the conceptually-smallest change I could come up with to handle that particular issue. That is, I felt it was the change that could most easily be verified as rejecting the same cases as the old test. nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
