On Wed, Dec 29, 2010 at 15:05, Gurjeet Singh <singh.gurjeet@gmail.com> wrote:
> On Wed, Dec 29, 2010 at 5:09 AM, Magnus Hagander <magnus@hagander.net>
> wrote:
>>
>> > Ok, here's an updated patch that does both these and includes
>> > documentation and regression test changes. With that, I think we're
>> > good to go.
>>
>> I've applied this version (with some minor typo-fixes).
>>
>
> Do you think we could have worded these a bit better
>
> <entry>Prepare for performing on-line backup (restricted to superusers or
> replication roles)</entry>
>
> to say 'restricted to superusers _and_ replication roles'.
>
> Saying 'restricted to superusers _or_ replication roles' may mean that at
> any time we allow one or the other, but not both (reader might assume that
> decision is based on some other GUC).
Uh, not sure, actually. I would read the "and" as meaning you needed
*both*, which isn't true. We do allow, at any time, one or the other -
*or* both.
> Any specific reason NOREPLICATION_P and REPLICATION_P use the _P suffix?
Um, I just copied it off a similar entry elsewhere. I saw no comment
about what _P actually means, and I can't say I know. I know very
little about the bison files :-)
> AIUI, that suffix is used in gram.y to tag a token to mean it belongs to
> Parser, and to avoid conflict with the same token elsewhere; NULL_P is a
> good example.
>
> In pg_authid.h, 8 spaces used between 'bool' and 'rolreplication', instead
> tabs should have been used as the surrounding code.
Bleh. Well, pgindent will fix that.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/