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

Andreas Karlsson <andreas(at)proxel(dot)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