ALTER INDEX

Started by Gavin Sherryalmost 22 years ago12 messagespatches
Jump to latest
#1Gavin Sherry
swm@linuxworld.com.au

Attached is a patch implementing ALTER INDEX <name> [ RENAME | OWNER TO |
SET TABLESPACE ]

I haven't included any regression tests because most of the code should be
well tested by the existing alter table tests.

Thanks,

Gavin

Attachments:

alter_index.difftext/plain; charset=US-ASCII; name=alter_index.diffDownload+269-49
#2Gavin Sherry
swm@linuxworld.com.au
In reply to: Gavin Sherry (#1)
Re: ALTER INDEX

This patch has a fix for a 'thought-o' in the docs.

Gavin

Attachments:

alter_index2.difftext/plain; charset=US-ASCII; name=alter_index2.diffDownload+269-49
#3Stefan Kaltenbrunner
stefan@kaltenbrunner.cc
In reply to: Gavin Sherry (#2)
Re: ALTER INDEX

Gavin Sherry wrote:

Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql-server/src/bin/psql/tab-complete.c,v
retrieving revision 1.109
diff -2 -c -r1.109 tab-complete.c
*** src/bin/psql/tab-complete.c	28 Jul 2004 14:23:30 -0000	1.109
--- src/bin/psql/tab-complete.c	13 Aug 2004 06:34:55 -0000
***************
*** 633,637 ****
{
static const char *const list_ALTER[] =
! 		{"DATABASE", "GROUP", "SCHEMA", "TABLE", "TRIGGER", "USER", NULL};
COMPLETE_WITH_LIST(list_ALTER);
--- 633,638 ----
{
static const char *const list_ALTER[] =
! 		{"DATABASE", "GROUP", "SCHEMA", "TABLE", "TRIGGER", "USER", "INDEX",
! 			 NULL};
COMPLETE_WITH_LIST(list_ALTER);
***************
*** 647,650 ****
--- 648,661 ----
COMPLETE_WITH_LIST(list_ALTERDATABASE);
}
+ 	/* ALTER INDEX <name> */
+ 	else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
+              pg_strcasecmp(prev2_wd, "INDEX") == 0)
+     {
+         static const char *const list_ALTERDATABASE[] =
+         {"SET TABLESPACE", "OWNER TO", "RENAME TO", NULL};
+                                                                                 
+         COMPLETE_WITH_LIST(list_ALTERDATABASE);

minor issue/nit(?): reusing list_ALTERDATABASE for the ALTER INDEX part
looks a little strange ...

Stefan(who could really need some feedback on his own tab-complete patch
*g*)

#4Gavin Sherry
swm@linuxworld.com.au
In reply to: Stefan Kaltenbrunner (#3)
Re: ALTER INDEX

Oops.

Too much with the ol' cut and paste.

I'm happy to send an updated patch but perhaps the committer, assuming the
patch is accepted, would be kind enough to update for me.

Thanks for reviewing.

Gavin

On Fri, 13 Aug 2004, Stefan Kaltenbrunner wrote:

Show quoted text

Gavin Sherry wrote:

Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql-server/src/bin/psql/tab-complete.c,v
retrieving revision 1.109
diff -2 -c -r1.109 tab-complete.c
*** src/bin/psql/tab-complete.c	28 Jul 2004 14:23:30 -0000	1.109
--- src/bin/psql/tab-complete.c	13 Aug 2004 06:34:55 -0000
***************
*** 633,637 ****
{
static const char *const list_ALTER[] =
! 		{"DATABASE", "GROUP", "SCHEMA", "TABLE", "TRIGGER", "USER", NULL};
COMPLETE_WITH_LIST(list_ALTER);
--- 633,638 ----
{
static const char *const list_ALTER[] =
! 		{"DATABASE", "GROUP", "SCHEMA", "TABLE", "TRIGGER", "USER", "INDEX",
! 			 NULL};
COMPLETE_WITH_LIST(list_ALTER);
***************
*** 647,650 ****
--- 648,661 ----
COMPLETE_WITH_LIST(list_ALTERDATABASE);
}
+ 	/* ALTER INDEX <name> */
+ 	else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
+              pg_strcasecmp(prev2_wd, "INDEX") == 0)
+     {
+         static const char *const list_ALTERDATABASE[] =
+         {"SET TABLESPACE", "OWNER TO", "RENAME TO", NULL};
+
+         COMPLETE_WITH_LIST(list_ALTERDATABASE);

minor issue/nit(?): reusing list_ALTERDATABASE for the ALTER INDEX part
looks a little strange ...

Stefan(who could really need some feedback on his own tab-complete patch
*g*)

!DSPAM:411c802d169118747610806!

#5Bruce Momjian
bruce@momjian.us
In reply to: Stefan Kaltenbrunner (#3)
Re: ALTER INDEX

Your adjustment has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------

Stefan Kaltenbrunner wrote:

Gavin Sherry wrote:

Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql-server/src/bin/psql/tab-complete.c,v
retrieving revision 1.109
diff -2 -c -r1.109 tab-complete.c
*** src/bin/psql/tab-complete.c	28 Jul 2004 14:23:30 -0000	1.109
--- src/bin/psql/tab-complete.c	13 Aug 2004 06:34:55 -0000
***************
*** 633,637 ****
{
static const char *const list_ALTER[] =
! 		{"DATABASE", "GROUP", "SCHEMA", "TABLE", "TRIGGER", "USER", NULL};
COMPLETE_WITH_LIST(list_ALTER);
--- 633,638 ----
{
static const char *const list_ALTER[] =
! 		{"DATABASE", "GROUP", "SCHEMA", "TABLE", "TRIGGER", "USER", "INDEX",
! 			 NULL};
COMPLETE_WITH_LIST(list_ALTER);
***************
*** 647,650 ****
--- 648,661 ----
COMPLETE_WITH_LIST(list_ALTERDATABASE);
}
+ 	/* ALTER INDEX <name> */
+ 	else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
+              pg_strcasecmp(prev2_wd, "INDEX") == 0)
+     {
+         static const char *const list_ALTERDATABASE[] =
+         {"SET TABLESPACE", "OWNER TO", "RENAME TO", NULL};
+                                                                                 
+         COMPLETE_WITH_LIST(list_ALTERDATABASE);

minor issue/nit(?): reusing list_ALTERDATABASE for the ALTER INDEX part
looks a little strange ...

Stefan(who could really need some feedback on his own tab-complete patch
*g*)

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#6Bruce Momjian
bruce@momjian.us
In reply to: Gavin Sherry (#2)
Re: ALTER INDEX

Patch applied. Thanks.

I originally thought of this as a feature addition, but I realized that
ALTER INDEX is being added because people are going to want to move
tablespaces for indexes, and without this, they can't easily.

---------------------------------------------------------------------------

Gavin Sherry wrote:

This patch has a fix for a 'thought-o' in the docs.

Gavin

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#7Bruce Momjian
bruce@momjian.us
In reply to: Stefan Kaltenbrunner (#3)
Re: ALTER INDEX

I have made this adjustment.

---------------------------------------------------------------------------

Stefan Kaltenbrunner wrote:

Gavin Sherry wrote:

Index: src/bin/psql/tab-complete.c
===================================================================
RCS file: /usr/local/cvsroot/pgsql-server/src/bin/psql/tab-complete.c,v
retrieving revision 1.109
diff -2 -c -r1.109 tab-complete.c
*** src/bin/psql/tab-complete.c	28 Jul 2004 14:23:30 -0000	1.109
--- src/bin/psql/tab-complete.c	13 Aug 2004 06:34:55 -0000
***************
*** 633,637 ****
{
static const char *const list_ALTER[] =
! 		{"DATABASE", "GROUP", "SCHEMA", "TABLE", "TRIGGER", "USER", NULL};
COMPLETE_WITH_LIST(list_ALTER);
--- 633,638 ----
{
static const char *const list_ALTER[] =
! 		{"DATABASE", "GROUP", "SCHEMA", "TABLE", "TRIGGER", "USER", "INDEX",
! 			 NULL};
COMPLETE_WITH_LIST(list_ALTER);
***************
*** 647,650 ****
--- 648,661 ----
COMPLETE_WITH_LIST(list_ALTERDATABASE);
}
+ 	/* ALTER INDEX <name> */
+ 	else if (pg_strcasecmp(prev3_wd, "ALTER") == 0 &&
+              pg_strcasecmp(prev2_wd, "INDEX") == 0)
+     {
+         static const char *const list_ALTERDATABASE[] =
+         {"SET TABLESPACE", "OWNER TO", "RENAME TO", NULL};
+                                                                                 
+         COMPLETE_WITH_LIST(list_ALTERDATABASE);

minor issue/nit(?): reusing list_ALTERDATABASE for the ALTER INDEX part
looks a little strange ...

Stefan(who could really need some feedback on his own tab-complete patch
*g*)

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#8The Hermit Hacker
scrappy@hub.org
In reply to: Bruce Momjian (#6)
Re: ALTER INDEX

On Fri, 20 Aug 2004, Bruce Momjian wrote:

Patch applied. Thanks.

I originally thought of this as a feature addition, but I realized that
ALTER INDEX is being added because people are going to want to move
tablespaces for indexes, and without this, they can't easily.

Which would fall under adding a feature onto the tablespaces, not fixing a
bug in tablespaces itself ... does *not* having ALTER INDEX *break*
tablespaces? Causes it not to work, or not build?

---------------------------------------------------------------------------

Gavin Sherry wrote:

This patch has a fix for a 'thought-o' in the docs.

Gavin

Content-Description:

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

-- 
Bruce Momjian                        |  http://candle.pha.pa.us
pgman@candle.pha.pa.us               |  (610) 359-1001
+  If your life is a hard drive,     |  13 Roberts Road
+  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

----
Marc G. Fournier Hub.Org Networking Services (http://www.hub.org)
Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664

#9Bruce Momjian
bruce@momjian.us
In reply to: The Hermit Hacker (#8)
Re: ALTER INDEX

Marc G. Fournier wrote:

On Fri, 20 Aug 2004, Bruce Momjian wrote:

Patch applied. Thanks.

I originally thought of this as a feature addition, but I realized that
ALTER INDEX is being added because people are going to want to move
tablespaces for indexes, and without this, they can't easily.

Which would fall under adding a feature onto the tablespaces, not fixing a
bug in tablespaces itself ... does *not* having ALTER INDEX *break*
tablespaces? Causes it not to work, or not build?

No, but it is a missing capability many will complain about. I can
easily remove it. I saw no one comment when I added it to the patches
queue.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#9)
Re: ALTER INDEX

Bruce Momjian <pgman@candle.pha.pa.us> writes:

No, but it is a missing capability many will complain about. I can
easily remove it. I saw no one comment when I added it to the patches
queue.

I hadn't seen you add it to the patches queue ...

I did see Gavin's submission but did not yet have time to look at the
details. What does it *do* exactly --- simply allow INDEX as a
substitute for TABLE in the syntax, or more? I'm not thrilled at the
idea of adding a lot of duplicate coding for this.

regards, tom lane

#11Gavin Sherry
swm@linuxworld.com.au
In reply to: Tom Lane (#10)
Re: ALTER INDEX

On Fri, 20 Aug 2004, Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

No, but it is a missing capability many will complain about. I can
easily remove it. I saw no one comment when I added it to the patches
queue.

I hadn't seen you add it to the patches queue ...

I did see Gavin's submission but did not yet have time to look at the
details. What does it *do* exactly --- simply allow INDEX as a
substitute for TABLE in the syntax, or more? I'm not thrilled at the
idea of adding a lot of duplicate coding for this.

I tried to avoid any duplication. The patch still uses all the ALTER TABLE
code. Its just a grammar modification and some setting of completion tags.

That being said, I felt obliged to provide at patch when I started hearing
noise about ALTER TABLE <index name> being a bit of a hack -- which it is.

Gavin

#12Bruce Momjian
bruce@momjian.us
In reply to: Gavin Sherry (#11)
Re: ALTER INDEX

Gavin Sherry wrote:

On Fri, 20 Aug 2004, Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

No, but it is a missing capability many will complain about. I can
easily remove it. I saw no one comment when I added it to the patches
queue.

I hadn't seen you add it to the patches queue ...

I did see Gavin's submission but did not yet have time to look at the
details. What does it *do* exactly --- simply allow INDEX as a
substitute for TABLE in the syntax, or more? I'm not thrilled at the
idea of adding a lot of duplicate coding for this.

I tried to avoid any duplication. The patch still uses all the ALTER TABLE
code. Its just a grammar modification and some setting of completion tags.

That being said, I felt obliged to provide at patch when I started hearing
noise about ALTER TABLE <index name> being a bit of a hack -- which it is.

The issue was that few people currently modify indexes because in the
past you could only rename an index or change its owner. With
tablespaces, we are going to have lots more people moving indexes
around, and I think people were getting confused because there was no
ALTER INDEX to move them.

I can still back it out and leave it for 8.1 but it probably will reduce
confusion and perhaps need for an FAQ, "How do I move an index between
tablespaces?"

FYI, I just fixed a typo in the ALTER INDEX manual page that mentioned
INDEXSPACE instead of TABLESPACE.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073