Re: Request for Comments: ALTER [OBJECT] SET SCHEMA

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bernd Helmle <mailings(at)oopsware(dot)de>
Cc: PostgreSQL-Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Request for Comments: ALTER [OBJECT] SET SCHEMA
Date: 2005-06-08 18:49:56
Message-ID: 9725.1118256596@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Bernd Helmle <mailings(at)oopsware(dot)de> writes:
> You can find a preliminary patch attached to this posting and i'm looking
> for comments, critics and perhaps some proposals for improvements /
> necessary changes i didn't consider yet.

The code seems fairly schizoid about whether the operation is an "alter
namespace" or a "rename". Please be consistent. I'd say it is *not*
a rename, but I suppose you could make an argument the other way ...

The locking you are doing is inconsistent with the rest of the backend.
We generally don't hold locks on catalogs longer than necessary.

Applying "const" to pointers that point to things that are not const,
as in

+ void
+ ApplyTypeNamespace( Oid typeOid,
+ const Relation rel,

seems to me to be horrible style, even if the compiler lets you do it.
It's too easy to misread it as a promise not to alter the pointed-to
object.

(In general I dislike consts on parameters, as that seems to me to be
conflating interface and implementation --- it's certainly no business
of a caller's whether your routine modifies the parameter internally.
Of course this is C's fault not yours, but one has to work with the
language one has.)

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marc G. Fournier 2005-06-08 19:20:46 Re: The Contrib Roundup (long)
Previous Message Alvaro Herrera 2005-06-08 18:48:55 Re: Request for Comments: ALTER [OBJECT] SET SCHEMA