On Sun, Dec 08, 2019 at 02:14:03PM -0500, Tom Lane wrote:
> That is, these arguments *used* to be a different LSN pointer, and that
> commit changed them to be mostly equal to RedoRecPtr, and made what
> seems like a not very well-advised renaming to go with that.
Indeed. That part was ill-thought.
>> So it might make sense to remove the parameter from this
>> function, too, and replace it with a flag parameter named
>> something like "is_valid", or perhaps split the function
>> into two functions, one for valid and one for invalid.
>
> Don't think I buy that. The fact that these arguments were until recently
> different from RedoRecPtr suggests that they might someday be different
> again, whereupon we'd have to laboriously revert such a parameter redesign.
> I think I'd just go for names that don't have a hard implication that
> the parameter values are the same as any particular global variable.
Yeah, those APIs may have a slightly different meaning in the future,
so I agree that it makes the most sense to rename the variables of the
functions from RedoRecPtr to lastRedoPtr to outline the fact that we
are referring to the redo LSN of the last checkpoint. Attached is a
patch for that. I'd rather back-patch that to avoid any conflicts
when working on bug fixes for stable branches. Thoughts?
--
Michael