Re: REVIEW: PL/Python validator function

2011/1/18 Jan Urbański <wulczer(at)wulczer(dot)org>:
> On 17/01/11 09:26, Jan Urbański wrote:
>> On 17/01/11 01:02, Hitoshi Harada wrote:
>>> This is a review for the patch sent as
>>> https://commitfest.postgresql.org/action/patch_view?id=456
>>> It includes adequate amount of test. I found regression test failure
>>> in plpython_error.
>>
>>> My environment is CentOS release 5.4 (Final) with python 2.4.3
>>> installed default.
>
> Seems that somewhere between Python 2.4 and Python 2.6 the whole module
> that was providing SyntaxError got rewritten and the way a syntax error
> from Py_CompileString is reported changed :( I tried some tricks but in
> the end I don't think it's worth it: I just added an alternative
> regression output file for older Pythons.
>
>>> It looks fine overall. The only thing that I came up with is trigger
>>> check logic in PLy_procedure_is_trigger. Although it seems following
>>> plperl's corresponding function, the check of whether the prorettype
>>> is pseudo type looks redundant since it checks prorettype is
>>> TRIGGEROID or OPAQUEOID later. But it is not critical.
>>
>> Yes, you're right, a check for prorettype only should be sufficient. Wil
>> fix.
>
> I removed the test for TYPTYPE_PSEUDO in the is_trigger function.
>
> Updated patch attached.

Thanks. I tested the new version and looks ok. I'll mark it "Ready for
Commiter".

Regards,

--
Hitoshi Harada