From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> |
Cc: | PgHacker <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: ALTER command reworks |
Date: | 2012-08-30 19:24:06 |
Message-ID: | CA+TgmoaEiwecLHxk68TK3-c7uV==fw05PREhOFOs4avRaBPPpw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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.
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?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2012-08-30 19:25:37 | Re: PATCH: pgbench - aggregation of info written into log |
Previous Message | Tomas Vondra | 2012-08-30 19:17:46 | Re: PATCH: optimized DROP of multiple tables within a transaction |