Hi!
I've watched over the patch and consider it useful. Applies without conflicts. The functionality of the patch itself is
meets declared functionality.
But, in my view, some improvements may be proposed. We should be more, let's say, cautious (or a paranoid if you wish),
in pg_walfile_offset_lsn while dealing with user input values. What I mean by that is:
- to init input vars of sscanf, i.e. tli, log andseg;
- check the return value of sscanf call;
- check filename max length.
Another question arises for me: is this function can be called during recovery? If not, we should ereport about this, should we?
And one last note here: pg_walfile_offset_lsn is accepting NULL values and return NULL in this case. From a theoretical
point of view, this is perfectly fine. Actually, I think this is exactly how it supposed to be, but I'm not sure if there are no other opinions here.
--
Best regards,
Maxim Orlov.
