Re: Patch to implement pg_current_logfile() function

From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(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-16 04:14:51
Message-ID: 20170115221451.12927124@slate.meme.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Sun, 15 Jan 2017 07:20:40 -0600
"Karl O. Pinc" <kop(at)meme(dot)com> wrote:

> On Sun, 15 Jan 2017 14:54:44 +0900
> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
>
> > I think that's all I have for this patch.

> I'd like to submit with it an addendum patch that
> makes symbols out of some constants. I'll see if I can
> get that done soon.

Attached is a version 23 of the patch. The only change
is follow-through and cleanup of code posted in-line in emails.

src/backend/utils/adt/misc.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)

This includes making comments into full sentences.

I do have a question here regards code formatting.
The patch now contains:

if (log_filepath == NULL)
{
/* Bad data. Avoid segfaults etc. and return NULL to caller. */
break;
}

I'm not sure how this fits in with PG coding style,
whether the {} should be removed or what. I've looked
around and can't find an example of an if with a single
line then block and a comment. Maybe this means that
I shouldn't be doing this, but if I put the comment above
the if then it will "run into" the comment block which
immediately precedes the above code which describes
a larger body of code. So perhaps someone should look
at this and tell me how to improve it.

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.

The first patch changes only code on the master
branch, the 2nd patch changes the new code.

I have not looked further at the patch or checked
to see that all changes previously recommended have been
made. Michael, if you're confident that everything is good
go ahead and move the patchs forward to the maintainers. Otherwise
please let me know and I'll see about finding time
for further review.

Regards,

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-v23.diff text/x-patch 15.5 KB
patch_pg_current_logfile-v23.diff.abstract_guc_part1 application/octet-stream 2.6 KB
patch_pg_current_logfile-v23.diff.abstract_guc_part2 application/octet-stream 2.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Khandekar 2017-01-16 04:19:04 Re: Parallel Append implementation
Previous Message Andres Freund 2017-01-16 03:29:52 Re: Implement targetlist SRFs using ROWS FROM() (was Changed SRF in targetlist handling)