On Tue, Feb 9, 2016 at 5:06 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Fabien COELHO wrote:
>>
>> Hello,
>>
>> >>v23 attached, which does not change the message but does the other fixes.
>> >
>> >This doesn't apply anymore
>>
>> Indeed, but the latest version was really v25.
>>
>> >-- please rebase and submit to the next CF.
>>
>> I already provided it as v25 on Feb 1st.
>>
>> >I closed it here as returned with feedback.
>>
>> I turned it to "moved to next CF" as the patch is already on the queue.
>
> Ah, I didn't see v25 anywhere. What you did should be fine, thanks.
I just had another look at this patch.
+ <replaceable>parameter</>% of the time.
Nitpick: double space here.
+ switch (func) {
[...]
+ }
+ default:
+ return false; }
In evalFunc(), the default case in switch for the operator functions
should never be reached. Adding for example Assert(0) is something to
consider.
PGBT_NONE and PGBENCH_NONE are used nowhere. Why not removing them or
replace the code paths where they would be used by assertions to
trigger errors for future developments?
Other than that the patch looks in pretty good shape to me.
--
Michael
