Re: Review: plpgsql.extra_warnings, plpgsql.extra_errors - Mailing list pgsql-hackers

From Pavel Stehule
Subject Re: Review: plpgsql.extra_warnings, plpgsql.extra_errors
Date
Msg-id CAFj8pRAap=XSZf4Z0iOHG90SJN4+CrQgXFQyCxH7Y1RZmJeY+Q@mail.gmail.com
Whole thread Raw
In response to Re: Review: plpgsql.extra_warnings, plpgsql.extra_errors  (Pavel Stehule <pavel.stehule@gmail.com>)
List pgsql-hackers



2014-03-23 15:14 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.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@2ndquadrant.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


pgsql-hackers by date:

Previous
From: Pavel Stehule
Date:
Subject: Re: Review: plpgsql.extra_warnings, plpgsql.extra_errors
Next
From: Petr Jelinek
Date:
Subject: Re: Review: plpgsql.extra_warnings, plpgsql.extra_errors