Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-08-12 Thread Simon Riggs
On 3 August 2015 at 01:35, Andreas Karlsson andr...@proxel.se wrote:

 On 07/15/2015 09:30 PM, Robert Haas wrote:

 On Tue, Jul 14, 2015 at 5:57 PM, Jim Nasby jim.na...@bluetreble.com
 wrote:

 On 7/7/15 7:07 AM, Andres Freund wrote:

 On 2015-07-03 18:03:58 -0400, Tom Lane wrote:

 I have just looked through this thread, and TBH I think we should
 reject
 this patch altogether --- not RWF, but no we don't want this.  The
 use-case remains hypothetical: no performance numbers showing a
 real-world
 benefit have been exhibited AFAICS.



 It's not that hard to imagine a performance benefit though? If the
 toasted column is accessed infrequently/just after filtering on other
 columns (not uncommon) it could very well be beneficial to put the main
 table on fast storage (SSD) and the toast table on slow storage
 (spinning rust).

 I've seen humoungous toast tables that are barely ever read for tables
 that are constantly a couple times in the field.


 +1. I know of one case where the heap was ~8GB and TOAST was over 400GB.


 Yeah, I think that's a good argument for this.  I have to admit that
 I'm a bit fatigued by this thread - it started out as a learn
 PostgreSQL project, and we iterated through a few design problems
 that made me kind of worried about what sort of state the patch was in
 - and now this patch is more than 4 years old.  But if some committer
 still has the energy to go through it in detail and make sure that all
 of the problems have been fixed and that the patch is, as Andreas
 says, in good shape, then I don't see why we shouldn't take it.


 I personally think the patch is in a decent shape, and a worthwhile
 feature. I agree though with Tom's objections about the pg_dump code.

 I do not have enough time or interest right now to fix up this patch (this
 is not a feature I personally have a lot of interest in), but if someone
 else wishes to I do not think it would be too much work.


To me the patch looks like it is in reasonable shape, caveats noted.

The patch doesn't really take us very far forwards in terms of
administration since workarounds exist which people use and understand.
Noting Tom's comments on additional complexity it seems like it could be a
source of bugs or production problems.

The deciding factor is that it brings TOAST as a keyword for us to support
forever. While reviewing this, I've come to realise that a better approach
to column stores and/or vertical partitioning is the more general way of
handling this, so creating a support legacy at this time doesn't make sense
to me personally.

On balance, the negatives seem to outweigh the positives so isn't the best
use of my time to fix it up.

I'm now returning this with feedback.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
http://www.2ndquadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-08-02 Thread Andreas Karlsson

On 07/15/2015 09:30 PM, Robert Haas wrote:

On Tue, Jul 14, 2015 at 5:57 PM, Jim Nasby jim.na...@bluetreble.com wrote:

On 7/7/15 7:07 AM, Andres Freund wrote:

On 2015-07-03 18:03:58 -0400, Tom Lane wrote:

I have just looked through this thread, and TBH I think we should reject
this patch altogether --- not RWF, but no we don't want this.  The
use-case remains hypothetical: no performance numbers showing a
real-world
benefit have been exhibited AFAICS.



It's not that hard to imagine a performance benefit though? If the
toasted column is accessed infrequently/just after filtering on other
columns (not uncommon) it could very well be beneficial to put the main
table on fast storage (SSD) and the toast table on slow storage
(spinning rust).

I've seen humoungous toast tables that are barely ever read for tables
that are constantly a couple times in the field.


+1. I know of one case where the heap was ~8GB and TOAST was over 400GB.


Yeah, I think that's a good argument for this.  I have to admit that
I'm a bit fatigued by this thread - it started out as a learn
PostgreSQL project, and we iterated through a few design problems
that made me kind of worried about what sort of state the patch was in
- and now this patch is more than 4 years old.  But if some committer
still has the energy to go through it in detail and make sure that all
of the problems have been fixed and that the patch is, as Andreas
says, in good shape, then I don't see why we shouldn't take it.


I personally think the patch is in a decent shape, and a worthwhile 
feature. I agree though with Tom's objections about the pg_dump code.


I do not have enough time or interest right now to fix up this patch 
(this is not a feature I personally have a lot of interest in), but if 
someone else wishes to I do not think it would be too much work.


Andreas



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-07-15 Thread Robert Haas
On Tue, Jul 14, 2015 at 5:57 PM, Jim Nasby jim.na...@bluetreble.com wrote:
 On 7/7/15 7:07 AM, Andres Freund wrote:
 On 2015-07-03 18:03:58 -0400, Tom Lane wrote:
 I have just looked through this thread, and TBH I think we should reject
 this patch altogether --- not RWF, but no we don't want this.  The
 use-case remains hypothetical: no performance numbers showing a
 real-world
 benefit have been exhibited AFAICS.


 It's not that hard to imagine a performance benefit though? If the
 toasted column is accessed infrequently/just after filtering on other
 columns (not uncommon) it could very well be beneficial to put the main
 table on fast storage (SSD) and the toast table on slow storage
 (spinning rust).

 I've seen humoungous toast tables that are barely ever read for tables
 that are constantly a couple times in the field.

 +1. I know of one case where the heap was ~8GB and TOAST was over 400GB.

Yeah, I think that's a good argument for this.  I have to admit that
I'm a bit fatigued by this thread - it started out as a learn
PostgreSQL project, and we iterated through a few design problems
that made me kind of worried about what sort of state the patch was in
- and now this patch is more than 4 years old.  But if some committer
still has the energy to go through it in detail and make sure that all
of the problems have been fixed and that the patch is, as Andreas
says, in good shape, then I don't see why we shouldn't take it.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-07-14 Thread Jim Nasby

On 7/7/15 7:07 AM, Andres Freund wrote:

On 2015-07-03 18:03:58 -0400, Tom Lane wrote:

I have just looked through this thread, and TBH I think we should reject
this patch altogether --- not RWF, but no we don't want this.  The
use-case remains hypothetical: no performance numbers showing a real-world
benefit have been exhibited AFAICS.


It's not that hard to imagine a performance benefit though? If the
toasted column is accessed infrequently/just after filtering on other
columns (not uncommon) it could very well be beneficial to put the main
table on fast storage (SSD) and the toast table on slow storage
(spinning rust).

I've seen humoungous toast tables that are barely ever read for tables
that are constantly a couple times in the field.


+1. I know of one case where the heap was ~8GB and TOAST was over 400GB.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-07-07 Thread Andres Freund
On 2015-07-03 18:03:58 -0400, Tom Lane wrote:
 I have just looked through this thread, and TBH I think we should reject
 this patch altogether --- not RWF, but no we don't want this.  The
 use-case remains hypothetical: no performance numbers showing a real-world
 benefit have been exhibited AFAICS.

It's not that hard to imagine a performance benefit though? If the
toasted column is accessed infrequently/just after filtering on other
columns (not uncommon) it could very well be beneficial to put the main
table on fast storage (SSD) and the toast table on slow storage
(spinning rust).

I've seen humoungous toast tables that are barely ever read for tables
that are constantly a couple times in the field.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-07-03 Thread Tom Lane
Andreas Karlsson andr...@proxel.se writes:
 On 03/21/2015 01:19 PM, Julien Tachoires wrote:
 I am confused by your fix. Wouldn't cleaner fix be to use
 tbinfo-reltablespace rather than tbinfo-reltoasttablespace when
 calling ArchiveEntry()?

 Yes, doing this that way is cleaner. Here is a new version including
 your fix. Thanks.

 I am now satisfied with how the patch looks. Thanks for your work. I 
 will mark this as ready for committeer now but not expect any committer 
 to look at it until the commitfest starts.

I have just looked through this thread, and TBH I think we should reject
this patch altogether --- not RWF, but no we don't want this.  The
use-case remains hypothetical: no performance numbers showing a real-world
benefit have been exhibited AFAICS.  And allowing a toast table to be in a
different tablespace from its parent opens up a host of corner cases that,
despite the many revisions of the patch so far, probably haven't all been
addressed yet.  (For instance, I wonder whether pg_upgrade copes with
toast tables in non-parent tablespaces.)

I regret the idea of wasting all the work that's been poured into this,
but I think pushing this patch forward will just waste even more time,
now and in the future, for benefit that will be at most marginal.

If any committers nonetheless want to take this up, be advised that it's
far from committable as-is.  Here are some notes just from looking at
the pg_dump part of the patch:

* Addition in pg_backup_archiver.c seems pretty dubious; we don't
handle --no-tablespace that way for other cases.

* Why is getTables() collecting toast tablespace only from latest-model
servers?  This will likely lead to doing the wrong thing (ie, dumping
incorrect ALTER SET TOAST TABLESPACE pg_default commands) when dumping
from an older server.

* dumpTOASTTablespace (man, that's an ugly choice of name ... camel
case combined with all-upper-case-words is a bad idea) neglects the
possibility that it needs to quote the tablespace name.

* Assorted random whitespace inconsistencies, only some of which would
be cleaned up by pgindent.

I've not studied the rest of the patch, but it would clearly need to
be gone over very carefully to get to a committable state.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-21 Thread Andreas Karlsson

On 03/21/2015 01:19 PM, Julien Tachoires wrote:

I am confused by your fix. Wouldn't cleaner fix be to use
tbinfo-reltablespace rather than tbinfo-reltoasttablespace when
calling ArchiveEntry()?


Yes, doing this that way is cleaner. Here is a new version including
your fix. Thanks.


I am now satisfied with how the patch looks. Thanks for your work. I 
will mark this as ready for committeer now but not expect any committer 
to look at it until the commitfest starts.


--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-21 Thread Julien Tachoires
On 20/03/2015 00:33, Andreas Karlsson wrote:
 On 03/19/2015 04:55 PM, Julien Tachoires wrote:
 On 18/03/2015 19:54, Andreas Karlsson wrote:
 Looks good but I think one minor improvement could be to set the table
 space of the toast entires to the same as the tablespace of the table to
 reduce the amount of SET default_tablespace. What do you think?

 Yes, you're right, some useless SET default_tablespace were added for
 each ALTER TABLE SET TOAST TABLESPACE statement. It's now fixed with
 this new patch. Thanks.
 
 I am confused by your fix. Wouldn't cleaner fix be to use 
 tbinfo-reltablespace rather than tbinfo-reltoasttablespace when 
 calling ArchiveEntry()?

Yes, doing this that way is cleaner. Here is a new version including
your fix. Thanks.

--
Julien


set_toast_tablespace_v0.17.patch.gz
Description: application/gzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-19 Thread Andreas Karlsson

On 03/19/2015 04:55 PM, Julien Tachoires wrote:

On 18/03/2015 19:54, Andreas Karlsson wrote:

Looks good but I think one minor improvement could be to set the table
space of the toast entires to the same as the tablespace of the table to
reduce the amount of SET default_tablespace. What do you think?


Yes, you're right, some useless SET default_tablespace were added for
each ALTER TABLE SET TOAST TABLESPACE statement. It's now fixed with
this new patch. Thanks.


I am confused by your fix. Wouldn't cleaner fix be to use 
tbinfo-reltablespace rather than tbinfo-reltoasttablespace when 
calling ArchiveEntry()?


I tried the attached path and it seemed to work just fine.

--
Andreas Karlsson
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index c589372..de6b359 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -8157,7 +8157,7 @@ dumpTOASTTablespace(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo,
 		ArchiveEntry(fout, nilCatalogId, createDumpId(),
 	fmtId(tbinfo-dobj.name),
 	tbinfo-dobj.namespace-dobj.name,
-	tbinfo-reltoasttablespace, tbinfo-rolname,
+	tbinfo-reltablespace, tbinfo-rolname,
 	false, TOAST TABLESPACE, SECTION_NONE,
 	query-data, , NULL,
 	(tbinfo-dobj.dumpId), 1,

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-18 Thread Andreas Karlsson

On 03/17/2015 09:00 AM, Julien Tachoires wrote:

Here is a new version fixing this issue. I've added a new kind of TOC
entry for being able to handle pg_restore --no-tablespace case.


Looks good but I think one minor improvement could be to set the table 
space of the toast entires to the same as the tablespace of the table to 
reduce the amount of SET default_tablespace. What do you think?


--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-17 Thread Julien Tachoires
On 15/03/2015 20:27, Julien Tachoires wrote:
 On 15/03/2015 04:34, Andreas Karlsson wrote:
 On 03/15/2015 04:25 AM, Andreas Karlsson wrote:
 Nice. You will also want to apply the attached patch which fixes support
 for the --no-tablespaces flag.

 Just realized that --no-tablespaces need to be fixed for pg_restore too.
 
 Indeed, after taking a look at pg_restore case, I would say it won't be
 so easy.
 Will try to fix it.

Here is a new version fixing this issue. I've added a new kind of TOC
entry for being able to handle pg_restore --no-tablespace case.

--
Julien


set_toast_tablespace_v0.15.patch.gz
Description: application/gzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-15 Thread Julien Tachoires
On 15/03/2015 04:34, Andreas Karlsson wrote:
 On 03/15/2015 04:25 AM, Andreas Karlsson wrote:
 Nice. You will also want to apply the attached patch which fixes support
 for the --no-tablespaces flag.
 
 Just realized that --no-tablespaces need to be fixed for pg_restore too.

Indeed, after taking a look at pg_restore case, I would say it won't be
so easy.
Will try to fix it.

--
Julien


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-14 Thread Andreas Karlsson

On 03/15/2015 04:25 AM, Andreas Karlsson wrote:

Nice. You will also want to apply the attached patch which fixes support
for the --no-tablespaces flag.


Just realized that --no-tablespaces need to be fixed for pg_restore too.

--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-14 Thread Andreas Karlsson

On 03/14/2015 05:59 PM, Julien Tachoires wrote:

On 14/03/2015 16:10, Andreas Karlsson wrote:

Noticed a bug when playing round some more with pg_dump. It does not
seem to dump custom table space for the table and default table space
for the toast correctly. It forgets about the toast table being in
pg_default.


Good catch. This is now fixed.


Nice. You will also want to apply the attached patch which fixes support 
for the --no-tablespaces flag.


--
Andreas Karlsson
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bc4a0b1..8ef2df9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -13746,7 +13746,8 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
 			destroyPQExpBuffer(result);
 
 			/* Change TOAST tablespace */
-			if (strcmp(tbinfo-reltablespace, tbinfo-reltoasttablespace) != 0)
+			if (!dopt-outputNoTablespaces 
+strcmp(tbinfo-reltablespace, tbinfo-reltoasttablespace) != 0)
 			{
 appendPQExpBuffer(q, \nALTER MATERIALIZED VIEW %s ,
 	fmtId(tbinfo-dobj.name));
@@ -14032,8 +14033,9 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
 	}
 
 	/* Change TOAST tablespace */
-	if (strcmp(tbinfo-reltoasttablespace, tbinfo-reltablespace) != 0 
-			tbinfo-relkind == RELKIND_RELATION)
+	if (!dopt-outputNoTablespaces 
+		strcmp(tbinfo-reltoasttablespace, tbinfo-reltablespace) != 0 
+		tbinfo-relkind == RELKIND_RELATION)
 	{
 		appendPQExpBuffer(q, \nALTER TABLE %s ,
 			fmtId(tbinfo-dobj.name));

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-14 Thread Andreas Karlsson

On 03/13/2015 11:48 AM, Julien Tachoires wrote:

Here is a new version rebased against head and considering Andreas' last
comments:

   - New regression tests about the fact that a notice is raised when
ALTER TABLE SET TOAST TABLESPACE is done for a non-TOASTed table.
   - New regression tests to check that a TOAST index follows the TOAST
table when it's moved.
   - Some fixes in the documentation.
   - psql's '\d' command shows now both table and TOAST table tablespaces
when they are different, even if one of them is pg_default.
   - patch added to the next open commitfest:
https://commitfest.postgresql.org/5/188/


Nice, I have no remaining comments on this patch other than some 
incorrect mixing of whitespace in the indentation of the Case when 
TOAST and table tablespaces are different and als comment.


Once that has been fixed I would say this patch is technically ready for 
committer, but since it is in the open commitfest I do not think it will 
get committed any time soon.


--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-14 Thread Andreas Karlsson
Noticed a bug when playing round some more with pg_dump. It does not 
seem to dump custom table space for the table and default table space 
for the toast correctly. It forgets about the toast table being in 
pg_default.


--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-14 Thread Julien Tachoires
On 14/03/2015 16:10, Andreas Karlsson wrote:
 Noticed a bug when playing round some more with pg_dump. It does not 
 seem to dump custom table space for the table and default table space 
 for the toast correctly. It forgets about the toast table being in 
 pg_default.

Good catch. This is now fixed.

--
Julien



set_toast_tablespace_v0.14.patch.gz
Description: application/gzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-13 Thread Julien Tachoires
Hi,

On 10/03/2015 00:31, Andreas Karlsson wrote:
 On 03/03/2015 04:14 PM, Julien Tachoires wrote:
 Sorry for the delay, I missed this thread.
 Here is a new version of this patch considering Andreas' comments.
 
 Please also add it to the next open commitfest so we do not lose the patch.

Here is a new version rebased against head and considering Andreas' last
comments:

  - New regression tests about the fact that a notice is raised when
ALTER TABLE SET TOAST TABLESPACE is done for a non-TOASTed table.
  - New regression tests to check that a TOAST index follows the TOAST
table when it's moved.
  - Some fixes in the documentation.
  - psql's '\d' command shows now both table and TOAST table tablespaces
when they are different, even if one of them is pg_default.
  - patch added to the next open commitfest:
https://commitfest.postgresql.org/5/188/

Regards,

--
Julien



set_toast_tablespace_v0.13.patch.gz
Description: application/gzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-12 Thread Julien Tachoires
On 10/03/2015 13:27, Alvaro Herrera wrote:
 Robert Haas wrote:
 On Mon, Mar 9, 2015 at 7:26 PM, Andreas Karlsson andr...@proxel.se wrote:
 
 I think we should allow moving the indexes for consistency. With this patch
 we can move everything except for TOAST indexes.

 It might make sense to always put the TOAST index with the TOAST
 table, but it seems strange to put the TOAST index with the heap and
 the TOAST table someplace else.  Or at least, that's how it seems to
 me.
 
 Agreed.  It doesn't seem necessary to allow moving the toast index to a
 tablespace other than the one containing the toast table.  In other
 words, if you move the toast table, the index always follows it, and
 there's no option to move it independently.

This behaviour is already implemented, the TOAST index always follows
the TOAST table. I'll add a couple of regression tests about it.

--
Julien



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-11 Thread Andreas Karlsson

On 03/10/2015 01:23 PM, Robert Haas wrote:

On Mon, Mar 9, 2015 at 7:26 PM, Andreas Karlsson andr...@proxel.se wrote:

- I do not like how \d handles the toast tablespace. Having TOAST in
pg_default and the table in another space looks the same as if there was
no TOAST table at all. I think we should always print both tablespaces
if either of them are not pg_default.


If we do it that way, we should always show the tablespace even if it's
pg_default in any case to be consistent. I'm pretty sure that we don't
want that.


I think we will have to agree to disagree here. I think it should be obvious
when the toast table is in the default tablespace for tables outside it.


I'm not sure about the details here, but that seems like a pretty
sound principle.


What I am talking about are the 6 cases below of \d with the current 
code. I think that case 4) and 5) look the same, which may confuse users 
skimming the \d into thinking that we have no TOAST table in the case 
where the TOAST table is in pg_default while the table tablespace is 
somewhere else.


1. Table in pg_default, no TOAST

 Column |  Type   | Modifiers
+-+---
 x  | integer |

2. Table in pg_default, TOAST in pg_default

 Column | Type | Modifiers
+--+---
 x  | text |

3. Table in pg_default, TOAST in ts1

 Column | Type | Modifiers
+--+---
 x  | text |
TOAST Tablespace: ts1

4. Table in ts1, no TOAST

 Column |  Type   | Modifiers
+-+---
 x  | integer |
Tablespace: ts1

5. Table in ts1, TOAST in pg_default

 Column | Type | Modifiers
+--+---
 x  | text |
Tablespace: ts1

6. Table in ts1, TOAST in ts1

 Column | Type | Modifiers
+--+---
 x  | text |
Tablespace: ts1
TOAST Tablespace: ts1


I think we should allow moving the indexes for consistency. With this patch
we can move everything except for TOAST indexes.


It might make sense to always put the TOAST index with the TOAST
table, but it seems strange to put the TOAST index with the heap and
the TOAST table someplace else.  Or at least, that's how it seems to
me.


Ok, my idea was that one might want to put all indexes in a third table 
space, including TOAST indexes. Not sure how realistic this use case is.


--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-10 Thread Robert Haas
On Mon, Mar 9, 2015 at 7:26 PM, Andreas Karlsson andr...@proxel.se wrote:
 - I do not like how \d handles the toast tablespace. Having TOAST in
 pg_default and the table in another space looks the same as if there was
 no TOAST table at all. I think we should always print both tablespaces
 if either of them are not pg_default.

 If we do it that way, we should always show the tablespace even if it's
 pg_default in any case to be consistent. I'm pretty sure that we don't
 want that.

 I think we will have to agree to disagree here. I think it should be obvious
 when the toast table is in the default tablespace for tables outside it.

I'm not sure about the details here, but that seems like a pretty
sound principle.

 - Would it be interesting to add syntax for moving the toast index to a
 separate tablespace?

 -1, we just want to be able to move TOAST heap, which is supposed to
 contain a lot of data and we want to move on a different storage device
 / filesystem.

 I think we should allow moving the indexes for consistency. With this patch
 we can move everything except for TOAST indexes.

It might make sense to always put the TOAST index with the TOAST
table, but it seems strange to put the TOAST index with the heap and
the TOAST table someplace else.  Or at least, that's how it seems to
me.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-10 Thread Alvaro Herrera
Robert Haas wrote:
 On Mon, Mar 9, 2015 at 7:26 PM, Andreas Karlsson andr...@proxel.se wrote:

  I think we should allow moving the indexes for consistency. With this patch
  we can move everything except for TOAST indexes.
 
 It might make sense to always put the TOAST index with the TOAST
 table, but it seems strange to put the TOAST index with the heap and
 the TOAST table someplace else.  Or at least, that's how it seems to
 me.

Agreed.  It doesn't seem necessary to allow moving the toast index to a
tablespace other than the one containing the toast table.  In other
words, if you move the toast table, the index always follows it, and
there's no option to move it independently.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training  Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-09 Thread Andreas Karlsson

On 03/03/2015 04:14 PM, Julien Tachoires wrote:

On 30/12/2014 03:48, Andreas Karlsson wrote:

- A test fails in create_view.out. I looked some into it and did not see
how this could happen.

[...]


I can't reproduce this issue.


Neither can I anymore.


- pg_dump is broken

  pg_dump: [archiver (db)] query failed: ERROR:  syntax error at or
near (
  LINE 1: ...nest(tc.reloptions) x), ', ') AS toast_reloptions
(SELECT sp...


Fixed.


I still get a failing pg_dump test though when running make check-world. 
And it does not work when run manually either.


+ pg_dumpall -f 
/home/andreas/dev/postgresql/contrib/pg_upgrade/tmp_check/dump1.sql

pg_dump: column number -1 is out of range 0..28
cannot duplicate null pointer (internal error)
pg_dumpall: pg_dump failed on database postgres, exiting
+ pg_dumpall1_status=1
+ [ /home/andreas/dev/postgresql != /home/andreas/dev/postgresql ]
+ 
/home/andreas/dev/postgresql/contrib/pg_upgrade/tmp_check/install//home/andreas/dev/postgresql-inst/bin/pg_ctl 
-m fast stop

waiting for server to shut down done
server stopped
+ [ -n  ]
+ [ -n  ]
+ [ -n 1 ]
+ echo pg_dumpall of pre-upgrade database cluster failed
pg_dumpall of pre-upgrade database cluster failed
+ exit 1
+ rm -rf /tmp/pg_upgrade_check-5A3wsI


- I do not like how \d handles the toast tablespace. Having TOAST in
pg_default and the table in another space looks the same as if there was
no TOAST table at all. I think we should always print both tablespaces
if either of them are not pg_default.


If we do it that way, we should always show the tablespace even if it's
pg_default in any case to be consistent. I'm pretty sure that we don't
want that.


I think we will have to agree to disagree here. I think it should be 
obvious when the toast table is in the default tablespace for tables 
outside it.



- Would it be interesting to add syntax for moving the toast index to a
separate tablespace?


-1, we just want to be able to move TOAST heap, which is supposed to
contain a lot of data and we want to move on a different storage device
/ filesystem.


I think we should allow moving the indexes for consistency. With this 
patch we can move everything except for TOAST indexes.



- There is no warning if you set the toast table space of a table
lacking toast. I think there should be one.


A notice is raised now in that case.


Excellent, also add a test case for this.


- Missing periods on the ALTER TABLE manual page after See also CREATE
TABLESPACE (in two places).

- Missing period last in the new paragraph added to the storage manual page.


I don't understand those 2 last points ?



I mean that TOAST table can be moved to a different tablespace with 
commandALTER TABLE SET TOAST TABLESPACE/ should be changed to 
TOAST table can be moved to a different tablespace with commandALTER 
TABLE SET TOAST TABLESPACE/. since a sentece should always end in ..


= New comments

- The patch does not apply cleanly anymore, I had to solve to minor 
conflicts.


- Rewrap the documentation for SET TABLESPACE.

- You have added a tab in alter_table.sgml.

- Merge case AT_SetTableTableSpace: and case AT_SetToastTableSpace: 
where they both do the same thing, i.e. nothing.


- Should it not be foo_mv in the query from the test suite below?

SELECT spcname FROM pg_class c JOIN pg_class d ON 
(c.reltoastrelid=d.oid) JOIN pg_tablespace ON (d.reltablespace = 
pg_tablespace.oid) WHERE c.relname = 'foo2';


--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-09 Thread Andreas Karlsson

On 03/03/2015 04:14 PM, Julien Tachoires wrote:

Sorry for the delay, I missed this thread.
Here is a new version of this patch considering Andreas' comments.


Please also add it to the next open commitfest so we do not lose the patch.

--
Andreas Karlsson


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-03-03 Thread Julien Tachoires
Hi,

Sorry for the delay, I missed this thread.
Here is a new version of this patch considering Andreas' comments.

On 30/12/2014 03:48, Andreas Karlsson wrote:
 - A test fails in create_view.out. I looked some into it and did not see 
 how this could happen.
 
  *** 
 /home/andreas/dev/postgresql/src/test/regress/expected/create_view.out 
 2014-08-09 01:35:50.008886776 +0200
  --- 
 /home/andreas/dev/postgresql/src/test/regress/results/create_view.out 
 2014-12-30 00:41:17.966525339 +0100
  ***
  *** 283,289 ***
  relname   | relkind |reloptions
+-+--
 mysecview1 | v   |
  !  mysecview2 | v   |
 mysecview3 | v   | {security_barrier=true}
 mysecview4 | v   | {security_barrier=false}
(4 rows)
  --- 283,289 
  relname   | relkind |reloptions
+-+--
 mysecview1 | v   |
  !  mysecview2 | v   | {security_barrier=true}
 mysecview3 | v   | {security_barrier=true}
 mysecview4 | v   | {security_barrier=false}
(4 rows)

I can't reproduce this issue.

 - pg_dump is broken
 
  pg_dump: [archiver (db)] query failed: ERROR:  syntax error at or 
 near (
  LINE 1: ...nest(tc.reloptions) x), ', ') AS toast_reloptions 
 (SELECT sp...

Fixed.

 - ALTER INDEX foo_idx SET TABLE TABLESPACE ... should not be allowed, 
 currently it changes the tablespace of the index. I do not think ALTER 
 INDEX foo_idx SET (TABLE|TOAST) TABLESPACE ... should even be allowed 
 in the grammar.

Fixed.

 - You should add a link to 
 http://www.postgresql.org/docs/current/interactive/storage-toast.html to 
 the manual page of ALTER TABLE.

Added.

 - I do not like how \d handles the toast tablespace. Having TOAST in 
 pg_default and the table in another space looks the same as if there was 
 no TOAST table at all. I think we should always print both tablespaces 
 if either of them are not pg_default.

If we do it that way, we should always show the tablespace even if it's
pg_default in any case to be consistent. I'm pretty sure that we don't
want that.

 - Would it be interesting to add syntax for moving the toast index to a 
 separate tablespace?

-1, we just want to be able to move TOAST heap, which is supposed to
contain a lot of data and we want to move on a different storage device
/ filesystem.

 - There is no warning if you set the toast table space of a table 
 lacking toast. I think there should be one.

A notice is raised now in that case.

 - No support for materialized views as pointed out by Alex.

Support, documentation and regression tests added.

 - I think the code would be cleaner if ATPrepSetTableSpace and 
 ATPrepSetToastTableSpace were merged into one function or split into 
 two, one setting the toast and one setting the tablespace of the table 
 and one setting it on the TOAST table.

Done.

 - I think a couple of tests for the error cases would be nice.

2 more tests added.

 - Missing periods on the ALTER TABLE manual page after See also CREATE 
 TABLESPACE (in two places).
 
 - Missing period last in the new paragraph added to the storage manual page.

I don't understand those 2 last points ?

 - Double spaces in src/backend/catalog/toasting.c after if (new_toast.

Fixed.

 - The comment and if this is not a TOAST re-creation case (through 
 CLUSTER). incorrectly implies that CLUSTER is the only case where the 
 TOAST table is recreated.

Fixed.

 - The local variables ToastTableSpace and ToastRel do not follow the 
 naming conventions.

Fixed (I hope so).

 - Remove the parentheses around (is_system_catalog).

Fixed.

 - Why was the Save info for Phase 3 to do the real work comment 
 changed to XXX: Save info for Phase 3 to do the real work?

Fixed.

 - Incorrect indentation for new code added last in ATExecSetTableSpace.

Fixed.

 - The patch adds commented out code in src/include/commands/tablecmds.h.

Fixed.

Thank you for your review.

--
Julien
diff --git a/doc/src/sgml/ref/alter_materialized_view.sgml b/doc/src/sgml/ref/alter_materialized_view.sgml
index b0759fc..321ffc9 100644
--- a/doc/src/sgml/ref/alter_materialized_view.sgml
+++ b/doc/src/sgml/ref/alter_materialized_view.sgml
@@ -44,6 +44,8 @@ ALTER MATERIALIZED VIEW ALL IN TABLESPACE replaceable class=parametername/r
 RESET ( replaceable class=PARAMETERstorage_parameter/replaceable [, ... ] )
 OWNER TO replaceable class=PARAMETERnew_owner/replaceable
 SET TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
+SET TABLE TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
+SET TOAST TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
 /synopsis
  /refsynopsisdiv
 
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index b3a4970..777ba57 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ 

Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2015-01-14 Thread Michael Paquier
On Tue, Dec 30, 2014 at 11:48 AM, Andreas Karlsson andr...@proxel.se wrote:
 Here is my review of the feature.
Marked as returned with feedback.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2014-12-02 Thread Robert Haas
On Fri, Nov 28, 2014 at 11:38 AM, Alex Shulgin a...@commandprompt.com wrote:
 Should I add this patch to the next CommitFest?

If you don't want it to get forgotten about, yes.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2014-11-28 Thread Alex Shulgin
Julien Tachoires jul...@gmail.com writes:

 2011/12/15 Alvaro Herrera alvhe...@commandprompt.com:

 Uhm, surely you could compare the original toast tablespace to the heap
 tablespace, and if they differ, handle appropriately when creating the
 new toast table? =A0Just pass down the toast tablespace into
 AlterTableCreateToastTable, instead of having it assume that
 rel-rd_rel-relnamespace is sufficient. =A0This should be done in all
 cases where a toast tablespace is created, which shouldn't be more than
 a handful of them.

 Thank you, that way seems right.
 Now, I distinguish before each creation of a TOAST table with
 AlterTableCreateToastTable() : if it will create a new one or recreate
 an existing one.
 Thus, in create_toast_table() when toastTableSpace is equal to
 InvalidOid, we are able :
 - to fallback to the main table tablespace in case of new TOAST table creat=
 ion
 - to keep it previous tablespace in case of recreation.

 Here's a new version rebased against HEAD.

Almost 3 years later, here's a version rebased against current HEAD. :-)

It compiles and even passes the included regression test.

There were doubts originally if this is actually a useful feature, but
there is at least one plausible use case (main table on SSD, TOAST on
HDD):

  http://www.postgresql.org/message-id/4f3267ee.80...@deltasoft.no

I didn't add anything on top of the latest version, but I notice we've
added mat. views since then, so it looks like we now also need this:

  ALTER MATERIALIZED VIEW SET [VIEW | TOAST] TABLESPACE


Should I add this patch to the next CommitFest?

--
Alex



set_toast_tablespace_v0.11.patch.gz
Description: application/gzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-02-10 Thread Jim Nasby

On 2/8/12 6:17 AM, Christian Nicolaisen wrote:

Hi

We have some tables with documents and their metadata (filename, filetype, etc).
Most of the time we just want to get the metadata (filename, filetype, etc) and 
not the document itself.
In this case it would be nice to have the metadata (which wouldn't end up on 
the TOAST) on a fast array (SSD-based),
and put the filedata on a slow array (HDD-based). It's probably possible to 
have two tables one for metadata and one
for the extra file, but this is a workaround.

Did you intend to post a patch? Because nothing was attached. Also, if you're 
submitting a patch you should add it to the next commitfest.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-02-10 Thread Robert Haas
On Fri, Feb 10, 2012 at 2:23 PM, Jim Nasby j...@nasby.net wrote:
 On 2/8/12 6:17 AM, Christian Nicolaisen wrote:
 We have some tables with documents and their metadata (filename, filetype,
 etc).
 Most of the time we just want to get the metadata (filename, filetype,
 etc) and not the document itself.
 In this case it would be nice to have the metadata (which wouldn't end up
 on the TOAST) on a fast array (SSD-based),
 and put the filedata on a slow array (HDD-based). It's probably possible
 to have two tables one for metadata and one
 for the extra file, but this is a workaround.

 Did you intend to post a patch? Because nothing was attached. Also, if
 you're submitting a patch you should add it to the next commitfest.

He was commenting on the possible utility of a previously-submitted
patch, which got pushed off because we weren't sure it was good for
anything.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-02-08 Thread Christian Nicolaisen

Hi

We have some tables with documents and their metadata (filename, 
filetype, etc).
Most of the time we just want to get the metadata (filename, filetype, 
etc) and not the document itself.
In this case it would be nice to have the metadata (which wouldn't end 
up on the TOAST) on a fast array (SSD-based),
and put the filedata on a slow array (HDD-based). It's probably possible 
to have two tables one for metadata and one

for the extra file, but this is a workaround.

--
Mvh
Christian Nicolaisen
Deltasoft AS
Tlf  +47 938 38 596


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-01-25 Thread Julien Tachoires
2012/1/24 Robert Haas robertmh...@gmail.com:
 On Sun, Jan 22, 2012 at 11:04 AM, Julien Tachoires jul...@gmail.com wrote:
 2011/12/15 Alvaro Herrera alvhe...@commandprompt.com:

 Uhm, surely you could compare the original toast tablespace to the heap
 tablespace, and if they differ, handle appropriately when creating the
 new toast table?  Just pass down the toast tablespace into
 AlterTableCreateToastTable, instead of having it assume that
 rel-rd_rel-relnamespace is sufficient.  This should be done in all
 cases where a toast tablespace is created, which shouldn't be more than
 a handful of them.

 Thank you, that way seems right.
 Now, I distinguish before each creation of a TOAST table with
 AlterTableCreateToastTable() : if it will create a new one or recreate
 an existing one.
 Thus, in create_toast_table() when toastTableSpace is equal to
 InvalidOid, we are able :
 - to fallback to the main table tablespace in case of new TOAST table 
 creation
 - to keep it previous tablespace in case of recreation.

 Here's a new version rebased against HEAD.

 To ask more directly the question that's come up a few times upthread,
 why do *you* think this is useful?  What motivated you to want this
 behavior, and/or how do you think it could benefit other PostgreSQL
 users?


Sorry, I didn't get this question to me. I've just picked up this item
from the TODO list and then I was thinking that it could be useful. My
motivation was to learn more about PostgreSQL dev. and to work on a
concrete case. Now, I'm not sure anymore this is useful.

--
JT

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-01-25 Thread Robert Haas
On Wed, Jan 25, 2012 at 7:13 AM, Julien Tachoires jul...@gmail.com wrote:
 2012/1/24 Robert Haas robertmh...@gmail.com:
 On Sun, Jan 22, 2012 at 11:04 AM, Julien Tachoires jul...@gmail.com wrote:
 2011/12/15 Alvaro Herrera alvhe...@commandprompt.com:

 Uhm, surely you could compare the original toast tablespace to the heap
 tablespace, and if they differ, handle appropriately when creating the
 new toast table?  Just pass down the toast tablespace into
 AlterTableCreateToastTable, instead of having it assume that
 rel-rd_rel-relnamespace is sufficient.  This should be done in all
 cases where a toast tablespace is created, which shouldn't be more than
 a handful of them.

 Thank you, that way seems right.
 Now, I distinguish before each creation of a TOAST table with
 AlterTableCreateToastTable() : if it will create a new one or recreate
 an existing one.
 Thus, in create_toast_table() when toastTableSpace is equal to
 InvalidOid, we are able :
 - to fallback to the main table tablespace in case of new TOAST table 
 creation
 - to keep it previous tablespace in case of recreation.

 Here's a new version rebased against HEAD.

 To ask more directly the question that's come up a few times upthread,
 why do *you* think this is useful?  What motivated you to want this
 behavior, and/or how do you think it could benefit other PostgreSQL
 users?

 Sorry, I didn't get this question to me. I've just picked up this item
 from the TODO list and then I was thinking that it could be useful. My
 motivation was to learn more about PostgreSQL dev. and to work on a
 concrete case. Now, I'm not sure anymore this is useful.

OK.  In that case, I don't think it makes sense to add this right now.
 I share the feeling that it could possibly be useful under some set
of circumstances, but it doesn't seem prudent to add features because
there might hypothetically be a use case.  I suggest that we mark this
patch Returned with Feedback and add links to your latest version of
this patch and one of the emails expressing concerns about the utility
of this patch to the Todo list.  If we later have a clearer idea in
mind why this might be useful, we can add it then - and document the
use case.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-01-24 Thread Robert Haas
On Sun, Jan 22, 2012 at 11:04 AM, Julien Tachoires jul...@gmail.com wrote:
 2011/12/15 Alvaro Herrera alvhe...@commandprompt.com:

 Uhm, surely you could compare the original toast tablespace to the heap
 tablespace, and if they differ, handle appropriately when creating the
 new toast table?  Just pass down the toast tablespace into
 AlterTableCreateToastTable, instead of having it assume that
 rel-rd_rel-relnamespace is sufficient.  This should be done in all
 cases where a toast tablespace is created, which shouldn't be more than
 a handful of them.

 Thank you, that way seems right.
 Now, I distinguish before each creation of a TOAST table with
 AlterTableCreateToastTable() : if it will create a new one or recreate
 an existing one.
 Thus, in create_toast_table() when toastTableSpace is equal to
 InvalidOid, we are able :
 - to fallback to the main table tablespace in case of new TOAST table creation
 - to keep it previous tablespace in case of recreation.

 Here's a new version rebased against HEAD.

To ask more directly the question that's come up a few times upthread,
why do *you* think this is useful?  What motivated you to want this
behavior, and/or how do you think it could benefit other PostgreSQL
users?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-01-16 Thread Alvaro Herrera

Excerpts from Jaime Casanova's message of lun ene 16 03:23:30 -0300 2012:
 On Tue, Dec 13, 2011 at 12:29 PM, Julien Tachoires jul...@gmail.com wrote:
 
  OK, considering that, I don't see any way to handle the case raised by 
  Jaime :(
 
 Did you consider what Álvaro suggested? anyway, seems is too late for
 this commitfest.
 are you intending to resume work on this for the next cycle?
 do we consider this as a useful thing?

The remaining bits shouldn't be too hard.  In case Julien is not
interested in the task, I have added a link to this discussion in the
TODO item.  

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-01-16 Thread Julien Tachoires
Hi,

2012/1/16 Alvaro Herrera alvhe...@commandprompt.com:

 Excerpts from Jaime Casanova's message of lun ene 16 03:23:30 -0300 2012:
 On Tue, Dec 13, 2011 at 12:29 PM, Julien Tachoires jul...@gmail.com wrote:
 
  OK, considering that, I don't see any way to handle the case raised by 
  Jaime :(

 Did you consider what Álvaro suggested? anyway, seems is too late for
 this commitfest.

Not yet.

 are you intending to resume work on this for the next cycle?
 do we consider this as a useful thing?

That's a good question.
If the answer is yes, I'll continue on this work.


 The remaining bits shouldn't be too hard.  In case Julien is not
 interested in the task, I have added a link to this discussion in the
 TODO item.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2012-01-15 Thread Jaime Casanova
On Tue, Dec 13, 2011 at 12:29 PM, Julien Tachoires jul...@gmail.com wrote:

 OK, considering that, I don't see any way to handle the case raised by Jaime 
 :(

Did you consider what Álvaro suggested? anyway, seems is too late for
this commitfest.
are you intending to resume work on this for the next cycle?
do we consider this as a useful thing?

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-15 Thread Greg Smith

On 12/13/2011 12:29 PM, Julien Tachoires wrote:

2011/12/13 Robert Haasrobertmh...@gmail.com:
   

On Tue, Dec 13, 2011 at 12:02 PM, Julien Tachoiresjul...@gmail.com  wrote:
 

Right, it seems to happen when the destination tablespace is the same
as the database's tbs, because, in this case, relation's tbs is set to
InvalidOid :
src/backend/commands/tablecmds.c line 8342

+   rd_rel-reltablespace = (newTableSpace == MyDatabaseTableSpace) ?
InvalidOid : newTableSpace;

Why don't just asign newTableSpace value here ?
   

When a relation is stored in the default tablespace, we always record
that in the system catalogs as InvalidOid.  Otherwise, if the
database's default tablespace were changed, things would break.
 

OK, considering that, I don't see any way to handle the case raised by Jaime :(
   


So we have a problem here:  there's a case that's messy to handle.  And 
there's really a large issue hanging over this whole patch, which is 
that it needs a better explanation of what exactly it's going to get 
used for.  Especially if the implementation gets more complicated, we'd 
want to see a clear reason to use this feature.  And that's not really 
clear.


If you can return with an update that perhaps finds a way to work around 
this OID issue, please re-submit that.  And if you can explain some more 
about where you think this feature is useful, more information on that 
would be helpful.  Since this isn't going to get committed soon, I'm 
going to mark it returned with feedback for now.


--
Greg Smith   2ndQuadrant USg...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-15 Thread Alvaro Herrera

Excerpts from Julien Tachoires's message of mar dic 13 14:29:56 -0300 2011:
 2011/12/13 Robert Haas robertmh...@gmail.com:
  On Tue, Dec 13, 2011 at 12:02 PM, Julien Tachoires jul...@gmail.com wrote:
  Right, it seems to happen when the destination tablespace is the same
  as the database's tbs, because, in this case, relation's tbs is set to
  InvalidOid :
  src/backend/commands/tablecmds.c line 8342
 
  +       rd_rel-reltablespace = (newTableSpace == MyDatabaseTableSpace) ?
  InvalidOid : newTableSpace;
 
  Why don't just asign newTableSpace value here ?
 
  When a relation is stored in the default tablespace, we always record
  that in the system catalogs as InvalidOid.  Otherwise, if the
  database's default tablespace were changed, things would break.
 
 OK, considering that, I don't see any way to handle the case raised by Jaime 
 :(

Uhm, surely you could compare the original toast tablespace to the heap
tablespace, and if they differ, handle appropriately when creating the
new toast table?  Just pass down the toast tablespace into
AlterTableCreateToastTable, instead of having it assume that
rel-rd_rel-relnamespace is sufficient.  This should be done in all
cases where a toast tablespace is created, which shouldn't be more than
a handful of them.

-- 
Álvaro Herrera alvhe...@commandprompt.com
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-13 Thread Julien Tachoires
2011/12/13 Jaime Casanova ja...@2ndquadrant.com:
 On Mon, Dec 12, 2011 at 10:54 AM, Julien Tachoires jul...@gmail.com wrote:
 2011/12/10 Jaime Casanova ja...@2ndquadrant.com:
 On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires jul...@gmail.com wrote:

 2) after CLUSTER the index of the toast table gets moved to the same
 tablespace as the main table


 there is still a variant of this one, i created 3 tablespaces 
 (datos_tblspc):

 
 create table t1 (
     i serial primary key,
     t text
 ) tablespace datos_tblspc;

 ALTER TABLE t1 SET TOAST TABLESPACE pg_default;
 CLUSTER t1 USING t1_pkey;
 

 I am not able to reproduce this case, could you show me exactly how to
 reproduce it ?


 just as that...
 - create a table in a certain tablespace (diferent from pg_default),
 the toast table will be in the same tablespace,
 - then change the tablespace to pg_default and
 - then cluster the table...
 the toast table will be again in the same tablespace as the main table

Right, it seems to happen when the destination tablespace is the same
as the database's tbs, because, in this case, relation's tbs is set to
InvalidOid :
src/backend/commands/tablecmds.c line 8342

+   rd_rel-reltablespace = (newTableSpace == MyDatabaseTableSpace) ?
InvalidOid : newTableSpace;

Why don't just asign newTableSpace value here ?

Thanks,

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-13 Thread Robert Haas
On Tue, Dec 13, 2011 at 12:02 PM, Julien Tachoires jul...@gmail.com wrote:
 Right, it seems to happen when the destination tablespace is the same
 as the database's tbs, because, in this case, relation's tbs is set to
 InvalidOid :
 src/backend/commands/tablecmds.c line 8342

 +       rd_rel-reltablespace = (newTableSpace == MyDatabaseTableSpace) ?
 InvalidOid : newTableSpace;

 Why don't just asign newTableSpace value here ?

When a relation is stored in the default tablespace, we always record
that in the system catalogs as InvalidOid.  Otherwise, if the
database's default tablespace were changed, things would break.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-13 Thread Julien Tachoires
2011/12/13 Robert Haas robertmh...@gmail.com:
 On Tue, Dec 13, 2011 at 12:02 PM, Julien Tachoires jul...@gmail.com wrote:
 Right, it seems to happen when the destination tablespace is the same
 as the database's tbs, because, in this case, relation's tbs is set to
 InvalidOid :
 src/backend/commands/tablecmds.c line 8342

 +       rd_rel-reltablespace = (newTableSpace == MyDatabaseTableSpace) ?
 InvalidOid : newTableSpace;

 Why don't just asign newTableSpace value here ?

 When a relation is stored in the default tablespace, we always record
 that in the system catalogs as InvalidOid.  Otherwise, if the
 database's default tablespace were changed, things would break.

OK, considering that, I don't see any way to handle the case raised by Jaime :(

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-12 Thread Julien Tachoires
Hi,

2011/12/10 Jaime Casanova ja...@2ndquadrant.com:
 On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires jul...@gmail.com wrote:

 2) after CLUSTER the index of the toast table gets moved to the same
 tablespace as the main table


 there is still a variant of this one, i created 3 tablespaces (datos_tblspc):

 
 create table t1 (
     i serial primary key,
     t text
 ) tablespace datos_tblspc;

 ALTER TABLE t1 SET TOAST TABLESPACE pg_default;
 CLUSTER t1 USING t1_pkey;
 

I am not able to reproduce this case, could you show me exactly how to
reproduce it ?



 now, if we are now supporting this variants
 ALTER TABLE SET TABLE TABLESPACE
 ALTER TABLE SET TOAST TABLESPACE

 why not also support ALTER TABLE SET INDEX TABLESPACE which should
 have the same behaviour as ALTER INDEX SET TABLESPACE... just an idea,
 and of course not necessary for this patch


 any opinion about this? maybe i can make a patch for that if there is
 consensus that it could be good for symettry

Thanks,

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-12 Thread Robert Haas
On Sat, Dec 10, 2011 at 4:16 PM, Jaime Casanova ja...@2ndquadrant.com wrote:
 now, if we are now supporting this variants
 ALTER TABLE SET TABLE TABLESPACE
 ALTER TABLE SET TOAST TABLESPACE

 why not also support ALTER TABLE SET INDEX TABLESPACE which should
 have the same behaviour as ALTER INDEX SET TABLESPACE... just an idea,
 and of course not necessary for this patch

 any opinion about this? maybe i can make a patch for that if there is
 consensus that it could be good for symettry

I'm not really convinced we need it.  I think it would end up just
being a shorthand for ALTER INDEX .. SET TABLESPACE for each index.
Most tables don't have more than a handful of indexes, so it doesn't
seem like we'd be gaining much (compare GRANT ... ON ALL TABLES IN
SCHEMA, which could easily be a shorthand for hundreds or perhaps even
thousands of individual GRANT statements).

Also, it seems that we haven't really discussed much why moving the
TOAST table to a different tablespace from the main table might be
useful.  I'm not saying we shouldn't have it if it's good for
something, but what's the reason for wanting it?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-12 Thread Jaime Casanova
On Mon, Dec 12, 2011 at 10:54 AM, Julien Tachoires jul...@gmail.com wrote:
 Hi,

 2011/12/10 Jaime Casanova ja...@2ndquadrant.com:
 On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires jul...@gmail.com wrote:

 2) after CLUSTER the index of the toast table gets moved to the same
 tablespace as the main table


 there is still a variant of this one, i created 3 tablespaces (datos_tblspc):

 
 create table t1 (
     i serial primary key,
     t text
 ) tablespace datos_tblspc;

 ALTER TABLE t1 SET TOAST TABLESPACE pg_default;
 CLUSTER t1 USING t1_pkey;
 

 I am not able to reproduce this case, could you show me exactly how to
 reproduce it ?


just as that...
- create a table in a certain tablespace (diferent from pg_default),
the toast table will be in the same tablespace,
- then change the tablespace to pg_default and
- then cluster the table...
the toast table will be again in the same tablespace as the main table


-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-12-10 Thread Jaime Casanova
On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires jul...@gmail.com wrote:
 Hi Jaime,

 Please find a new version.


cool

 2) after CLUSTER the index of the toast table gets moved to the same
 tablespace as the main table


there is still a variant of this one, i created 3 tablespaces (datos_tblspc):


create table t1 (
 i serial primary key,
 t text
) tablespace datos_tblspc;

ALTER TABLE t1 SET TOAST TABLESPACE pg_default;
CLUSTER t1 USING t1_pkey;



 now, if we are now supporting this variants
 ALTER TABLE SET TABLE TABLESPACE
 ALTER TABLE SET TOAST TABLESPACE

 why not also support ALTER TABLE SET INDEX TABLESPACE which should
 have the same behaviour as ALTER INDEX SET TABLESPACE... just an idea,
 and of course not necessary for this patch


any opinion about this? maybe i can make a patch for that if there is
consensus that it could be good for symettry

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-11-15 Thread Jaime Casanova
On Tue, Nov 15, 2011 at 11:08 AM, Julien Tachoires jul...@gmail.com wrote:

 Maybe I'd missed something, but the normal case is :
 ALTER TABLE ... SET TABLESPACE = moves Table + moves associated TOAST Table
 ALTER TABLE ... SET TABLE TABLESPACE = moves Table  keeps associated
 TOAST Table at its place
 ALTER TABLE ... SET TOAST TABLESPACE = keeps Table at its place 
 moves associated TOAST Table


oh! i didn't test the patch just read it... and maybe i misunderstood,
will see it again.

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-11-15 Thread Jaime Casanova
On Tue, Nov 15, 2011 at 11:08 AM, Julien Tachoires jul...@gmail.com wrote:

 Maybe I'd missed something, but the normal case is :
 ALTER TABLE ... SET TABLESPACE = moves Table + moves associated TOAST Table
 ALTER TABLE ... SET TABLE TABLESPACE = moves Table  keeps associated
 TOAST Table at its place
 ALTER TABLE ... SET TOAST TABLESPACE = keeps Table at its place 
 moves associated TOAST Table


it has docs, and pg_dump support which is good.

but i found a few problems with the behaviour:
1) it accepts the sintax ALTER INDEX ... SET TOAST TABLESPACE ...;
which does nothing
2) after CLUSTER the index of the toast table gets moved to the same
tablespace as the main table
3) after ALTER TABLE ... ALTER ... TYPE ...; the toast table gets
moved to the same tablespace as the main table

now, if we are now supporting this variants
ALTER TABLE SET TABLE TABLESPACE
ALTER TABLE SET TOAST TABLESPACE

why not also support ALTER TABLE SET INDEX TABLESPACE which should
have the same behaviour as ALTER INDEX SET TABLESPACE... just an idea,
and of course not necessary for this patch

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-11-14 Thread Jaime Casanova
On Fri, Oct 7, 2011 at 10:10 AM, Julien Tachoires jul...@gmail.com wrote:
 Hi,

 Here's a patch to allow TOAST tables to be moved to a different tablespace.
 This item has been picked up from the TODO list.
 Main idea is to consider that a TOAST table can have its own tablespace.


Hi,

This patch doesn't apply cleanly to head now... can you send a new
version against head?

about the patch itself. i don't like the fact that now the normal case
needs to include the word TABLE. IMHO, it should be optional and if
ommited TABLE should be assumed

-- 
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte 24x7 y capacitación

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-10-08 Thread Kevin Grittner
Julien Tachoires  10/07/11 10:17 AM 
 
 Here's a patch to allow TOAST tables to be moved to a different
 tablespace. This item has been picked up from the TODO list.
 Main idea is to consider that a TOAST table can have its own
 tablespace.
 
Thanks for the patch.  Please add this to the open CommitFest at:
 
https://commitfest.postgresql.org/action/commitfest_view/open
 
That will help ensure we don't lose track of it before the next
review cycle.  For more information on the review and commit process,
see this page:
 
http://wiki.postgresql.org/wiki/CommitFest
 
We are currently well in to a CF and still have patches which nobody
has yet volunteered to review.  If you could help with that, it would
be great!
 
https://commitfest.postgresql.org/action/commitfest_view/inprogress
 
-Kevin

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] patch : Allow toast tables to be moved to a different tablespace

2011-10-07 Thread Julien Tachoires

Hi,

Here's a patch to allow TOAST tables to be moved to a different 
tablespace. This item has been picked up from the TODO list.

Main idea is to consider that a TOAST table can have its own tablespace.

Regards,

--
JT
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index 00a477e..a2360f4 100644
--- a/doc/src/sgml/ref/alter_table.sgml
+++ b/doc/src/sgml/ref/alter_table.sgml
@@ -66,6 +66,8 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable
 NOT OF
 OWNER TO replaceable class=PARAMETERnew_owner/replaceable
 SET TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
+SET TABLE TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
+SET TOAST TABLESPACE replaceable class=PARAMETERnew_tablespace/replaceable
 
 phraseand replaceable class=PARAMETERtable_constraint_using_index/replaceable is:/phrase
 
@@ -549,6 +551,30 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable
  /para
 /listitem
/varlistentry
+   
+   varlistentry
+termliteralSET TABLE TABLESPACE/literal/term
+listitem
+ para
+  This form changes only table's tablespace (not associated TOAST table's tablespace) 
+	  to the specified tablespace and moves the data file(s) associated to the new tablespace.
+  See also
+  xref linkend=SQL-CREATETABLESPACE
+ /para
+/listitem
+   /varlistentry
+
+   varlistentry
+termliteralSET TOAST TABLESPACE/literal/term
+listitem
+ para
+  This form changes the TOAST table's tablespace to the specified tablespace and
+  moves the data file(s) associated with the TOAST table to the new tablespace.
+  See also
+  xref linkend=SQL-CREATETABLESPACE
+ /para
+/listitem
+   /varlistentry
 
varlistentry
 termliteralRENAME/literal/term
diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml
index 0a133bb..d7d4235 100644
--- a/doc/src/sgml/storage.sgml
+++ b/doc/src/sgml/storage.sgml
@@ -422,6 +422,11 @@ pages). There was no run time difference compared to an un-acronymTOAST/ed
 comparison table, in which all the HTML pages were cut down to 7 kB to fit.
 /para
 
+para
+TOAST table can be moved to a different tablespace with
+commandALTER TABLE SET TOAST TABLESPACE/
+/para
+
 /sect1
 
 sect1 id=storage-fsm
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index a938c98..7ad965e 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -36,7 +36,7 @@ extern Oid	binary_upgrade_next_toast_pg_class_oid;
 Oid			binary_upgrade_next_toast_pg_type_oid = InvalidOid;
 
 static bool create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
-   Datum reloptions);
+   Datum reloptions, Oid toastTableSpace);
 static bool needs_toast_table(Relation rel);
 
 
@@ -53,19 +53,30 @@ static bool needs_toast_table(Relation rel);
  * to end with CommandCounterIncrement if it makes any changes.
  */
 void
-AlterTableCreateToastTable(Oid relOid, Datum reloptions)
+AlterTableCreateToastTable(Oid relOid, Datum reloptions, Oid toastTableSpace)
 {
 	Relation	rel;
-
+	Relation	toast_rel;
 	/*
 	 * Grab a DDL-exclusive lock on the target table, since we'll update the
 	 * pg_class tuple.	This is redundant for all present users.  Tuple
 	 * toasting behaves safely in the face of a concurrent TOAST table add.
 	 */
 	rel = heap_open(relOid, ShareUpdateExclusiveLock);
+	
+	/*
+	 * if NewToastTableSpace is null then try to find old TOAST table's tablespace
+	 */
+	if (!OidIsValid(toastTableSpace)  OidIsValid(rel-rd_rel-reltoastrelid))
+	{
+		toast_rel = relation_open(rel-rd_rel-reltoastrelid, NoLock);
+		if (OidIsValid(toast_rel-rd_rel-reltablespace))
+		toastTableSpace = toast_rel-rd_rel-reltablespace;
+		relation_close(toast_rel, NoLock);
+	}
 
 	/* create_toast_table does all the work */
-	(void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions);
+	(void) create_toast_table(rel, InvalidOid, InvalidOid, reloptions, toastTableSpace);
 
 	heap_close(rel, NoLock);
 }
@@ -91,7 +102,7 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
 		relName)));
 
 	/* create_toast_table does all the work */
-	if (!create_toast_table(rel, toastOid, toastIndexOid, (Datum) 0))
+	if (!create_toast_table(rel, toastOid, toastIndexOid, (Datum) 0,InvalidOid))
 		elog(ERROR, \%s\ does not require a toast table,
 			 relName);
 
@@ -107,7 +118,7 @@ BootstrapToastTable(char *relName, Oid toastOid, Oid toastIndexOid)
  * bootstrap they can be nonzero to specify hand-assigned OIDs
  */
 static bool
-create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptions)
+create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptions, Oid toastTableSpace)
 {
 	Oid			relOid = RelationGetRelid(rel);
 	HeapTuple	reltup;
@@ -207,10 +218,15 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptio
 		toast_typid = binary_upgrade_next_toast_pg_type_oid;