Skip site navigation (1) Skip section navigation (2)

Re: ALTER TYPE 2: skip already-provable no-work rewrites

From: Noah Misch <noah(at)leadboat(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: ALTER TYPE 2: skip already-provable no-work rewrites
Date: 2011-02-06 09:15:53
Message-ID: 20110206091553.GA14423@tornado.gateway.2wire.net (view raw or flat)
Thread:
Lists: pgsql-hackers
On Sun, Feb 06, 2011 at 02:15:47AM -0500, Robert Haas wrote:
> On Sat, Feb 5, 2011 at 7:44 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> >> But this is a little unsatisfying, for two reasons. ?First, the error
> >> message will be subtly wrong: we can make it complain about a table or
> >> a type, but not a foreign table. ?At a quick look, it likes the right
> >> fix might be to replace the second and third arguments to
> >> find_composite_type_dependencies() with a Relation.
> >
> > Seems like a clear improvement.
> 
> 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.

> >> Second, I wonder
> >> if we shouldn't refactor things so that all the checks fire in
> >> ATRewriteTables() rather than doing them in different places. ?Seems
> >> like that might be cleaner.
> >
> > Offhand, this seems reasonable, too. ?I assumed there was some good reason it
> > couldn't be done there for non-tables, but nothing comes to mind.
> 
> Actually, thinking about this more, I'm thinking if we're going to
> change anything, it seems we ought to go the other way.  If we ever
> actually did support recursing into wherever the composite type
> dependencies take us, we'd want to detect that before phase 3 and add
> work-queue items for each table that we needed to futz with.
> 
> 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.

nm

In response to

Responses

pgsql-hackers by date

Next:From: Jan UrbaƄskiDate: 2011-02-06 09:52:56
Subject: Re: pl/python explicit subtransactions
Previous:From: Alex HunsakerDate: 2011-02-06 07:43:51
Subject: Re: arrays as pl/perl input arguments [PATCH]

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group