On Tue, Jul 22, 2014 at 12:01 PM, Fabrízio de Royes Mello <fabriziomello@gmail.com> wrote: > > > > On Thu, Jul 17, 2014 at 8:02 PM, Andres Freund <andres@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.
