Re: [v9.2] DROP statement reworks

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.2] DROP statement reworks
Date: 2011-10-13 16:46:16
Message-ID: CADyhKSXuMh6sELxJc52sZm__7-1yP1FKbdFgxboi_CfVQZ+PXg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The attached patches are revised one, based on the latest version.

The get_object_namespace() got simplified to take only ObjectAddress,
and attnum_namespace was added to ObjectPropertyType array.

The DropErrorMsgNonExistent() is a function to raise a notice
when the supplied object name was missing and IF EXISTS was
also appended.
Unfortunately, it was not possible to hold all the notification
messages for each object types an array something like
dropmsgstringarray[] in tablecmds.c, because some of object
type requires to generate "%s" part using characteristic way.

Like:
+ case OBJECT_CAST:
+ msgfmt = gettext_noop("cast from type %s to type %s does
not exist, skipping");
+ marg1 = format_type_be(typenameTypeId(NULL,
+ (TypeName *) linitial(objname)));
+ marg2 = format_type_be(typenameTypeId(NULL,
+ (TypeName *) linitial(objargs)));
+ break;

So, I used a big switch statement to handle notification messages
when IF EXISTS was given.

And, also I added regression test cases to detect these code paths,
because some of object types does not cover the case when it was
dropped.

Reworking ALTER xxx SET SCHEMA/RENAME TO/SET OWNER
may be a homework of mine towards the next commit fest?

Thanks,

2011/10/11 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Mon, Oct 10, 2011 at 1:38 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> I'm sorry again. I tought it was obvious from the filenames.
>
> I guess I got confused because you re-posted part 2 without the other
> parts, and I got mixed up and thought you were reposting part one.
>
> I've committed a stripped-down version of the part one patch, which
> had several mistakes even in just the part I committed - e.g., you
> forgot TABLESPACEOID.  I also did some renaming for clarity.
>
> I'm going to throw this back to you for rebasing at this point, which
> I realize is going to be somewhat of an unenjoyable task given the way
> I cut up your changes to objectaddress.c, but I wasn't very confident
> that all of the entries were correct (the one for attributes seemed
> clearly wrong to me, for example), and I didn't want to commit a bunch
> of stuff that wasn't going to be exercised.  I suggest that you merge
> the remainder of the part-one changes into part-two.  On the flip
> side, I think you should take the stuff that deals with dropping
> relations OUT of part two.  I don't see what good it does us to try to
> centralize the drop logic if we still have to have special cases for
> relations, so let's just leave that separate for now until we figure
> out a better approach, or at least split it off as a separate patch so
> that it doesn't hold up all the other changes.
>
> I think get_object_namespace() needs substantial revision.  Instead of
> passing the object type and the object address, why not just pass the
> object address?  You should be able to use the classId in the address
> to figure out everything you need to know.  Since this function is
> private to objectaddress.c, there's no reason for it to use those
> accessor functions - it can just iterate through the array just as
> object_exists() does.  That way you also avoid iterating through the
> array multiple times.  I also think that we probably ought to revise
> AlterObjectNamespace() to make use of this new machinery, instead of
> making the caller pass in all the same information that
> objectaddress.c is now learning how to provide.  That would possibly
> open the way to a bunch more consolidation of the SET SCHEMA code; in
> fact, we might want to clean that up first, before dealing with the
> DROP stuff.
>
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Attachment Content-Type Size
pgsql-v9.2-drop-reworks-2.v5.patch application/octet-stream 41.6 KB
pgsql-v9.2-drop-reworks-3.v5.patch application/octet-stream 71.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2011-10-13 17:02:40 Re: Change 'pg_ctl: no server running' Exit Status to 3
Previous Message Kohei KaiGai 2011-10-13 16:27:12 Re: [bug] relcache leaks in get_object_address