Re: ALTER command reworks

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ALTER command reworks
Date: 2012-08-31 04:27:34
Message-ID: CADyhKSUY5YEh6BH4pjiRNfyrj48XprLER057A_JVL-8stChPeA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

2012/8/30 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Mon, Aug 13, 2012 at 3:35 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> The attached patch is a refreshed version of ALTER command
>> reworks towards the latest tree. Here is few big changes except
>> for code integration of the code to rename event triggers.
>
> This seems to have bit-rotted a bit. Please rebase.
>
>> BTW, I had to adjust between oid of pg_largeobject_metadata
>> and pg_largeobject on three points of this patch, like:
>>
>> if (classId == LargeObjectRelationId)
>> classId = LargeObjectMetadataRelationId;
>>
>> When we supported largeobject permission features, we put
>> special handling to track dependency of its ownership.
>> The pg_depend records oid of pg_largeobject, instead of
>> pg_largeobject_metadata. Thus, we cannot use classId of
>> ObjectAddress being returned from get_object_address()
>> as an argument of heap_open() as is, if it indicates oid of
>> pg_largeobject.
>>
>> Was it a right decision to track dependency of large object using
>> oid of pg_largeobject, instead of pg_largeobject_metadata?
>> IIRC, the reason why we used oid of pg_largeobject is backward
>> compatibility for applications that tries to reference pg_depend
>> with built-in oids.
>
> I think it was a terrible decision and I'm pretty sure I said as much
> at the time, but I lost the argument, so I'm inclined to think we're
> stuck with continuing to support that kludge.
>
OK, we will keep to implement carefully...

> Some other preliminary comments:
>
> - Surely you need to take AccessExclusiveLock on the object being
> renamed, not just ShareUpdateExclusiveLock.
> - I don't think it's acceptable to assemble the object-type
> "object-name" already exists message using getObjectDescription();
> it's not good for translators. Use an array of messages, one per
> object-type, as we have done in other cases.
> - I would like to handle either the RENAME case or the ALTER OWNER
> case in one patch, and the other in a follow-on patch. Can you pick
> one to do first and remove everything related to the other one?
>
OK, I'll split the patch into three (isn't it?) portions; RENAME, SET OWNER
and SET SCHEMA, with all above your suggestions.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2012-08-31 04:59:51 Re: foreign key locks
Previous Message Bruce Momjian 2012-08-31 04:09:54 Re: Using pg_upgrade on log-shipping standby servers