[patch] ALTER RENAME and indexes
The attached patch works for my case...
regression=# create table test (id serial, col1 varchar(64));
NOTICE: CREATE TABLE will create implicit sequence 'test_id_seq' for SERIAL column 'test.id'
NOTICE: CREATE TABLE/UNIQUE will create implicit index 'test_id_key' for table 'test'
CREATE
regression=# create index i_t_c on test(col1);
CREATE
regression=# alter table test rename col1 to col2;
ALTER
regression=# \d test
Table "test"
Column | Type | Modifiers
--------+-----------------------+-------------------------------------------------
id | integer | not null default nextval('"test_id_seq"'::text)
col2 | character varying(64) |
Indexes: i_t_c
Unique keys: test_id_key
regression=# \d itc
Did not find any relation named "itc".
regression=# \d i_t_c
Index "i_t_c"
Column | Type
--------+-----------------------
col2 | character varying(64)
btree
wooohoo!!! Of course, it would be best if someone else looked this
code over, because I get the feeling there is an easier way to get
this done. The only thing I can say is that it solves my problem
and does not /appear/ to create any others.
cheers.
Brent
p.s., I appreciate the gurus not jumping in with the quick fix --
The experience has been fun :-)
--
"Develop your talent, man, and leave the world something. Records are
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing." -- Duane Allman
Attachments:
pgsql.200110062153.difftext/plain; charset=us-asciiDownload
Index: src/backend/commands/rename.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/rename.c,v
retrieving revision 1.57
diff -r1.57 rename.c
35a36,37
> static void
> update_indexed_attnames(Oid indrelid, const char *_old, const char *_new);
169a172,235
>
> update_indexed_attnames(relid,oldattname,newattname);
>
> }
>
> /* man, I want some sql here...
> *
> * UPDATE pg_attribute
> * SET attname='$3'
> * WHERE indrelid=$1 AND attname='$2'
> */
> static void
> update_indexed_attnames(Oid indrelid,
> const char *oldattname,
> const char *newattname)
> {
> List *indexoidlist;
> List *indexoidscan;
> Relation relation;
> Relation attrelation;
>
> relation = heap_open(indrelid, AccessShareLock);
> indexoidlist = RelationGetIndexList(relation);
>
> attrelation = heap_openr(AttributeRelationName, RowExclusiveLock);
>
> foreach(indexoidscan, indexoidlist)
> {
> HeapTuple atttup;
> Oid indexoid;
> indexoid = lfirsti(indexoidscan);
> elog(DEBUG,"update_indexed_attnames: indexoid: %d", indexoid);
> atttup = SearchSysCacheCopy(ATTNAME,
> ObjectIdGetDatum(indexoid),
> PointerGetDatum(oldattname),
> 0, 0);
> if (!HeapTupleIsValid(atttup)){
> elog(DEBUG, "update_indexed_attnames: not updating %d.", indexoid);
> }
> else{
> elog(DEBUG, "update_indexed_attnames: updating %d.", indexoid);
>
>
> StrNCpy(NameStr(((Form_pg_attribute) GETSTRUCT(atttup))->attname),
> newattname, NAMEDATALEN);
>
> simple_heap_update(attrelation, &atttup->t_self, atttup);
>
> /* keep system catalog indices current */
> {
> Relation irelations[Num_pg_attr_indices];
>
> CatalogOpenIndices(Num_pg_attr_indices, Name_pg_attr_indices, irelations);
> CatalogIndexInsert(irelations, Num_pg_attr_indices, attrelation, atttup);
> CatalogCloseIndices(Num_pg_attr_indices, irelations);
> }
> heap_freetuple(atttup);
> }
> }
>
> freeList(indexoidlist);
>
> heap_close(attrelation, RowExclusiveLock);
> heap_close(relation, AccessShareLock);
It occurs to me that the real problem is not so much ALTER RENAME not
doing enough, as it is psql doing the wrong thing. The \d display for
indexes is almost entirely unhelpful, since it doesn't tell you such
critical stuff as whether the index is a functional index nor which
index opclasses are being used. I wonder whether we oughtn't rip out
the whole display and make it report the results of pg_get_indexdef(),
instead.
regression=# create table foo(f1 int,f2 int, primary key(f1,f2));
NOTICE: CREATE TABLE/PRIMARY KEY will create implicit index 'foo_pkey' for table 'foo'
CREATE
regression=# \d foo
Table "foo"
Column | Type | Modifiers
--------+---------+-----------
f1 | integer | not null
f2 | integer | not null
Primary key: foo_pkey
regression=# create index foofn on foo (int4pl(f1,f2));
CREATE
regression=# \d foofn
Index "foofn"
Column | Type
--------+---------
int4pl | integer
btree
regression=# select pg_get_indexdef(oid) from pg_class where relname = 'foofn';
pg_get_indexdef
--------------------------------------------------------
CREATE INDEX foofn ON foo USING btree (int4pl(f1, f2))
(1 row)
regression=# alter table foo rename f1 to f1new;
ALTER
regression=# select pg_get_indexdef(oid) from pg_class where relname = 'foofn';
pg_get_indexdef
-----------------------------------------------------------
CREATE INDEX foofn ON foo USING btree (int4pl(f1new, f2))
(1 row)
regards, tom lane
On 07 Oct 2001 at 10:56 (-0400), Tom Lane wrote:
| It occurs to me that the real problem is not so much ALTER RENAME not
| doing enough, as it is psql doing the wrong thing. The \d display for
| indexes is almost entirely unhelpful, since it doesn't tell you such
| critical stuff as whether the index is a functional index nor which
| index opclasses are being used. I wonder whether we oughtn't rip out
| the whole display and make it report the results of pg_get_indexdef(),
| instead.
This would solve the display problem for sure, but we'd still have
bad data in the pg_attribute tuple for the index -- specifically,
attname would still contain the original column name that the index
was created on. I'm now aware that PG does not use this attname
directly/internally, but it would still be wrong if anyone happens
to look at the system catalog.
cheers.
Brent
--
"Develop your talent, man, and leave the world something. Records are
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing." -- Duane Allman
Brent Verner <brent@rcfile.org> writes:
wooohoo!!! Of course, it would be best if someone else looked this
code over, because I get the feeling there is an easier way to get
this done.
No, that's about right, except that you forgot one step: you shouldn't
try to update column names of functional indexes, since their columns
are named after the function not the column. If the function name
happened to match the column name then you'd have applied an erroneous
renaming.
Fixed and applied to CVS.
regards, tom lane
On 08 Oct 2001 at 14:43 (-0400), Tom Lane wrote:
| Brent Verner <brent@rcfile.org> writes:
| > wooohoo!!! Of course, it would be best if someone else looked this
| > code over, because I get the feeling there is an easier way to get
| > this done.
|
| No, that's about right, except that you forgot one step: you shouldn't
| try to update column names of functional indexes, since their columns
| are named after the function not the column. If the function name
| happened to match the column name then you'd have applied an erroneous
| renaming.
|
| Fixed and applied to CVS.
Thanks for fixing that problem. Out of curiousity, did you put that
code in the renameatt() function for any reason, or is it just a
style thing? (I ask in hopes of getting closer to submitting
code/patches that you'd not have to rewrite to apply ;-) )
Thanks.
Brent
--
"Develop your talent, man, and leave the world something. Records are
really gifts from people. To think that an artist would love you enough
to share his music with anyone is a beautiful thing." -- Duane Allman
Brent Verner <brent@rcfile.org> writes:
Thanks for fixing that problem. Out of curiousity, did you put that
code in the renameatt() function for any reason, or is it just a
style thing?
Just to avoid closing and reopening the target relation and pg_attribute
relation. Releasing and then reacquiring the lock on pg_attribute
seemed like a not-so-good idea, although it probably shouldn't make any
difference.
regards, tom lane