Re: ALTER command reworks - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: ALTER command reworks
Date
Msg-id 20130107204327.GA10595@perhan.postgresql.org
Whole thread Raw
In response to Re: ALTER command reworks  (Kohei KaiGai <kaigai@kaigai.gr.jp>)
Responses Re: ALTER command reworks
List pgsql-hackers
Kohei KaiGai escribió:

> The attached patch is a revised version.
>
> It follows the manner in ExecAlterObjectSchemaStmt(); that tries to check
> name duplication for object classes that support catcache with name-key.
> Elsewhere, it calls individual routines for each object class to solve object
> name and check namespace conflicts.
> For example, RenameFunction() is still called from ExecRenameStmt(),
> however, it looks up the given function name and checks namespace
> conflict only, then it just invokes AlterObjectRename_internal() in spite
> of system catalog updates by itself.
> It allows to use this consolidated routine by object classes with special
> rule for namespace conflicts, such as collation.
> In addition, this design allowed to eliminate get_object_type() to pull
> OBJECT_* enum from class_id/object_id pair.

I checked this patch.  It needed a rebase for the changes to return
OIDs.  Attached patch applies to current HEAD.  In general this looks
good, with one exception: it's using getObjectDescriptionOids() to build
the messages to complain in case the object already exists in the
current schema, which results in diffs like this:

-ERROR:  event trigger "regress_event_trigger2" already exists
+ERROR:  event trigger regress_event_trigger2 already exists

I don't know how tense we are about keeping the quotes, but I fear there
would be complaints because it took us lots of sweat, blood and tears to
get where we are now.

If this is considered a problem, I think the way to fix it is to have a
getObjectDescriptionOids() variant that quotes the object name in the
output.  This would be pretty intrusive: we'd have to change things
so that, for instance,

        appendStringInfo(&buffer, _("collation %s"),
            NameStr(coll->collname));

would become

        if (quotes)
            appendStringInfo(&buffer, _("collation \"%s\""),
                NameStr(coll->collname));
        else
            appendStringInfo(&buffer, _("collation %s"),
                NameStr(coll->collname));

Not really thrilled with this idea.  Of course,
getObjectDescription() itself should keep the same API as now, to avoid
useless churn all over the place.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Improve compression speeds in pg_lzcompress.c
Next
From: Robert Haas
Date:
Subject: Re: recent ALTER whatever .. SET SCHEMA refactoring