Review shadow_v6 patch


I did a recheck a newest version of this patch:

1. There is a wide agreement on implemented feature - nothing changed from
previous review - it is not necessary comment it again.

2. v6 patch: patching cleanly, compilation without errors and warnings, all
regress tests passed

Tom's objections was related to GUC part. It is redesigned as Tom proposed.

The code is good - and I don't see any problem there.

I have only one objection - What I remember - more usual is using a list
instead a bitmap for these purposes - typical is DefElem struct. Isn't it



2014-03-20 12:39 GMT+01:00 Petr Jelinek <>:

> On 20/03/14 00:32, Tom Lane wrote:
>> TBH, if I thought this specific warning was the only one that would ever
>> be there, I'd probably be arguing to reject this patch altogether.
> Of course, nobody assumes that it will be the only one.
>> Also, adding GUC_LIST_INPUT later is not really cool since it changes
>> the parsing behavior for the GUC.  If it's going to be a list, it should
>> be one from day zero.
> Actually it does not since it all has to be handled in check/assign hook
> anyway.
> But nevertheless, I made V6 with doc change suggested by Alvaro and also
> added this list handling framework for the GUC params.
> In the end it is probably less confusing now that the implementation uses
> bitmask instead of bool when the user facing functionality talks about
> list...
> This obviously needs code review again (I haven't changed tests since
> nothing changed from user perspective).
> --
>  Petr Jelinek        
>  PostgreSQL Development, 24x7 Support, Training & Services

Reply via email to