Re: Review: plpgsql.extra_warnings, plpgsql.extra_errors

2014-03-23 15:14 GMT+01:00 Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>:

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

A using DefElem will be longer, but it is typical pattern for this case in
Postgres.

What is opinion of other hackers?

Pavel

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