Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

Thanks Steve, that's exactly what I wanted to send when you sent your patches :)

I need to figure out why that patch v2 worked for me, I think I made mess somewhere in my git repo and didn't create the patch properly. Sorry for that.

Patch is attached, I've also added information about cdecimal to plpython documentation.

I'm just wondering how to make integration tests to check when cdecimal is installed and when it is not.

thanks,

Szymon

On 26 June 2013 10:12, Ronan Dunklau <rdunklau@gmail.com> wrote:

The v2 patch does not work for me: regression tests for plpython fail on the plpython_types test: every numeric is converted to None.

It seems the global decimal ctor is not initialized.

Please find two patches, to be applied on top of the v2 patch: one initializes the decimal ctor, the other uses cdecimal when possible.

Using the performance test from steve, on my machine:

- with cdecimal installed: ~84ms
- without cdecimal installed (standard decimal module): ~511ms

2013/6/26 Szymon Guz <mabewlun@gmail.com>

On 26 June 2013 01:40, Steve Singer <steve@ssinger.info> wrote:

On 06/25/2013 06:42 AM, Szymon Guz wrote:

Hi,

I've attached a new patch. I've fixed all the problems you've found, except for the efficiency problem, which has been described in previous email.

thanks,
Szymon

This version of the patch addresses the issues I mentioned.  Thanks for looking into seeing if the performance issue is with our conversions to strings or inherit with the python decimal type.  I guess we (Postgresql) can't do much about it.   A runtime switch to use cdecimal if it is available is a good idea, but I agree with you that could be a different patch.

One minor thing I noticed in this round,

 PLy_elog(ERROR, "could not import module 'decimal'");

I think should have "decimal" in double-quotes.

I think this patch is ready for a committer to look at it.

Steve

Hi Steve,

thanks for the review.

I was thinking about speeding up the Decimal conversion using the module you wrote about. What about trying to import it, if it fails, than trying to load decimal.Decimal? There will be no warning in logs, just additional information in documentation that it uses this module if it is available?

thanks,

Szymon