Thread: Updated ALTER TABLE ... SET TABLESPACE patch
Hi all, Attached is an updated ALTER TABLE ... SET TABLESPACE patch. It uses the block by block copy mechanism proposed by Tom and handles i) ALTER TABLE <index> .... and ii) Copying of TOAST tables and the TOAST table's index. It doesn't handle copying of system tables (pg_largeobject) and, in the interests of code reuse, the patch fiddles with the code used by CLUSTER. This isn't great but I wanted to get a patch in before 1 July since I think the feature is very important -- even for the first release. Thanks, Gavin
Attachment
On Thu, 1 Jul 2004, Gavin Sherry wrote: > Hi all, > > Attached is an updated ALTER TABLE ... SET TABLESPACE patch. > > It uses the block by block copy mechanism proposed by Tom and handles i) > ALTER TABLE <index> .... and ii) Copying of TOAST tables and the TOAST > table's index. > > It doesn't handle copying of system tables (pg_largeobject) and, in the > interests of code reuse, the patch fiddles with the code used by CLUSTER. > This isn't great but I wanted to get a patch in before 1 July since I > think the feature is very important -- even for the first release. Oh, and it doesn't handle WAL. Tom mentioned/agreed that btree's method of dumping whole pages into WAL would be the best/most efficient way to journaling this and that the btree code should be generalised. I haven't had time to look at this yet. > > Thanks, > > Gavin
On Wed, 2004-06-30 at 22:29, Gavin Sherry wrote: > On Thu, 1 Jul 2004, Gavin Sherry wrote: > Oh, and it doesn't handle WAL. Tom mentioned/agreed that btree's method of > dumping whole pages into WAL would be the best/most efficient way to > journaling this and that the btree code should be generalised. I haven't > had time to look at this yet. > Not sure whether it is really desirable that it does. Could be some damn big tablespaces moved and do we really want all of that in the log? I think we can leave it for now, maybe add a "dont log" option later, and then change the default to logging. Best Regards, Simon Riggs
> Attached is an updated ALTER TABLE ... SET TABLESPACE patch. > > It uses the block by block copy mechanism proposed by Tom and handles i) > ALTER TABLE <index> .... and ii) Copying of TOAST tables and the TOAST > table's index. > > It doesn't handle copying of system tables (pg_largeobject) and, in the > interests of code reuse, the patch fiddles with the code used by CLUSTER. > This isn't great but I wanted to get a patch in before 1 July since I > think the feature is very important -- even for the first release. Does this patch allow setting the tablespace of sequences as well? If so, then you will need to modify pg_dump of SERIAL sequences. Perhaps output a ALTER TABLE/SET TABLESPACE command after the CREATE TABLE definition to move the SERIAL sequence. The same argument applies if it allows moving indexes. (Unique and Primary Keys) Chris
I realize this still need WAL work, but we should get this in.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
Gavin Sherry wrote:
> Hi all,
>
> Attached is an updated ALTER TABLE ... SET TABLESPACE patch.
>
> It uses the block by block copy mechanism proposed by Tom and handles i)
> ALTER TABLE <index> .... and ii) Copying of TOAST tables and the TOAST
> table's index.
>
> It doesn't handle copying of system tables (pg_largeobject) and, in the
> interests of code reuse, the patch fiddles with the code used by CLUSTER.
> This isn't great but I wanted to get a patch in before 1 July since I
> think the feature is very important -- even for the first release.
>
> Thanks,
>
> Gavin
Content-Description:
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.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
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> Attached is an updated ALTER TABLE ... SET TABLESPACE patch.
> Does this patch allow setting the tablespace of sequences as well? If
> so, then you will need to modify pg_dump of SERIAL sequences. Perhaps
> output a ALTER TABLE/SET TABLESPACE command after the CREATE TABLE
> definition to move the SERIAL sequence.
> The same argument applies if it allows moving indexes. (Unique and
> Primary Keys)
Sequences no, toast tables no, indexes yes. So we need the
latter part of the above-mentioned patch. Anyone?
regards, tom lane
Applied by Tom. --------------------------------------------------------------------------- Gavin Sherry wrote: > Hi all, > > Attached is an updated ALTER TABLE ... SET TABLESPACE patch. > > It uses the block by block copy mechanism proposed by Tom and handles i) > ALTER TABLE <index> .... and ii) Copying of TOAST tables and the TOAST > table's index. > > It doesn't handle copying of system tables (pg_largeobject) and, in the > interests of code reuse, the patch fiddles with the code used by CLUSTER. > This isn't great but I wanted to get a patch in before 1 July since I > think the feature is very important -- even for the first release. > > Thanks, > > Gavin Content-Description: [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.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
Tom Lane wrote:
> Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> >> Attached is an updated ALTER TABLE ... SET TABLESPACE patch.
>
> > Does this patch allow setting the tablespace of sequences as well? If
> > so, then you will need to modify pg_dump of SERIAL sequences. Perhaps
> > output a ALTER TABLE/SET TABLESPACE command after the CREATE TABLE
> > definition to move the SERIAL sequence.
> > The same argument applies if it allows moving indexes. (Unique and
> > Primary Keys)
>
> Sequences no, toast tables no, indexes yes. So we need the
> latter part of the above-mentioned patch. Anyone?
Added to TODO;
* Allow moving sequences and toast tables to other tablespaces
in case no one does it.
--
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
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Added to TODO;
> * Allow moving sequences and toast tables to other tablespaces
> in case no one does it.
Please remove that; if I thought either one was a good idea, I would
have allowed it in the committed patch.
Sequences are too small to be worth moving around, and may someday be
reimplemented in a fashion that doesn't use up a separate disk file for
each one. If we allow SET TABLESPACE on them we will be limiting our
future flexibility for no useful gain.
Toast tables are bound to their parent tables because (a) pg_dump isn't
nearly smart enough to handle moving them, and (b) I've got concerns
about how you decide whether a person is authorized to move one.
regards, tom lane
OK, removed. --------------------------------------------------------------------------- Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Added to TODO; > > * Allow moving sequences and toast tables to other tablespaces > > in case no one does it. > > Please remove that; if I thought either one was a good idea, I would > have allowed it in the committed patch. > > Sequences are too small to be worth moving around, and may someday be > reimplemented in a fashion that doesn't use up a separate disk file for > each one. If we allow SET TABLESPACE on them we will be limiting our > future flexibility for no useful gain. > > Toast tables are bound to their parent tables because (a) pg_dump isn't > nearly smart enough to handle moving them, and (b) I've got concerns > about how you decide whether a person is authorized to move one. > > regards, tom lane > -- 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
> Please remove that; if I thought either one was a good idea, I would > have allowed it in the committed patch. > > Sequences are too small to be worth moving around, and may someday be > reimplemented in a fashion that doesn't use up a separate disk file for > each one. If we allow SET TABLESPACE on them we will be limiting our > future flexibility for no useful gain. Why do we allow them to be created in tablespaces in the first place then? Seems like a bit of a misfeature? I mean we don't allow views in tablespaces... Chris
>>Does this patch allow setting the tablespace of sequences as well? If >>so, then you will need to modify pg_dump of SERIAL sequences. Perhaps >>output a ALTER TABLE/SET TABLESPACE command after the CREATE TABLE >>definition to move the SERIAL sequence. >>The same argument applies if it allows moving indexes. (Unique and >>Primary Keys) > > Sequences no, toast tables no, indexes yes. So we need the > latter part of the above-mentioned patch. Anyone? I'll do it. Chris
Christopher Kings-Lynne wrote: > > Please remove that; if I thought either one was a good idea, I would > > have allowed it in the committed patch. > > > > Sequences are too small to be worth moving around, and may someday be > > reimplemented in a fashion that doesn't use up a separate disk file for > > each one. If we allow SET TABLESPACE on them we will be limiting our > > future flexibility for no useful gain. > > Why do we allow them to be created in tablespaces in the first place > then? Seems like a bit of a misfeature? I mean we don't allow views in > tablespaces... True, that does seem unusual. -- 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
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> Sequences are too small to be worth moving around, and may someday be
>> reimplemented in a fashion that doesn't use up a separate disk file for
>> each one. If we allow SET TABLESPACE on them we will be limiting our
>> future flexibility for no useful gain.
> Why do we allow them to be created in tablespaces in the first place
> then? Seems like a bit of a misfeature? I mean we don't allow views in
> tablespaces...
I had forgotten that the original patch allowed that. Personally I'd
vote for taking it out, for the above-stated reasons --- any objections?
If people do want to have it then we can instead change ALTER SET
TABLESPACE to allow sequences; but we'd also need a nontrivial addition
to pg_dump, so there had better be a better reason than "might be nice
to have".
regards, tom lane
Will toast go in the same tablespace as the base table? I can see some advantages to splitting that to another drive for extreme cases (think heap/toast lookups over and over again). --------------------------------------------------------------------------- Tom Lane wrote: > Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes: > >> Sequences are too small to be worth moving around, and may someday be > >> reimplemented in a fashion that doesn't use up a separate disk file for > >> each one. If we allow SET TABLESPACE on them we will be limiting our > >> future flexibility for no useful gain. > > > Why do we allow them to be created in tablespaces in the first place > > then? Seems like a bit of a misfeature? I mean we don't allow views in > > tablespaces... > > I had forgotten that the original patch allowed that. Personally I'd > vote for taking it out, for the above-stated reasons --- any objections? > > If people do want to have it then we can instead change ALTER SET > TABLESPACE to allow sequences; but we'd also need a nontrivial addition > to pg_dump, so there had better be a better reason than "might be nice > to have". > > regards, tom lane > > ---------------------------(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
However, based on our feature freeze, maybe what we have now is fine and I can just add the TODO item again. --------------------------------------------------------------------------- Tom Lane wrote: > Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes: > >> Sequences are too small to be worth moving around, and may someday be > >> reimplemented in a fashion that doesn't use up a separate disk file for > >> each one. If we allow SET TABLESPACE on them we will be limiting our > >> future flexibility for no useful gain. > > > Why do we allow them to be created in tablespaces in the first place > > then? Seems like a bit of a misfeature? I mean we don't allow views in > > tablespaces... > > I had forgotten that the original patch allowed that. Personally I'd > vote for taking it out, for the above-stated reasons --- any objections? > > If people do want to have it then we can instead change ALTER SET > TABLESPACE to allow sequences; but we'd also need a nontrivial addition > to pg_dump, so there had better be a better reason than "might be nice > to have". > > regards, tom lane > -- 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
> I had forgotten that the original patch allowed that. Personally I'd > vote for taking it out, for the above-stated reasons --- any objections? I vote for taking it out. Chris
On Mon, 12 Jul 2004, Christopher Kings-Lynne wrote: > > I had forgotten that the original patch allowed that. Personally I'd > > vote for taking it out, for the above-stated reasons --- any objections? > > I vote for taking it out. Pull it. I added it intentionally but now I'm questioning my reasoning (which was, sequences might be accessed lots, people might want to put them somewhere intentionally). > > Chris > > > !DSPAM:40f1f2de259842510072165! > >
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Will toast go in the same tablespace as the base table?
That is the current design, and I'd prefer to keep it that way because
anything else adds great complexity for unclear gain.
Two examples of pain points:
1. That pending patch to report a table's tablespace in psql \d would
get lots more complex: you'd have up to three tablespaces to worry about
(base table, toast table, toast index).
2. How would pg_dump restore such a setup? It could not generate a
script that says ALTER TABLE pg_toast.pg_toast_NNN SET TABLESPACE,
because it has no way to know what NNN should be. I think we'd have
to add locutions like "ALTER TABLE foo SET TOAST TABLESPACE t" and
"ALTER TABLE foo SET TOAST INDEX TABLESPACE t" to do this in a clean
fashion.
In fact, now that I think of it, the patch-as-committed already
introduces some serious headaches for pg_dump: it can't know for sure
what name will be assigned to constraint indexes (pkey and unique
indexes) so it has no good way to emit ALTER TABLE SET TABLESPACE
commands for those indexes.
regards, tom lane
Gavin Sherry <swm@linuxworld.com.au> writes:
> On Mon, 12 Jul 2004, Christopher Kings-Lynne wrote:
>> I vote for taking it out.
> Pull it. I added it intentionally but now I'm questioning my reasoning
> (which was, sequences might be accessed lots, people might want to put
> them somewhere intentionally).
I thought about that, but AFAICS a heavily used sequence would merely
end up as a single hot entry in the shared buffer arena (or at worst,
an often-hit page in kernel buffers). It could not result in a lot of
actual I/O because it's only one page; at most one would expect one
write per checkpoint cycle. So putting it on particularly fast disk
would be a useless exercise.
regards, tom lane
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> Otherwise, we need to extend the ADD CONSTRAINT syntax.
Yeah, I was wondering if there was some minimal-impact way to do that.
regards, tom lane
> In fact, now that I think of it, the patch-as-committed already > introduces some serious headaches for pg_dump: it can't know for sure > what name will be assigned to constraint indexes (pkey and unique > indexes) so it has no good way to emit ALTER TABLE SET TABLESPACE > commands for those indexes. I guess I'll have to make it punt that the name will be what it currently is :( Otherwise, we need to extend the ADD CONSTRAINT syntax. That would be handy because the you could specify the TABLESPACE at creation time as well. Chris
>>Otherwise, we need to extend the ADD CONSTRAINT syntax. > > Yeah, I was wondering if there was some minimal-impact way to do that. Shall I hold off on doing any pg_dump changes then? Chris
>>Otherwise, we need to extend the ADD CONSTRAINT syntax. > > > Yeah, I was wondering if there was some minimal-impact way to do that. Oh, or we create ALTER CONSTRAINT :) Chris