On Thu, Aug 21, 2014 at 10:26 PM, Fabrízio de Royes Mello <
fabriziomello@gmail.com> wrote:
>
>
> On Thu, Aug 21, 2014 at 8:04 PM, Alvaro Herrera <
alvherre@2ndquadrant.com> wrote:
> > Andres Freund wrote:
> >
> > > Have you looked at the correctness of the patch itself? Last time I'd
> > > looked it has fundamental correctness issues. I'd outlined a possible
> > > solution, but I haven't looked since.
> >
> > Yeah, Fabrizio had it passing the relpersistence down to make_new_heap,
> > so the transient table is created with the right setting. AFAICS it's
> > good now. I'm a bit uneasy about the way it changes indexes: it just
> > updates pg_class for them just before invoking the reindex in
> > finish_heap_swap. I think it's correct as it stands though; at least,
> > the rewrite phase happens with the right setting, so that if there are
> > constraints being checked and these invoke index scans, such accesses
> > would not leave buffers with the wrong setting in shared_buffers.
> >
>
> Ok.
>
>
> > Another option would be to pass the new relpersistence down to
> > finish_heap_swap, but that would be hugely complicated AFAICS.
> >
>
> I think isn't so complicated to do it, but will this improve something ?
> Maybe I didn't understand it very well. IMHO it just complicate a
> simple thing.
>
>
>
> > Here's the updated patch.
> >
>
> Thanks Alvaro!
>
I forgot to mention... I did again a lot of tests using different replication scenarios to make sure all is ok:
