On Mon, Jul 19, 2010 at 2:46 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On Sun, 2010-07-18 at 22:47 -0400, Robert Haas wrote:
>> But it seems
>> that it's far from clear what to do about it, and it's not the job of
>> this patch to fix it anyway.
>
> Agreed.
>
>> Regarding the actual patch, it looks mostly good. Questions:
>>
>> 1. Why in rewriteSupport.c are we adding a call to
>> heap_inplace_update() in some situations? Doesn't seem like this is
>> something we should need or want to be monkeying with.
>
> Hmm, yes, that looks like a hangover. Will change. No others similar.
>
>> 2. Instead of AlterTableGreatestLockLevel(), how about
>> AlterTableGetLockLevel()? Yeah, it's going to be the highest lock
>> level required by any subcommand, but it seems mildly overspecified.
>> I don't feel strongly about this one, though, if someone has a strong
>> contrary opinion...
>
> I felt it indicated the process it's using. Happy to change.
Cool. I think with those two changes it's time to commit this. It's
possible there's some case we've overlooked, but I think that we've
been over this fairly thoroughly, so hopefully not. Anyway, we're
doing this at the beginning of the 9.1 cycle rather than the end, so
there's hopefully time for any lingering bugs to be found...
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company