Re: Patch to implement pg_current_logfile() function

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
Cc: Gilles Darold <gilles(dot)darold(at)dalibo(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to implement pg_current_logfile() function
Date: 2017-01-17 02:22:45
Message-ID: CAB7nPqRNO3ni0bnF7iE-=TatMejPU3DLBHZqkSV59w8NjBq2Tw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jan 17, 2017 at 2:21 AM, Karl O. Pinc <kop(at)meme(dot)com> wrote:
> On January 15, 2017 11:47:51 PM CST, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
>>On Mon, Jan 16, 2017 at 1:14 PM, Karl O. Pinc <kop(at)meme(dot)com> wrote:
>>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.

Gilles, what's your opinion here? As the author that's your call at
the end. What the point here is would be to change
pg_current_logfiles() to something like that:
=# SELECT * FROM pg_current_logfiles();
method | file
--------|--------
stderr | pg_log/postgresql.log
csvlog | pg_log/postgresql.csv
And using this SRF users can filter the method with a WHERE clause.
And as a result the 1-arg version is removed. No rows are returned if
current_logfiles does not exist. I don't think there is much need for
a system view either.

>>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.

Hm... OK. At the same time not throwing at least a WARNING is
confusing, because a NULL result is returned as well even if the input
method is incorrect and even if the method is correct but it is not
present in current_logfiles. As the user is thought as a trusted user
as it has access to this function, I would think that being verbose on
the error handling, or at least warnings, would make things easier to
analyze.

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

No problem. That happens.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2017-01-17 02:28:32 Re: Packages: Again
Previous Message Peter Geoghegan 2017-01-17 01:56:10 Re: Polyphase merge is obsolete