Skip site navigation (1) Skip section navigation (2)

Re: ALTER command reworks

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>,Robert Haas <robertmhaas(at)gmail(dot)com>,PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ALTER command reworks
Date: 2013-01-07 20:43:28
Message-ID: 20130107204327.GA10595@perhan.postgresql.org (view raw or flat)
Thread:
Lists: 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-v9.3-alter-reworks.3-rename.v8.patch
Description: text/plain (48.0 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Robert HaasDate: 2013-01-07 20:47:13
Subject: Re: recent ALTER whatever .. SET SCHEMA refactoring
Previous:From: Tom LaneDate: 2013-01-07 20:41:00
Subject: Re: Improve compression speeds in pg_lzcompress.c

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group