I'll be responding to a bunch of long review emails in this thread
point by point separately, but just picking out a couple of points
here that jumped out at me:

On Wed, Aug 7, 2019 at 9:18 AM Andres Freund <and...@anarazel.de> wrote:
> > +                     {
> > +                             /*
> > +                              * For the "shared" category, we only discard 
> > when the
> > +                              * rm_undo_status callback tells us we can.
> > +                              */
> Is there a description as to what the rm_status callback is intended to
> do? It currently is mandatory, is that intended?  Why does this only
> apply to shared records? And why just for SHARED, not for any of the others?

Yeah, I will respond to this.  After recent discussions with Robert
the whole UNDO_SHARED concept looks a bit shaky, and there's a better
way trying to get out -- more on that soon.

> > See
> > +                              * DiscardWorkerMain.
> Hm. This actually reminds me of a complaint I have about this. ISTM that
> the logic for discarding itself should be separate from the discard
> worker. I'd just add that, and a UDF to invoke it, in a separate commit.

That's not a bad idea -- I have a 'pg_force_discard()' SP which I'll
include in my next patchset, which is a bit raw, which I'm planning to
make a bit smarter -- it might make sense to use the same code path
for that.

Thomas Munro

Reply via email to