Обсуждение: Request for Comments: ALTER [OBJECT] SET SCHEMA
I've took a look at the TODO item
Allow objects to be moved to different schemas
I've done some code so far which implements the syntax
ALTER [OBJECT] name SET SCHEMA name
where OBJECT currently is
SEQUENCE
TABLE
FUNCTION
DOMAIN
TYPE
Missing are (and i'm planning to add support for this):
AGGREGATE
OPERATOR
OPERATOR CLASS
CONVERSION
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.
One issue that comes to my mind is what to do when dealing with tables that
have assigned triggers and sequences (serials). Do we want to move them as
well or leave them in the source namespace?
TIA
--
Bernd
Вложения
On Wed, Jun 08, 2005 at 08:25:12PM +0200, Bernd Helmle wrote: > One issue that comes to my mind is what to do when dealing with tables that > have assigned triggers and sequences (serials). Do we want to move them as > well or leave them in the source namespace? I'd think it's important that the ALTER TABLE leaves things just like what you'd end up with if you created the table in the new schema in the first place. i.e., indexes, triggers, sequences should be moved too. One issue to check is what happens if you move the table and trigger but the function remains in the original namespace. Is this a problem if the new namespace is not in the search path? -- Alvaro Herrera (<alvherre[a]surnet.cl>) "This is a foot just waiting to be shot" (Andrew Dunstan)
Bernd Helmle <mailings@oopsware.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
> One issue that comes to my mind is what to do when dealing with tables > that have assigned triggers and sequences (serials). Do we want to move > them as well or leave them in the source namespace? They should all be moved. Remember nasties like indexes should be moved as well as toast tables. Chris
--On Donnerstag, Juni 09, 2005 10:33:08 +0800 Christopher Kings-Lynne <chriskl@familyhealth.com.au> wrote: >> One issue that comes to my mind is what to do when dealing with tables >> that have assigned triggers and sequences (serials). Do we want to move >> them as well or leave them in the source namespace? > > They should all be moved. Remember nasties like indexes should be moved > as well as toast tables. Oh, i thought toast tables should live in the pg_toast namespace? -- Bernd
--On Mittwoch, Juni 08, 2005 14:49:56 -0400 Tom Lane <tgl@sss.pgh.pa.us> wrote: > 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 ... No, i totally agree. Well, the Rename* stuff was influenced by my first shot, that follows the syntax ALTER OBJECT name RENAME SCHEMA TO name.... > > The locking you are doing is inconsistent with the rest of the backend. > We generally don't hold locks on catalogs longer than necessary. > Okay, needs to be adjusted. > 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. > Well, i thought there *should* be a promise, not to alter *rel in that specific case. -- Bernd
--On Mittwoch, Juni 08, 2005 14:48:55 -0400 Alvaro Herrera <alvherre@surnet.cl> wrote: > On Wed, Jun 08, 2005 at 08:25:12PM +0200, Bernd Helmle wrote: > >> One issue that comes to my mind is what to do when dealing with tables >> that have assigned triggers and sequences (serials). Do we want to move >> them as well or leave them in the source namespace? > > I'd think it's important that the ALTER TABLE leaves things just like > what you'd end up with if you created the table in the new schema in the > first place. i.e., indexes, triggers, sequences should be moved too. > That leads me to the question what gets attached to a table: SEQUENCE, INDEX, TRIGGER (function), CONSTRAINT, .... ? > One issue to check is what happens if you move the table and trigger but > the function remains in the original namespace. Is this a problem if > the new namespace is not in the search path? Hmm have triggers an own namespace? I can see in pg_trigger that they are attached to pg_proc, but can't see an own namespace specification... However, lets have a look at this example: bernd@[local]:bernd #= CREATE SCHEMA B; CREATE SCHEMA bernd@[local]:bernd #= set search_path TO b; SET bernd@[local]:bernd #= CREATE TABLE test ( id integer not null primary key ); NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "test_pkey" for table "test" CREATE TABLE bernd@[local]:bernd #= CREATE TABLE log_test ( usr text, log_time timestamp default NOW() ); CREATE TABLE ^ bernd@[local]:bernd #= CREATE OR REPLACE FUNCTION trigger_log_update() RETURNS TRIGGER AS $$ BEGIN INSERT INTO log_test VALUES( current_user ); RETURN new; END; $$ LANGUAGE PLPGSQL; CREATE FUNCTION bernd@[local]:bernd #= CREATE TRIGGER t_log_update AFTER UPDATE OR DELETE OR INSERT ON test FOR STATEMENT EXECUTE PROCEDURE trigger_log_update(); CREATE TRIGGER bernd@[local]:bernd #= INSERT INTO test VALUES (2); INSERT 0 1 bernd@[local]:bernd #= CREATE SCHEMA C; CREATE SCHEMA bernd@[local]:bernd #= ALTER TABLE test SET SCHEMA C; NOTICE: changed dependency to new schema "c" ALTER TABLE bernd@[local]:bernd #= SET search_path TO C; SET bernd@[local]:bernd #= INSERT INTO test VALUES (4); INSERT 0 1 So that works, but let's move the trigger function as well: bernd@[local]:bernd #= ALTER FUNCTION B.trigger_log_update() SET SCHEMA C; NOTICE: changed dependency to new schema "c" ALTER TABLE bernd@[local]:bernd #= INSERT INTO test VALUES (5); ERROR: relation "log_test" does not exist CONTEXT: SQL statement "INSERT INTO log_test VALUES( current_user )" PL/pgSQL function "trigger_log_update" line 1 at SQL statement So that doesn't work and it's likely that someone can mess up his schema with this, because the trigger function no longer finds its "log table". Don't know how to deal with that..... -- Bernd
>> They should all be moved. Remember nasties like indexes should be moved >> as well as toast tables. > > > Oh, i thought toast tables should live in the pg_toast namespace? Oh yes, you're probably right. Indexes should move though I think? Chris
--On Donnerstag, Juni 09, 2005 21:05:59 +0800 Christopher Kings-Lynne <chriskl@familyhealth.com.au> wrote: > > Oh yes, you're probably right. Indexes should move though I think? > Yes, i think so, too. -- Bernd
Bernd Helmle <mailings@oopsware.de> writes:
> --On Mittwoch, Juni 08, 2005 14:49:56 -0400 Tom Lane <tgl@sss.pgh.pa.us>
> wrote:
>> 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.
> Well, i thought there *should* be a promise, not to alter *rel in that
> specific case.
Hmm? You're planning to write into the relation in question. It's
hardly likely that the structure can be expected to remain virgin...
in practice I don't think we guarantee that even for read operations.
regards, tom lane
Bernd Helmle <mailings@oopsware.de> writes:
> --On Donnerstag, Juni 09, 2005 21:05:59 +0800 Christopher Kings-Lynne
>> Oh yes, you're probably right. Indexes should move though I think?
> Yes, i think so, too.
I don't think you have any choice about that --- I'm pretty sure that
there are places that assume a table's indexes are in the same schema
the table is. Constraints ditto.
regards, tom lane
--On Donnerstag, Juni 09, 2005 12:05:45 -0400 Tom Lane <tgl@sss.pgh.pa.us> wrote: > I don't think you have any choice about that --- I'm pretty sure that > there are places that assume a table's indexes are in the same schema > the table is. Constraints ditto. Okay, then the consenus is to go for it. -- Bernd
--On Donnerstag, Juni 09, 2005 10:17:33 -0400 Tom Lane <tgl@sss.pgh.pa.us> wrote: > Hmm? You're planning to write into the relation in question. It's > hardly likely that the structure can be expected to remain virgin... > in practice I don't think we guarantee that even for read operations. Oh, my stupid fault. Of course, that was a lack of understanding what Relation is on my side :( Will fix that. Thanks for your comments. -- Bernd
--On Freitag, Juni 10, 2005 21:20:33 +0200 ziga@mail.ljudmila.org wrote: > Wouldn't > > ALTER [OBJECT] RENAME TO [schema.][name] > > be a better? > > After all, this is essentially a rename operation, > so maybe it is better to extend existing syntax... I don't think it's a good idea to merge two different semantics: Renaming a table and "moving" a table to a different schema should be distinguished. Furthermore, i think it's too error prone, because people could accidently issue a "schema move" and renaming a table by a typo.... -- Bernd
Wouldn't ALTER [OBJECT] RENAME TO [schema.][name] be a better? After all, this is essentially a rename operation, so maybe it is better to extend existing syntax... 5WD-02-Foundation-2003-09.pdf doesn't seem to specify any renaming with ALTER TABLE...
What about: ALTER [OBJECT] RENAME TO [schema.]name [CASCADE] This has somewhat less new syntax. CASCADE would also move dependant objects. Perhaps trigger functions should not be moved, since it is not really obvious how to do this right. Warning should be issued in this case. Command basically frees user from having to hack system castalogs; Is moving dependant object really such a big deal? PostgreSQL does not really care where they are and just keeps working - maybe this should be left up to DBA.