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 (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers