Thread: [patch] ALTER RENAME and indexes

[patch] ALTER RENAME and indexes

From
Brent Verner
Date:
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
nulldefault 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

Re: [patch] ALTER RENAME and indexes

From
Tom Lane
Date:
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 nullf2     | 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


Re: [patch] ALTER RENAME and indexes

From
Brent Verner
Date:
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


Re: [patch] ALTER RENAME and indexes

From
Tom Lane
Date:
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


Re: [patch] ALTER RENAME and indexes

From
Brent Verner
Date:
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


Re: [patch] ALTER RENAME and indexes

From
Tom Lane
Date:
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