Re: [HACKERS] Patch to implement pg_current_logfile() function

On January 15, 2017 11:47:51 PM CST, Michael Paquier  wrote:
>On Mon, Jan 16, 2017 at 1:14 PM, Karl O. Pinc  wrote:
>
>
>> Attached also are 2 patches which abstract some hardcoded
>> constants into symbols.  Whether to apply them is a matter
>> of style and left to the maintainers, which is why these
>> are separate patches.
>
>Making the strings csvlog, stderr and eventlog variables? Why not
>because the patch presented here uses them in new places. Now it is
>not like it is a huge maintenance burden to keep them as strings, so I
>would personally not bother much.

To my mind it's a matter readability. It is useful to be able to search for or identify quickly when reading, e. g., all the places where the keyword stderr relates to output log destination and not some other more common use. The downside is it is more code...


>OK. I have done a round of hands-on review on this patch, finishing
>with the attached. In the things I have done:

Thank you.

>With all those issues fixed, I finish with the attached, that I am
>fine to pass down to a committer. I still think that this should be
>only one function using a SRF with rows shaped as (type, path) for
>simplicity, but as I am visibly outnumbered I won't insist further.

That also makes a certain amount of sense to me but I can't say I have thought deeply about it. Haven't paid any attention to this issue and am fine letting things move forward as is.

>Also, I would rather see an ERROR returned to the user in case of bad
>data in current_logfiles, I did not change that either as that's the
>original intention of Gilles.

I could be wrong but I seem to recall that Robert recommended against an error message. If there is bad data then something is really wrong, up to some sort of an attack on the back end. Because this sort of thing simply shouldn't happen it's enough in my opinion to guard against buffer overruns etc and just get on with life. If something goes unexpectedly and badly wrong with internal data structures in general there would have to be all kinds of additional code to cover every possible problem and that doesn't seem to make sense.

Sorry about the previous email with empty content. My email client got away from me.


Karl 
Free  Software:  "You don't pay back you pay forward."               -- Robert A. Heinlein