Re: Patch to implement pg_current_logfile() function

From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Gilles Darold <gilles(dot)darold(at)dalibo(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to implement pg_current_logfile() function
Date: 2017-02-16 05:55:58
Message-ID: 20170215235558.472eac7c@slate.meme.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Wed, 15 Feb 2017 15:23:00 -0500
Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> + ereport(WARNING,
> + (errcode(ERRCODE_INTERNAL_ERROR),
> + errmsg("corrupted data found in \"%s\"",
> + LOG_METAINFO_DATAFILE)));
>
> elog seems fine here. There's no need for this to be translatable.
> Also, I'd change the level to ERROR.
>
> + errhint("The supported log formats are
> \"stderr\""
> + " and \"csvlog\".")));
>
> I think our preferred style is not to break strings across lines like
> this.
>
> + log_filepath[strcspn(log_filepath, "\n")] = '\0';
>
> We have no existing dependency on strcspn(). It might be better not
> to add one just for this feature; I suggest using strchr() instead,
> which is widely used in our existing code.

Attached is a v29 patch which fixes the above problems.

The Syslogger hunk remains to be fixed. I have no plans
to do so at this time.

Note that since I have to write an "if" anyway, I'm going ahead
and raising an error condition when there's no newline in the
current_logfiles file. The strcspn() ignored the missing newline.
The new code could do so as well by negating the if condition
should that be preferable.

On a different topic, I pulled from master and now
I see that git finds the following untracked:

src/bin/pg_basebackup/pg_receivexlog
src/bin/pg_resetxlog/
src/bin/pg_xlogdump/

I'd appreciate knowing if I'm doing something dumb
on my end to make this happen. Thanks.

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

Attachment Content-Type Size
patch_pg_current_logfile-v29.diff text/x-patch 1.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-02-16 06:05:28 Re: Patch to implement pg_current_logfile() function
Previous Message Tom Lane 2017-02-16 05:53:50 Re: case_preservation_and_insensitivity = on