Re: Patch review for logging hooks (CF 2012-01)

From: Martin Pihlak <martin(dot)pihlak(at)gmail(dot)com>
To: Marti Raudsepp <marti(at)juffo(dot)org>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch review for logging hooks (CF 2012-01)
Date: 2012-01-17 21:07:24
Message-ID: 4F15E30C.2030603@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 01/17/2012 07:35 PM, Marti Raudsepp wrote:
> Here's my review for the "logging hooks" patch queued for the 2012-01
> CommitFest by Martin Pihlak.
>

Thanks for reviewing!

> There's a minor whitespace problem. When declaring variables, and the
> data type is longer than 12 characters, just use 1 single space
> character to delimit the variable name, instead of tab:
>

Fixed both in .h and .c

> I see that other hooks are declared with PGDLLIMPORT. I presume that
> this is necessary on Windows:
> extern PGDLLIMPORT emit_log_hook_type emit_log_hook;

Indeed, fixed now.

> I think the hook warrants a comment that, whether the messages will be
> seen, depends on the log_min_messages setting.
>

Comment added.

regards,
Martin

Attachment Content-Type Size
logging-hooks.patch text/x-patch 1.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marti Raudsepp 2012-01-17 21:40:04 Re: Patch review for logging hooks (CF 2012-01)
Previous Message Kevin Grittner 2012-01-17 20:47:51 Re: [WIP] Double-write with Fast Checksums