On Thu, Nov 14, 2013 at 8:46 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> On 2013-11-12 18:50:33 +0100, Andres Freund wrote:
>> > You've actually changed the meaning of this section (and not in a good way):
>> >
>> > be set at server start. <varname>wal_level</> must be set
>> > - to <literal>archive</> or <literal>hot_standby</> to allow
>> > - connections from standby servers.
>> > + to <literal>archive</>, <literal>hot_standby</> or <literal>logical</>
>> > + to allow connections from standby servers.
>> >
>> > I think that the previous text meant that you needed archive - or, if
>> > you want to allow connections, hot_standby. The new text loses that
>> > nuance.
>>
>> Yea, that's because it was lost on me in the first place...
>
> I think that's because the nuance isn't actually in the text - note that
> it is talking about max_wal_senders and talking about connections
> *from*, not *to* standby servers.
> I've reformulated the wal_level paragraph and used "or higher" in
> several places now.
>
> Ok, so here's a rebased version of this. I tried to fix all the issues
> you mentioned, and it's based on the split off IsSystemRelation() patch,
> I've sent yesterday (included here).
OK, I've committed the patch to adjust the definition of
IsSystemRelation()/IsSystemClass() and add
IsCatalogRelation()/IsCatalogClass(). I kibitzed your decision about
which function to use in a few places - specifically, I made all of
the places that cared about allow_system_table_mods uses the IsSystem
functions, and all the places that cared about invalidation messages
use the IsCatalog functions. I don't think any of these changes are
more cosmetic, but I think it may reduce the chance of errors or
inconsistencies in the face of future changes.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
