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-12 12:07:53
Message-ID: CADyhKSUPY8k2z_QGdntasON_9xHEDXFT55OBhO8oGbm1yovAXA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert,

I'm currently trying to revise my patches according to your suggestions,
but I'm facing a trouble about error messages when user tries to drop
a non-exists object.

Because the ObjectProperty array has an entry for each catalogs, it is
unavailable to hold the name of object type (such as "table" or "index")
when multiple object types are associated with a particular system
catalog, such as pg_class, pg_type or pg_proc.

How should I implement the following block?

if (!OidIsValid(address.objectId))
{
ereport(NOTICE,
(errmsg("%s \"%s\" does not exist, skipping",
get_object_property_typetext(stmt->removeType),
NameListToString(objname))));
continue;
}

One idea is to add a separated array to translate from OBJECT_* to
its text representation. (Maybe, it can be used to translattions with
opposite direction.)

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.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Florian Pflug 2011-10-12 12:44:29 Re: Overhead cost of Serializable Snapshot Isolation
Previous Message Fujii Masao 2011-10-12 09:45:42 loss of transactions in streaming replication