On Mon, Feb 8, 2016 at 6:18 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2016-02-08 15:58:49 +0900, Michael Paquier wrote:
>> On Sun, Feb 7, 2016 at 2:49 AM, Andres Freund <andres@anarazel.de> wrote:
>> /*
>> + * XLogInsert
>> + *
>> + * A shorthand for XLogInsertExtended, to update the progress of WAL
>> + * activity by default.
>> + */
>> +XLogRecPtr
>> +XLogInsert(RmgrId rmid, uint8 info)
>> +{
>> + return XLogInsertExtended(rmid, info, true);
>> +}
>> +
>> +/*
>> + * XLogInsertExtended
>
> I'm not really a fan. I'd rather change existing callers to add a
> 'flags' bitmask argument. Right now there can't really be XLogInserts()
> in extension code, so that's pretty ok to change.
So, you mean to extend directly XLogInsert() with an int flag that has
only one possible value for now, and update *all* the calls of
XLogInsert(). Well, I am detecting 218 calls of XLogInsert() in the
code, so that's not a small change. Honestly, I'd really rather have
the Extended() routine with default taking 0 for the integer flag, 0
meaning in the case of the progress that it gets updated. We have
similar routines for RangeVar, some in bufmgr.c and some other places.
So, which way do you want to take? I don't expect to win this argument
if that's a 1vs1 against a committer, just I would like to be sure
that I am not doing useless things. Time matters.
--
Michael