From: | Hitoshi Harada <umi(dot)tanuki(at)gmail(dot)com> |
---|---|
To: | Jan Urbański <wulczer(at)wulczer(dot)org> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: REVIEW: PL/Python validator function |
Date: | 2011-01-19 01:16:26 |
Message-ID: | AANLkTi=2jkSLtNcfH=EhpcjwXUMPoAQyzabQrKkaM1a1@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2011-01-19 02:51:10 | Re: auto-sizing wal_buffers |
Previous Message | Alvaro Herrera | 2011-01-19 01:07:28 | Re: log_hostname and pg_stat_activity |