Request for Comments: ALTER [OBJECT] SET SCHEMA

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

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.

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

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...

--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