On Wed, Jul 23, 2014 at 5:48 PM, Fabrízio de Royes Mello < fabriziome...@gmail.com> wrote: > > > On Tue, Jul 22, 2014 at 3:29 PM, Fabrízio de Royes Mello < fabriziome...@gmail.com> wrote: > > > > On Tue, Jul 22, 2014 at 12:01 PM, Fabrízio de Royes Mello < fabriziome...@gmail.com> wrote: > > > > > > > > > > > > On Thu, Jul 17, 2014 at 8:02 PM, Andres Freund <and...@2ndquadrant.com> wrote: > > > > > > > > > That means should I "FlushRelationBuffers(rel)" before change the > > > > > relpersistence? > > > > > > > > I don't think that'd help. I think what this means that you simply > > > > cannot change the relpersistence of the old relation before the heap > > > > swap is successful. So I guess it has to be something like (pseudocode): > > > > > > > > OIDNewHeap = make_new_heap(..); > > > > newrel = heap_open(OIDNewHeap, AEL); > > > > > > > > /* > > > > * Change the temporary relation to be unlogged/logged. We have to do > > > > * that here so buffers for the new relfilenode will have the right > > > > * persistency set while the original filenode's buffers won't get read > > > > * in with the wrong (i.e. new) persistency setting. Otherwise a > > > > * rollback after the rewrite would possibly result with buffers for the > > > > * original filenode having the wrong persistency setting. > > > > * > > > > * NB: This relies on swap_relation_files() also swapping the > > > > * persistency. That wouldn't work for pg_class, but that can't be > > > > * unlogged anyway. > > > > */ > > > > AlterTableChangeCatalogToLoggedOrUnlogged(newrel); > > > > FlushRelationBuffers(newrel); > > > > /* copy heap data into newrel */ > > > > finish_heap_swap(); > > > > > > > > And then in swap_relation_files() also copy the persistency. > > > > > > > > > > > > That's the best I can come up right now at least. > > > > > > > > > > Isn't better if we can set the relpersistence as an argument to "make_new_heap" ? > > > > > > I'm thinking to change the make_new_heap: > > > > > > From: > > > > > > make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, bool forcetemp, > > > LOCKMODE lockmode) > > > > > > To: > > > > > > make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, char relpersistence, > > > LOCKMODE lockmode) > > > > > > That way we can create the new heap with the appropriate relpersistence, so in the swap_relation_files also copy the persistency, of course. > > > > > > And after copy the heap data to the new table (ATRewriteTable) change relpersistence of the OldHeap's indexes, because in the "finish_heap_swap" they'll be rebuild. > > > > > > Thoughts? > > > > > > > The attached patch implement my previous idea based on Andres thoughts. > > > > I don't liked the last version of the patch, especially this part: > > + /* check if SetUnlogged or SetLogged exists in subcmds */ > + for(pass = 0; pass < AT_NUM_PASSES; pass++) > + { > + List *subcmds = tab->subcmds[pass]; > + ListCell *lcmd; > + > + if (subcmds == NIL) > + continue; > + > + foreach(lcmd, subcmds) > + { > + AlterTableCmd *cmd = (AlterTableCmd *) lfirst(lcmd); > + > + if (cmd->subtype == AT_SetUnLogged || cmd->subtype == AT_SetLogged) > + { > + /* > + * Change the temporary relation to be unlogged/logged. We have to do > + * that here so buffers for the new relfilenode will have the right > + * persistency set while the original filenode's buffers won't get read > + * in with the wrong (i.e. new) persistency setting. Otherwise a > + * rollback after the rewrite would possibly result with buffers for the > + * original filenode having the wrong persistency setting. > + * > + * NB: This relies on swap_relation_files() also swapping the > + * persistency. That wouldn't work for pg_class, but that can't be > + * unlogged anyway. > + */ > + if (cmd->subtype == AT_SetUnLogged) > + newrelpersistence = RELPERSISTENCE_UNLOGGED; > + > + isSetLoggedUnlogged = true; > + } > + } > + } > > > So I did a refactoring adding new items to AlteredTableInfo to pass the information through the phases. >
Hi all, There are something that should I do on this patch yet? Regards -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog sobre TI: http://fabriziomello.blogspot.com >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello