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.
Cheers,
Jan
