On Sun, Feb 6, 2011 at 4:15 AM, Noah Misch <noah@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?
>> 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.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company