patch : Allow toast tables to be moved to a different tablespace

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

On Fri, Oct 7, 2011 at 10:10 AM, Julien Tachoires <julmon@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

2011/11/15 Jaime Casanova <jaime@2ndquadrant.com>:

On Fri, Oct 7, 2011 at 10:10 AM, Julien Tachoires <julmon@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?

Hi Jaime,

New patch is attached.

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

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

Regards,

--
JT

On Tue, Nov 15, 2011 at 11:08 AM, Julien Tachoires <julmon@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

On Tue, Nov 15, 2011 at 11:08 AM, Julien Tachoires <julmon@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

Hi Jaime,

Please find a new version.

2011/11/16 Jaime Casanova <jaime@2ndquadrant.com>:

On Tue, Nov 15, 2011 at 11:08 AM, Julien Tachoires <julmon@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

Fixed.

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

Fixed.

On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires <julmon@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

Hi,

2011/12/10 Jaime Casanova <jaime@2ndquadrant.com>:

On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires <julmon@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,

On Sat, Dec 10, 2011 at 4:16 PM, Jaime Casanova <jaime@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

On Mon, Dec 12, 2011 at 10:54 AM, Julien Tachoires <julmon@gmail.com> wrote:

Hi,

2011/12/10 Jaime Casanova <jaime@2ndquadrant.com>:

On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires <julmon@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

2011/12/13 Jaime Casanova <jaime@2ndquadrant.com>:

On Mon, Dec 12, 2011 at 10:54 AM, Julien Tachoires <julmon@gmail.com> wrote:

2011/12/10 Jaime Casanova <jaime@2ndquadrant.com>:

On Mon, Nov 28, 2011 at 1:32 PM, Julien Tachoires <julmon@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,

On Tue, Dec 13, 2011 at 12:02 PM, Julien Tachoires <julmon@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

2011/12/13 Robert Haas <robertmhaas@gmail.com>:

On Tue, Dec 13, 2011 at 12:02 PM, Julien Tachoires <julmon@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 :(

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

2011/12/13 Robert Haas<robertmhaas@gmail.com>:

On Tue, Dec 13, 2011 at 12:02 PM, Julien Tachoires<julmon@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 US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us

Excerpts from Julien Tachoires's message of mar dic 13 14:29:56 -0300 2011:

2011/12/13 Robert Haas <robertmhaas@gmail.com>:

On Tue, Dec 13, 2011 at 12:02 PM, Julien Tachoires <julmon@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 <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Tue, Dec 13, 2011 at 12:29 PM, Julien Tachoires <julmon@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

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 <julmon@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 <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Hi,

2012/1/16 Alvaro Herrera <alvherre@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 <julmon@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.

2011/12/15 Alvaro Herrera <alvherre@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.

Regards,

--
JT