Re: Backpatch b61d161c14 (Introduce vacuum errcontext ...)

Hi,

On 2020-06-23 00:14:40 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I am only suggesting that where you save the old location, as currently
> > done with LVRelStats olderrinfo, you instead use a more specific
> > type. Not that you should pass that anywhere (except for
> > update_vacuum_error_info).
> 
> As things currently stand, I don't think we need another struct
> type at all.  ISTM we should hard-wire the handling of indname
> as I suggested above.  Then there are only two fields to be dealt
> with, and we could just as well save them in simple local variables.

That's fine with me too.


> If there's a clear future path to needing to save/restore more
> fields, then maybe another struct type would be useful ... but
> right now the struct type declaration itself would take more
> lines of code than it would save.

FWIW, I started to be annoyed by this code when I was addding
prefetching support to vacuum, and wanted to change what's tracked
where. The number of places that needed to be touched was
disproportional.


Here's a *draft* for how I thought this roughly could look like. I think
it's nicer to not specify the exact saved state in multiple places, and
I think it's much clearer to use a separate function to restore the
state than to set a "fresh" state.

I've only applied a hacky fix for the way the indname is tracked, I
thought that'd best be discussed separately.

Greetings,

Andres Freund