Thread: pgsql-server: Add ALTER INDEX, particularly for moving tablespaces.
pgsql-server: Add ALTER INDEX, particularly for moving tablespaces.
From
momjian@svr1.postgresql.org (Bruce Momjian)
Date:
Log Message:
-----------
Add ALTER INDEX, particularly for moving tablespaces.
Gavin Sherry
Modified Files:
--------------
pgsql-server/src/backend/parser:
gram.y (r2.471 -> r2.472)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/parser/gram.y.diff?r1=2.471&r2=2.472)
pgsql-server/src/backend/tcop:
utility.c (r1.225 -> r1.226)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/backend/tcop/utility.c.diff?r1=1.225&r2=1.226)
pgsql-server/src/bin/psql:
tab-complete.c (r1.109 -> r1.110)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/bin/psql/tab-complete.c.diff?r1=1.109&r2=1.110)
pgsql-server/src/include/nodes:
parsenodes.h (r1.266 -> r1.267)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/src/include/nodes/parsenodes.h.diff?r1=1.266&r2=1.267)
Added Files:
-----------
pgsql-server/doc/src/sgml/ref:
alter_index.sgml (r1.1)
(http://developer.postgresql.org/cvsweb.cgi/pgsql-server/doc/src/sgml/ref/alter_index.sgml?rev=1.1&content-type=text/x-cvsweb-markup)
momjian@svr1.postgresql.org (Bruce Momjian) writes:
> Add ALTER INDEX, particularly for moving tablespaces.
This patch is a perfect example of why unreviewed patches should not
go in during beta.
So far I have noticed the following problems with it:
* Added reference page wasn't linked into the docs build.
* Added a field to struct AlterTableStmt, but did not do the necessary
housekeeping for extending a Node (eg, copyfuncs and equalfuncs
adjustments), nor make sure the field is validly set in every place
an AlterTableStmt is constructed.
* ALTER INDEX RENAME doesn't actually work.
regression=# alter index foo_pkey rename to zzz;
ERROR: unrecognized rename stmt type: 9
That's not counting the problem someone else already reported with
incorrect tab-completion.
regards, tom lane
Tom Lane wrote: > momjian@svr1.postgresql.org (Bruce Momjian) writes: > > Add ALTER INDEX, particularly for moving tablespaces. > > This patch is a perfect example of why unreviewed patches should not > go in during beta. > > So far I have noticed the following problems with it: > > * Added reference page wasn't linked into the docs build. > > * Added a field to struct AlterTableStmt, but did not do the necessary > housekeeping for extending a Node (eg, copyfuncs and equalfuncs > adjustments), nor make sure the field is validly set in every place > an AlterTableStmt is constructed. > > * ALTER INDEX RENAME doesn't actually work. > > regression=# alter index foo_pkey rename to zzz; > ERROR: unrecognized rename stmt type: 9 > > That's not counting the problem someone else already reported with > incorrect tab-completion. Yes, you are right. Because Gavin is "Mr Tablespaces" I didn't give it a thorough read. FYI, I just fixed the tab completion problem. -- 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
On Sat, 21 Aug 2004, Bruce Momjian wrote: > Tom Lane wrote: >> momjian@svr1.postgresql.org (Bruce Momjian) writes: >>> Add ALTER INDEX, particularly for moving tablespaces. >> >> This patch is a perfect example of why unreviewed patches should not >> go in during beta. >> >> So far I have noticed the following problems with it: >> >> * Added reference page wasn't linked into the docs build. >> >> * Added a field to struct AlterTableStmt, but did not do the necessary >> housekeeping for extending a Node (eg, copyfuncs and equalfuncs >> adjustments), nor make sure the field is validly set in every place >> an AlterTableStmt is constructed. >> >> * ALTER INDEX RENAME doesn't actually work. >> >> regression=# alter index foo_pkey rename to zzz; >> ERROR: unrecognized rename stmt type: 9 >> >> That's not counting the problem someone else already reported with >> incorrect tab-completion. > > Yes, you are right. Because Gavin is "Mr Tablespaces" I didn't give it > a thorough read. > > FYI, I just fixed the tab completion problem. Considering point 3 seems a wee bit critical, at least to me, should the patch be removed and postponed for a non-Beta period? ---- Marc G. Fournier Hub.Org Networking Services (http://www.hub.org) Email: scrappy@hub.org Yahoo!: yscrappy ICQ: 7615664
"Marc G. Fournier" <scrappy@postgresql.org> writes:
> Considering point 3 seems a wee bit critical, at least to me, should the
> patch be removed and postponed for a non-Beta period?
No, the fixes are all trivial. I'm just chewing out Bruce and Gavin
for sloppiness ;-)
regards, tom lane
Umm... oops. On Sat, 21 Aug 2004, Tom Lane wrote: > momjian@svr1.postgresql.org (Bruce Momjian) writes: > > Add ALTER INDEX, particularly for moving tablespaces. > > This patch is a perfect example of why unreviewed patches should not > go in during beta. > > So far I have noticed the following problems with it: > > * Added reference page wasn't linked into the docs build. > > * Added a field to struct AlterTableStmt, but did not do the necessary > housekeeping for extending a Node (eg, copyfuncs and equalfuncs > adjustments), nor make sure the field is validly set in every place > an AlterTableStmt is constructed. > > * ALTER INDEX RENAME doesn't actually work. > > regression=# alter index foo_pkey rename to zzz; > ERROR: unrecognized rename stmt type: 9 Thanks for spotting this sloppy work. Basically, I cropped my diff because there was lots of unrelated code in the work space. But I was a little too hard and culled allfiles.sgml, alter.c, equal/copyfuncs.c and analyze.c. BUT I should have tested the patch on a fresh checkout, especially since someone else noticed the tab-completition naming thing. Sorry. I was getting together a patch against HEAD but I just noticed that you fixed this. Thanks. Gavin
Gavin Sherry <swm@linuxworld.com.au> writes:
> Thanks for spotting this sloppy work. Basically, I cropped my diff because
> there was lots of unrelated code in the work space. But I was a little too
> hard and culled allfiles.sgml, alter.c, equal/copyfuncs.c and analyze.c.
I was wondering if something like that might have happened --- I know
you know better on all these points ;-)
> I was getting together a patch against HEAD but I just noticed that you
> fixed this. Thanks.
Yeah, I think everything is fixed in CVS tip. Let me know if there was
anything I missed.
regards, tom lane
Here is a change to the alter_table regression test to use ALTER INDEX ... RENAME instead of ALTER TABLE ... RENAME in the case of an index renaming. Its probably not the right place for it, but I'm not sure where is the right place for it. Thanks, Gavin On Sat, 21 Aug 2004, Tom Lane wrote: > Gavin Sherry <swm@linuxworld.com.au> writes: > > Thanks for spotting this sloppy work. Basically, I cropped my diff because > > there was lots of unrelated code in the work space. But I was a little too > > hard and culled allfiles.sgml, alter.c, equal/copyfuncs.c and analyze.c. > > I was wondering if something like that might have happened --- I know > you know better on all these points ;-) > > > I was getting together a patch against HEAD but I just noticed that you > > fixed this. Thanks. > > Yeah, I think everything is fixed in CVS tip. Let me know if there was > anything I missed. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > > > !DSPAM:4127e64e43221930617052! > >
Attachment
>>Thanks for spotting this sloppy work. Basically, I cropped my diff because >>there was lots of unrelated code in the work space. But I was a little too >>hard and culled allfiles.sgml, alter.c, equal/copyfuncs.c and analyze.c. > > > I was wondering if something like that might have happened --- I know > you know better on all these points ;-) I have a whole separate CVS checkout for every path I'm working on - makes life a lot easier :) Not that Tom doesn't still find bugs in my stuff :P Chris
> I have a whole separate CVS checkout for every path I'm working on - > makes life a lot easier :) Dammit - I meant 'patch'. Chris
I think I missed this in the earlier patches, but does the alter table syntax
still work for indexes in addition to the new syntax?
On Saturday 21 August 2004 22:47, Gavin Sherry wrote:
> Here is a change to the alter_table regression test to use ALTER INDEX ...
> RENAME instead of ALTER TABLE ... RENAME in the case of an index renaming.
> Its probably not the right place for it, but I'm not sure where is the
> right place for it.
>
> Thanks,
>
> Gavin
>
> On Sat, 21 Aug 2004, Tom Lane wrote:
> > Gavin Sherry <swm@linuxworld.com.au> writes:
> > > Thanks for spotting this sloppy work. Basically, I cropped my diff
> > > because there was lots of unrelated code in the work space. But I was a
> > > little too hard and culled allfiles.sgml, alter.c, equal/copyfuncs.c
> > > and analyze.c.
> >
> > I was wondering if something like that might have happened --- I know
> > you know better on all these points ;-)
> >
> > > I was getting together a patch against HEAD but I just noticed that you
> > > fixed this. Thanks.
> >
> > Yeah, I think everything is fixed in CVS tip. Let me know if there was
> > anything I missed.
> >
> > regards, tom lane
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> >
> >
> > !DSPAM:4127e64e43221930617052!
--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL
Yes
On Tue, 24 Aug 2004, Robert Treat wrote:
> I think I missed this in the earlier patches, but does the alter table syntax
> still work for indexes in addition to the new syntax?
>
> On Saturday 21 August 2004 22:47, Gavin Sherry wrote:
> > Here is a change to the alter_table regression test to use ALTER INDEX ...
> > RENAME instead of ALTER TABLE ... RENAME in the case of an index renaming.
> > Its probably not the right place for it, but I'm not sure where is the
> > right place for it.
> >
> > Thanks,
> >
> > Gavin
> >
> > On Sat, 21 Aug 2004, Tom Lane wrote:
> > > Gavin Sherry <swm@linuxworld.com.au> writes:
> > > > Thanks for spotting this sloppy work. Basically, I cropped my diff
> > > > because there was lots of unrelated code in the work space. But I was a
> > > > little too hard and culled allfiles.sgml, alter.c, equal/copyfuncs.c
> > > > and analyze.c.
> > >
> > > I was wondering if something like that might have happened --- I know
> > > you know better on all these points ;-)
> > >
> > > > I was getting together a patch against HEAD but I just noticed that you
> > > > fixed this. Thanks.
> > >
> > > Yeah, I think everything is fixed in CVS tip. Let me know if there was
> > > anything I missed.
> > >
> > > regards, tom lane
> > >
> > > ---------------------------(end of broadcast)---------------------------
> > > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> > >
> > >
> > > !DSPAM:4127e64e43221930617052!
>
> --
> Robert Treat
> Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL
>
Shouldn't the regression tests reflect that?
Robert Treat
On Tue, 2004-08-24 at 22:31, Gavin Sherry wrote:
> Yes
>
> On Tue, 24 Aug 2004, Robert Treat wrote:
>
> > I think I missed this in the earlier patches, but does the alter table syntax
> > still work for indexes in addition to the new syntax?
> >
> > On Saturday 21 August 2004 22:47, Gavin Sherry wrote:
> > > Here is a change to the alter_table regression test to use ALTER INDEX ...
> > > RENAME instead of ALTER TABLE ... RENAME in the case of an index renaming.
> > > Its probably not the right place for it, but I'm not sure where is the
> > > right place for it.
> > >
> > > Thanks,
> > >
> > > Gavin
> > >
> > > On Sat, 21 Aug 2004, Tom Lane wrote:
> > > > Gavin Sherry <swm@linuxworld.com.au> writes:
> > > > > Thanks for spotting this sloppy work. Basically, I cropped my diff
> > > > > because there was lots of unrelated code in the work space. But I was a
> > > > > little too hard and culled allfiles.sgml, alter.c, equal/copyfuncs.c
> > > > > and analyze.c.
> > > >
> > > > I was wondering if something like that might have happened --- I know
> > > > you know better on all these points ;-)
> > > >
> > > > > I was getting together a patch against HEAD but I just noticed that you
> > > > > fixed this. Thanks.
> > > >
> > > > Yeah, I think everything is fixed in CVS tip. Let me know if there was
> > > > anything I missed.
> > > >
> > > > regards, tom lane
> > > >
> > > > ---------------------------(end of broadcast)---------------------------
> > > > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org
> > > >
> > > >
> > > > !DSPAM:4127e64e43221930617052!
> >
> > --
> > Robert Treat
> > Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL
> >
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly
--
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL
Patch applied. Thanks. --------------------------------------------------------------------------- Gavin Sherry wrote: > Here is a change to the alter_table regression test to use ALTER INDEX ... > RENAME instead of ALTER TABLE ... RENAME in the case of an index renaming. > Its probably not the right place for it, but I'm not sure where is the > right place for it. > > Thanks, > > Gavin > > On Sat, 21 Aug 2004, Tom Lane wrote: > > > Gavin Sherry <swm@linuxworld.com.au> writes: > > > Thanks for spotting this sloppy work. Basically, I cropped my diff because > > > there was lots of unrelated code in the work space. But I was a little too > > > hard and culled allfiles.sgml, alter.c, equal/copyfuncs.c and analyze.c. > > > > I was wondering if something like that might have happened --- I know > > you know better on all these points ;-) > > > > > I was getting together a patch against HEAD but I just noticed that you > > > fixed this. Thanks. > > > > Yeah, I think everything is fixed in CVS tip. Let me know if there was > > anything I missed. > > > > regards, tom lane > > > > ---------------------------(end of broadcast)--------------------------- > > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > > > > > > !DSPAM:4127e64e43221930617052! > > > > Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 8: explain analyze is your friend -- 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