Re: Review: plpgsql.extra_warnings, plpgsql.extra_errors

Review shadow_v6 patch

Hello

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
better?

Regards

Pavel

2014-03-20 12:39 GMT+01:00 Petr Jelinek <petr(at)2ndquadrant(dot)com>:

> 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 http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services
>