[PATCH] Fix conversion for Decimal arguments in plpython functions

Started by Szymon Guzover 12 years ago25 messages
#1Szymon Guz
mabewlun@gmail.com
1 attachment(s)

Hi,
I've got a patch.

This is for a plpython enhancement.

There is an item at the TODO list
http://wiki.postgresql.org/wiki/Todo#Server-Side_Languages
"Fix loss of information during conversion of numeric type to Python float"

This patch uses a decimal.Decimal type from Python standard library for the
plpthon function numeric argument instead of float.

Patch contains changes in code, documentation and tests.

Most probably there is something wrong, as this is my first Postgres patch
:)

thanks,
Szymon

Attachments:

plpython_decimal.patchapplication/octet-stream; name=plpython_decimal.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index aaf758d..7e55b75 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -312,10 +312,8 @@ $$ LANGUAGE plpythonu;
       <para>
        PostgreSQL <type>real</type>, <type>double</type>,
        and <type>numeric</type> are converted to
-       Python <type>float</type>.  Note that for
-       the <type>numeric</type> this loses information and can lead to
-       incorrect results.  This might be fixed in a future
-       release.
+       Python <type>Decimal</type>. This type is imported from
+       <literal>decimal.Decimal</literal>.
       </para>
      </listitem>
 
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 4641345..2c4bb6a 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -216,31 +216,39 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <type 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <type 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <type 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <type 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 0dad843..ffce97d 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -77,6 +77,9 @@ static const int plpython_python_version = PY_MAJOR_VERSION;
 /* initialize global variables */
 PyObject   *PLy_interp_globals = NULL;
 
+/* global pointer to decimal.Decimal costructor */
+PyObject   *PLy_decimal_ctor_global = NULL;
+
 /* this doesn't need to be global; use PLy_current_execution_context() */
 static PLyExecutionContext *PLy_execution_contexts = NULL;
 
@@ -147,9 +150,23 @@ PLy_init_interp(void)
 	if (PLy_interp_safe_globals == NULL)
 		PLy_elog(ERROR, "could not create globals");
 	PyDict_SetItemString(PLy_interp_globals, "GD", PLy_interp_safe_globals);
-	Py_DECREF(mainmod);
+
 	if (PLy_interp_globals == NULL || PyErr_Occurred())
 		PLy_elog(ERROR, "could not initialize globals");
+
+	PyObject *decimal = PyImport_ImportModule("decimal");
+	if (decimal == NULL)
+		PLy_elog(ERROR, "could not import module 'decimal'");
+
+	PyObject *decimal_dict = PyModule_GetDict(decimal);
+	if (decimal_dict == NULL)
+		PLy_elog(ERROR, "could not get decimal dict from imported module");
+
+	PLy_decimal_ctor_global = PyDict_GetItemString(decimal_dict, "Decimal");
+	if (PLy_decimal_ctor_global == NULL || !PyCallable_Check(PLy_decimal_ctor_global))
+		PLy_elog(ERROR, "could not get decimal consctructor for Decimal type");
+
+	Py_DECREF(mainmod);
 }
 
 Datum
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 8f2367d..83e0e50 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -18,6 +18,7 @@
 #include "utils/memutils.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
+#include "utils/numeric.h"
 
 #include "plpython.h"
 
@@ -26,6 +27,7 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 
+extern PyObject *PLy_decimal_ctor_global;
 
 /* I/O function caching */
 static void PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup);
@@ -35,7 +37,7 @@ static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup);
 static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
-static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
+static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
@@ -450,7 +452,7 @@ PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup)
 			arg->func = PLyFloat_FromFloat8;
 			break;
 		case NUMERICOID:
-			arg->func = PLyFloat_FromNumeric;
+			arg->func = PLyDecimal_FromNumeric;
 			break;
 		case INT2OID:
 			arg->func = PLyInt_FromInt16;
@@ -516,16 +518,12 @@ PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d)
 }
 
 static PyObject *
-PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
+PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
 {
-	/*
-	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
-	 * it be better to cast to PyString?
-	 */
-	Datum		f = DirectFunctionCall1(numeric_float8, d);
-	double		x = DatumGetFloat8(f);
-
-	return PyFloat_FromDouble(x);
+	char *x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
+	PyObject *pvalue = PyString_FromString(x);
+	PyObject *value = PyObject_CallFunctionObjArgs(PLy_decimal_ctor_global, pvalue, NULL);
+	return value;
 }
 
 static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 6a50b42..e35421f 100644
--- a/src/pl/plpython/sql/plpython_types.sql
+++ b/src/pl/plpython/sql/plpython_types.sql
@@ -90,10 +90,11 @@ plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
 
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
 SELECT * FROM test_type_conversion_numeric(-100);
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
 SELECT * FROM test_type_conversion_numeric(null);
 
 
#2Steve Singer
steve@ssinger.info
In reply to: Szymon Guz (#1)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 05/28/2013 04:41 PM, Szymon Guz wrote:

Hi,
I've got a patch.

This is for a plpython enhancement.

There is an item at the TODO list
http://wiki.postgresql.org/wiki/Todo#Server-Side_Languages
"Fix loss of information during conversion of numeric type to Python
float"

This patch uses a decimal.Decimal type from Python standard library
for the plpthon function numeric argument instead of float.

Patch contains changes in code, documentation and tests.

Most probably there is something wrong, as this is my first Postgres
patch :)

Thanks for contributing.

This patch applies cleanly against master and compiles with warnings

plpy_main.c: In function ‘PLy_init_interp’:
plpy_main.c:157:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]
plpy_main.c:161:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-statement]

You can avoid this by moving the declaration of decimal and decimal_dict
to be at the top of the function where mainmod is declared.

Also in this function you've introduced places where it returns with an
error (the PLy_elog(ERROR...) calls before decrementing the reference to
mainmod. I think you can decrement the mainmod reference after the call
to SetItemString before your changes that import the Decimal module.

The patch works as expected, I am able to write python functions that
take numerics as arguments and work with them. I can adjust the decimal
context precision inside of my function.

One concern I have is that this patch makes pl/python functions
involving numerics more than 3 times as slow as before.

create temp table b(a numeric);
insert into b select generate_series(1,10000);

create or replace function x(a numeric,b numeric) returns numeric as $$
if a==None:
return b
return a+b
$$ language plpythonu;
create aggregate sm(basetype=numeric, sfunc=x,stype=numeric);

test=# select sm(a) from b;
sm
----------
50005000
(1 row)

Time: 565.650 ms

versus before the patch this was taking in the range of 80ms.

Would it be faster to call numeric_send instead of numeric_out and then
convert the sequence of Int16's to a tuple of digits that can be passed
into the Decimal constructor? I think this is worth trying and testing,

Documentation
=================
Your patched version of the docs say

PostgreSQL <type>real</type>, <type>double</type>, and
<type>numeric</type> are converted to
Python <type>Decimal</type>. This type is imported
from<literal>decimal.Decimal</literal>.

I don't think this is correct, as far as I can tell your not changing
the behaviour for postgresql real and double types, they continue to use
floating point.

<listitem>
<para>
PostgreSQL <type>real</type> and <type>double</type>are converted to
Python <type>float</type>.
</para>
</listitem>

<listitem>
<para>
PostgreSQL <type>numeric</type> is converted to
Python <type>Decimal</type>. This type is imported from
<literal>decimal.Decimal</literal>.
</para>
</listitem>

Maybe?

Steve

thanks,
Szymon

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Szymon Guz
mabewlun@gmail.com
In reply to: Steve Singer (#2)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 25 June 2013 05:16, Steve Singer <steve@ssinger.info> wrote:

One concern I have is that this patch makes pl/python functions involving
numerics more than 3 times as slow as before.

create temp table b(a numeric);
insert into b select generate_series(1,10000);

create or replace function x(a numeric,b numeric) returns numeric as $$
if a==None:
return b
return a+b
$$ language plpythonu;
create aggregate sm(basetype=numeric, sfunc=x,stype=numeric);

test=# select sm(a) from b;
sm
----------
50005000
(1 row)

Time: 565.650 ms

versus before the patch this was taking in the range of 80ms.

Would it be faster to call numeric_send instead of numeric_out and then
convert the sequence of Int16's to a tuple of digits that can be passed
into the Decimal constructor? I think this is worth trying and testing,

Hi,
thanks for all the remarks.

I think I cannot do anything about speeding up the code. What I've found so
far is:

I cannot use simple fields from NumericVar in my code, so to not waste time
on something not sensible, I've tried to found out if using the tuple
constructor for decimal.Decimal will be faster. I've changed the function
to something like this:

static PyObject *
PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
{
PyObject *digits = PyTuple_New(4);
PyTuple_SetItem(digits, 0, PyInt_FromLong(1));
PyTuple_SetItem(digits, 1, PyInt_FromLong(4));
PyTuple_SetItem(digits, 2, PyInt_FromLong(1));
PyTuple_SetItem(digits, 3, PyInt_FromLong(4));

PyObject *tuple = PyTuple_New(3);
PyTuple_SetItem(tuple, 0, PyInt_FromLong(1));
PyTuple_SetItem(tuple, 1, digits);
PyTuple_SetItem(tuple, 2, PyInt_FromLong(-3));

value = PyObject_CallFunctionObjArgs(PLy_decimal_ctor_global,
tuple, NULL);

return value;
}

Yes, it returns the same value regardless the params. The idea is to call
Python code like:

Decimal((0, (1, 4, 1, 4), -3))

which is simply:

Decimal('1.414')

Unfortunately this is not faster. It is as slow as it was with string
constructor.

I've checked the speed of decimal.Decimal using pure python. For this I
used a simple function, similar to yours:

def x(a, b):
if a is None:
return b
return a + b

I've run the tests using simple ints:

def test():
a = 0
for i in xrange(0, 10000):
a += x(a, i)

for a in xrange(1, 100):
test()

And later I've run the same function, but with converting the arguments to
Decimals:

from decimal import Decimal

def x(a, b):
if a is None:
return b
return a + b

def test():
a = 0
for i in xrange(0, 10000):
a += x(Decimal(a), Decimal(i))

for a in xrange(1, 100):
test()

It was run 100 times for decreasing the impact of test initialization.

The results for both files are:
int: 0.697s
decimal: 38.859s

What gives average time for one function call of:
int: 69ms
decimal: 380ms

For me the problem is with slow code at Python's side, the Decimal
constructors are pretty slow, and there is nothing I can do with that at
the Postgres' side.

I will send patch with fixes later.

thanks,
Szymon

#4Szymon Guz
mabewlun@gmail.com
In reply to: Steve Singer (#2)
1 attachment(s)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 25 June 2013 05:16, Steve Singer <steve@ssinger.info> wrote:

On 05/28/2013 04:41 PM, Szymon Guz wrote:

Hi,
I've got a patch.

This is for a plpython enhancement.

There is an item at the TODO list http://wiki.postgresql.org/**
wiki/Todo#Server-Side_**Languages<http://wiki.postgresql.org/wiki/Todo#Server-Side_Languages&gt;
"Fix loss of information during conversion of numeric type to Python
float"

This patch uses a decimal.Decimal type from Python standard library for
the plpthon function numeric argument instead of float.

Patch contains changes in code, documentation and tests.

Most probably there is something wrong, as this is my first Postgres
patch :)

Thanks for contributing.

This patch applies cleanly against master and compiles with warnings

plpy_main.c: In function ‘PLy_init_interp’:
plpy_main.c:157:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-**statement]
plpy_main.c:161:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-**statement]

You can avoid this by moving the declaration of decimal and decimal_dict
to be at the top of the function where mainmod is declared.

Also in this function you've introduced places where it returns with an
error (the PLy_elog(ERROR...) calls before decrementing the reference to
mainmod. I think you can decrement the mainmod reference after the call to
SetItemString before your changes that import the Decimal module.

The patch works as expected, I am able to write python functions that take
numerics as arguments and work with them. I can adjust the decimal context
precision inside of my function.

One concern I have is that this patch makes pl/python functions involving
numerics more than 3 times as slow as before.

create temp table b(a numeric);
insert into b select generate_series(1,10000);

create or replace function x(a numeric,b numeric) returns numeric as $$
if a==None:
return b
return a+b
$$ language plpythonu;
create aggregate sm(basetype=numeric, sfunc=x,stype=numeric);

test=# select sm(a) from b;
sm
----------
50005000
(1 row)

Time: 565.650 ms

versus before the patch this was taking in the range of 80ms.

Would it be faster to call numeric_send instead of numeric_out and then
convert the sequence of Int16's to a tuple of digits that can be passed
into the Decimal constructor? I think this is worth trying and testing,

Documentation
=================
Your patched version of the docs say

PostgreSQL <type>real</type>, <type>double</type>, and
<type>numeric</type> are converted to
Python <type>Decimal</type>. This type is imported
from<literal>decimal.Decimal</**literal>.

I don't think this is correct, as far as I can tell your not changing the
behaviour for postgresql real and double types, they continue to use
floating point.

<listitem>
<para>
PostgreSQL <type>real</type> and <type>double</type>are converted to
Python <type>float</type>.
</para>
</listitem>

<listitem>
<para>
PostgreSQL <type>numeric</type> is converted to
Python <type>Decimal</type>. This type is imported from
<literal>decimal.Decimal</**literal>.
</para>
</listitem>

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

Attachments:

plpython_decimal_v2.patchapplication/octet-stream; name=plpython_decimal_v2.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index aaf758d..208f2a8 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -308,14 +308,18 @@ $$ LANGUAGE plpythonu;
       </para>
      </listitem>
 
+	 <listitem>
+      <para>
+       PostgreSQL <type>real</type> and <type>double</type> are converted to
+       Python <type>float</type>.
+      </para>
+     </listitem>
+
      <listitem>
       <para>
-       PostgreSQL <type>real</type>, <type>double</type>,
-       and <type>numeric</type> are converted to
-       Python <type>float</type>.  Note that for
-       the <type>numeric</type> this loses information and can lead to
-       incorrect results.  This might be fixed in a future
-       release.
+       PostgreSQL <type>numeric</type> is converted to
+       Python <type>Decimal</type>. This type is imported from
+       <literal>decimal.Decimal</literal>.
       </para>
      </listitem>
 
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 4641345..30c4455 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -216,31 +216,47 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <type 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <type 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <type 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  (Decimal('-1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <type 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 0dad843..8316b8d 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -77,6 +77,9 @@ static const int plpython_python_version = PY_MAJOR_VERSION;
 /* initialize global variables */
 PyObject   *PLy_interp_globals = NULL;
 
+/* global pointer to decimal.Decimal costructor */
+PyObject   *PLy_decimal_ctor_global = NULL;
+
 /* this doesn't need to be global; use PLy_current_execution_context() */
 static PLyExecutionContext *PLy_execution_contexts = NULL;
 
@@ -136,7 +139,7 @@ void
 PLy_init_interp(void)
 {
 	static PyObject *PLy_interp_safe_globals = NULL;
-	PyObject   *mainmod;
+	PyObject   *mainmod, *decimal, *decimal_dict;
 
 	mainmod = PyImport_AddModule("__main__");
 	if (mainmod == NULL || PyErr_Occurred())
@@ -147,9 +150,17 @@ PLy_init_interp(void)
 	if (PLy_interp_safe_globals == NULL)
 		PLy_elog(ERROR, "could not create globals");
 	PyDict_SetItemString(PLy_interp_globals, "GD", PLy_interp_safe_globals);
+	
 	Py_DECREF(mainmod);
+
 	if (PLy_interp_globals == NULL || PyErr_Occurred())
 		PLy_elog(ERROR, "could not initialize globals");
+
+	decimal = PyImport_ImportModule("decimal");
+	if (decimal == NULL)
+		PLy_elog(ERROR, "could not import module 'decimal'");
+
+	decimal_dict = PyModule_GetDict(decimal);
 }
 
 Datum
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 6a9a2cb..7da085f 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -18,6 +18,7 @@
 #include "utils/memutils.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
+#include "utils/numeric.h"
 
 #include "plpython.h"
 
@@ -26,6 +27,7 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 
+extern PyObject *PLy_decimal_ctor_global;
 
 /* I/O function caching */
 static void PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup);
@@ -35,7 +37,7 @@ static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup);
 static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
-static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
+static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
@@ -450,7 +452,7 @@ PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup)
 			arg->func = PLyFloat_FromFloat8;
 			break;
 		case NUMERICOID:
-			arg->func = PLyFloat_FromNumeric;
+			arg->func = PLyDecimal_FromNumeric;
 			break;
 		case INT2OID:
 			arg->func = PLyInt_FromInt16;
@@ -516,16 +518,15 @@ PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d)
 }
 
 static PyObject *
-PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
+PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
 {
-	/*
-	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
-	 * it be better to cast to PyString?
-	 */
-	Datum		f = DirectFunctionCall1(numeric_float8, d);
-	double		x = DatumGetFloat8(f);
+	char *x;
+	PyObject *pvalue, *value;
 
-	return PyFloat_FromDouble(x);
+	x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
+	pvalue = PyString_FromString(x);
+	value = PyObject_CallFunctionObjArgs(PLy_decimal_ctor_global, pvalue, NULL);
+	return value;
 }
 
 static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 6a50b42..6bf0c4a 100644
--- a/src/pl/plpython/sql/plpython_types.sql
+++ b/src/pl/plpython/sql/plpython_types.sql
@@ -90,10 +90,12 @@ plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
 
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
 SELECT * FROM test_type_conversion_numeric(-100);
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
 SELECT * FROM test_type_conversion_numeric(null);
 
 
#5Ronan Dunklau
rdunklau@gmail.com
In reply to: Szymon Guz (#4)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

Concerning the efficiency problem, it should be noted that the latest 3.3
release of cpython introduces an "accelerator" for decimal data types, as a
C-module. This module was previously available from the Python package
index at: https://pypi.python.org/pypi/cdecimal/2.2

It may be overkill to try to include such a dependency, but the performance
overhead from using decimal is really mitigated by this implementation.

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

Show quoted text

On 25 June 2013 05:16, Steve Singer <steve@ssinger.info> wrote:

On 05/28/2013 04:41 PM, Szymon Guz wrote:

Hi,
I've got a patch.

This is for a plpython enhancement.

There is an item at the TODO list http://wiki.postgresql.org/**
wiki/Todo#Server-Side_**Languages<http://wiki.postgresql.org/wiki/Todo#Server-Side_Languages&gt;
"Fix loss of information during conversion of numeric type to Python
float"

This patch uses a decimal.Decimal type from Python standard library for
the plpthon function numeric argument instead of float.

Patch contains changes in code, documentation and tests.

Most probably there is something wrong, as this is my first Postgres
patch :)

Thanks for contributing.

This patch applies cleanly against master and compiles with warnings

plpy_main.c: In function ‘PLy_init_interp’:
plpy_main.c:157:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-**statement]
plpy_main.c:161:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-**statement]

You can avoid this by moving the declaration of decimal and decimal_dict
to be at the top of the function where mainmod is declared.

Also in this function you've introduced places where it returns with an
error (the PLy_elog(ERROR...) calls before decrementing the reference to
mainmod. I think you can decrement the mainmod reference after the call to
SetItemString before your changes that import the Decimal module.

The patch works as expected, I am able to write python functions that
take numerics as arguments and work with them. I can adjust the decimal
context precision inside of my function.

One concern I have is that this patch makes pl/python functions involving
numerics more than 3 times as slow as before.

create temp table b(a numeric);
insert into b select generate_series(1,10000);

create or replace function x(a numeric,b numeric) returns numeric as $$
if a==None:
return b
return a+b
$$ language plpythonu;
create aggregate sm(basetype=numeric, sfunc=x,stype=numeric);

test=# select sm(a) from b;
sm
----------
50005000
(1 row)

Time: 565.650 ms

versus before the patch this was taking in the range of 80ms.

Would it be faster to call numeric_send instead of numeric_out and then
convert the sequence of Int16's to a tuple of digits that can be passed
into the Decimal constructor? I think this is worth trying and testing,

Documentation
=================
Your patched version of the docs say

PostgreSQL <type>real</type>, <type>double</type>, and
<type>numeric</type> are converted to
Python <type>Decimal</type>. This type is imported
from<literal>decimal.Decimal</**literal>.

I don't think this is correct, as far as I can tell your not changing the
behaviour for postgresql real and double types, they continue to use
floating point.

<listitem>
<para>
PostgreSQL <type>real</type> and <type>double</type>are converted
to
Python <type>float</type>.
</para>
</listitem>

<listitem>
<para>
PostgreSQL <type>numeric</type> is converted to
Python <type>Decimal</type>. This type is imported from
<literal>decimal.Decimal</**literal>.
</para>
</listitem>

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#6Szymon Guz
mabewlun@gmail.com
In reply to: Ronan Dunklau (#5)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

Well, I really don't like the idea of such a dependency.

However it could be added as configuration option, so you could compile
postgres with e.g. --with-cdecimal, and then it would be user dependent.
Maybe it is a good idea for another patch.

On 25 June 2013 14:23, Ronan Dunklau <rdunklau@gmail.com> wrote:

Show quoted text

Concerning the efficiency problem, it should be noted that the latest 3.3
release of cpython introduces an "accelerator" for decimal data types, as a
C-module. This module was previously available from the Python package
index at: https://pypi.python.org/pypi/cdecimal/2.2

It may be overkill to try to include such a dependency, but the
performance overhead from using decimal is really mitigated by this
implementation.

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

On 25 June 2013 05:16, Steve Singer <steve@ssinger.info> wrote:

On 05/28/2013 04:41 PM, Szymon Guz wrote:

Hi,
I've got a patch.

This is for a plpython enhancement.

There is an item at the TODO list http://wiki.postgresql.org/**
wiki/Todo#Server-Side_**Languages<http://wiki.postgresql.org/wiki/Todo#Server-Side_Languages&gt;
"Fix loss of information during conversion of numeric type to Python
float"

This patch uses a decimal.Decimal type from Python standard library for
the plpthon function numeric argument instead of float.

Patch contains changes in code, documentation and tests.

Most probably there is something wrong, as this is my first Postgres
patch :)

Thanks for contributing.

This patch applies cleanly against master and compiles with warnings

plpy_main.c: In function ‘PLy_init_interp’:
plpy_main.c:157:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-**statement]
plpy_main.c:161:2: warning: ISO C90 forbids mixed declarations and code
[-Wdeclaration-after-**statement]

You can avoid this by moving the declaration of decimal and decimal_dict
to be at the top of the function where mainmod is declared.

Also in this function you've introduced places where it returns with an
error (the PLy_elog(ERROR...) calls before decrementing the reference to
mainmod. I think you can decrement the mainmod reference after the call to
SetItemString before your changes that import the Decimal module.

The patch works as expected, I am able to write python functions that
take numerics as arguments and work with them. I can adjust the decimal
context precision inside of my function.

One concern I have is that this patch makes pl/python functions
involving numerics more than 3 times as slow as before.

create temp table b(a numeric);
insert into b select generate_series(1,10000);

create or replace function x(a numeric,b numeric) returns numeric as $$
if a==None:
return b
return a+b
$$ language plpythonu;
create aggregate sm(basetype=numeric, sfunc=x,stype=numeric);

test=# select sm(a) from b;
sm
----------
50005000
(1 row)

Time: 565.650 ms

versus before the patch this was taking in the range of 80ms.

Would it be faster to call numeric_send instead of numeric_out and then
convert the sequence of Int16's to a tuple of digits that can be passed
into the Decimal constructor? I think this is worth trying and testing,

Documentation
=================
Your patched version of the docs say

PostgreSQL <type>real</type>, <type>double</type>, and
<type>numeric</type> are converted to
Python <type>Decimal</type>. This type is imported
from<literal>decimal.Decimal</**literal>.

I don't think this is correct, as far as I can tell your not changing
the behaviour for postgresql real and double types, they continue to use
floating point.

<listitem>
<para>
PostgreSQL <type>real</type> and <type>double</type>are converted
to
Python <type>float</type>.
</para>
</listitem>

<listitem>
<para>
PostgreSQL <type>numeric</type> is converted to
Python <type>Decimal</type>. This type is imported from
<literal>decimal.Decimal</**literal>.
</para>
</listitem>

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#7Steve Singer
steve@ssinger.info
In reply to: Szymon Guz (#4)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Szymon Guz
mabewlun@gmail.com
In reply to: Steve Singer (#7)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

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

#9Ronan Dunklau
rdunklau@gmail.com
In reply to: Szymon Guz (#8)
2 attachment(s)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

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>

Show quoted text

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

Attachments:

cnumeric.patchapplication/octet-stream; name=cnumeric.patchDownload
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 35f15d9..8a74eec 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -155,8 +155,13 @@ PLy_init_interp(void)
 
 	if (PLy_interp_globals == NULL || PyErr_Occurred())
 		PLy_elog(ERROR, "could not initialize globals");
-
-	decimal = PyImport_ImportModule("decimal");
+	/* Try to import cdecimal, if it doesnt exist, fallback to decimal */
+	decimal = PyImport_ImportModule("cdecimal");
+	if (decimal == NULL)
+	{
+		PyErr_Clear();
+		decimal = PyImport_ImportModule("decimal");
+	}
 	if (decimal == NULL)
 		PLy_elog(ERROR, "could not import module 'decimal'");
 
null_ctor.patchapplication/octet-stream; name=null_ctor.patchDownload
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 8316b8d..35f15d9 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -161,6 +161,8 @@ PLy_init_interp(void)
 		PLy_elog(ERROR, "could not import module 'decimal'");
 
 	decimal_dict = PyModule_GetDict(decimal);
+	PLy_decimal_ctor_global = PyDict_GetItemString(decimal_dict, "Decimal");
+	Py_DECREF(decimal_dict);
 }
 
 Datum
#10Szymon Guz
mabewlun@gmail.com
In reply to: Ronan Dunklau (#9)
1 attachment(s)
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:

Show quoted text

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

Attachments:

plpython_decimal_v3.patchapplication/octet-stream; name=plpython_decimal_v3.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index aaf758d..da27874 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -308,14 +308,19 @@ $$ LANGUAGE plpythonu;
       </para>
      </listitem>
 
+	 <listitem>
+      <para>
+       PostgreSQL <type>real</type> and <type>double</type> are converted to
+       Python <type>float</type>.
+      </para>
+     </listitem>
+
      <listitem>
       <para>
-       PostgreSQL <type>real</type>, <type>double</type>,
-       and <type>numeric</type> are converted to
-       Python <type>float</type>.  Note that for
-       the <type>numeric</type> this loses information and can lead to
-       incorrect results.  This might be fixed in a future
-       release.
+       PostgreSQL <type>numeric</type> is converted to
+       Python <type>Decimal</type>. This type is imported from 
+	   <literal>cdecimal</literal> package if it is available. If cdecimal
+	   cannot be used, then <literal>decimal.Decimal</literal> will be used.
       </para>
      </listitem>
 
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 4641345..30c4455 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -216,31 +216,47 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <type 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <type 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <type 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  (Decimal('-1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <type 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 0dad843..8a74eec 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -77,6 +77,9 @@ static const int plpython_python_version = PY_MAJOR_VERSION;
 /* initialize global variables */
 PyObject   *PLy_interp_globals = NULL;
 
+/* global pointer to decimal.Decimal costructor */
+PyObject   *PLy_decimal_ctor_global = NULL;
+
 /* this doesn't need to be global; use PLy_current_execution_context() */
 static PLyExecutionContext *PLy_execution_contexts = NULL;
 
@@ -136,7 +139,7 @@ void
 PLy_init_interp(void)
 {
 	static PyObject *PLy_interp_safe_globals = NULL;
-	PyObject   *mainmod;
+	PyObject   *mainmod, *decimal, *decimal_dict;
 
 	mainmod = PyImport_AddModule("__main__");
 	if (mainmod == NULL || PyErr_Occurred())
@@ -147,9 +150,24 @@ PLy_init_interp(void)
 	if (PLy_interp_safe_globals == NULL)
 		PLy_elog(ERROR, "could not create globals");
 	PyDict_SetItemString(PLy_interp_globals, "GD", PLy_interp_safe_globals);
+	
 	Py_DECREF(mainmod);
+
 	if (PLy_interp_globals == NULL || PyErr_Occurred())
 		PLy_elog(ERROR, "could not initialize globals");
+	/* Try to import cdecimal, if it doesnt exist, fallback to decimal */
+	decimal = PyImport_ImportModule("cdecimal");
+	if (decimal == NULL)
+	{
+		PyErr_Clear();
+		decimal = PyImport_ImportModule("decimal");
+	}
+	if (decimal == NULL)
+		PLy_elog(ERROR, "could not import module 'decimal'");
+
+	decimal_dict = PyModule_GetDict(decimal);
+	PLy_decimal_ctor_global = PyDict_GetItemString(decimal_dict, "Decimal");
+	Py_DECREF(decimal_dict);
 }
 
 Datum
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 6a9a2cb..7da085f 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -18,6 +18,7 @@
 #include "utils/memutils.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
+#include "utils/numeric.h"
 
 #include "plpython.h"
 
@@ -26,6 +27,7 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 
+extern PyObject *PLy_decimal_ctor_global;
 
 /* I/O function caching */
 static void PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup);
@@ -35,7 +37,7 @@ static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup);
 static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
-static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
+static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
@@ -450,7 +452,7 @@ PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup)
 			arg->func = PLyFloat_FromFloat8;
 			break;
 		case NUMERICOID:
-			arg->func = PLyFloat_FromNumeric;
+			arg->func = PLyDecimal_FromNumeric;
 			break;
 		case INT2OID:
 			arg->func = PLyInt_FromInt16;
@@ -516,16 +518,15 @@ PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d)
 }
 
 static PyObject *
-PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
+PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
 {
-	/*
-	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
-	 * it be better to cast to PyString?
-	 */
-	Datum		f = DirectFunctionCall1(numeric_float8, d);
-	double		x = DatumGetFloat8(f);
+	char *x;
+	PyObject *pvalue, *value;
 
-	return PyFloat_FromDouble(x);
+	x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
+	pvalue = PyString_FromString(x);
+	value = PyObject_CallFunctionObjArgs(PLy_decimal_ctor_global, pvalue, NULL);
+	return value;
 }
 
 static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 6a50b42..6bf0c4a 100644
--- a/src/pl/plpython/sql/plpython_types.sql
+++ b/src/pl/plpython/sql/plpython_types.sql
@@ -90,10 +90,12 @@ plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
 
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
 SELECT * FROM test_type_conversion_numeric(-100);
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
 SELECT * FROM test_type_conversion_numeric(null);
 
 
#11Szymon Guz
mabewlun@gmail.com
In reply to: Szymon Guz (#10)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

You had a great idea, the time with cdecimal is really great, the
difference on my machine is 64 ms vs 430 ms.

Szymon

#12Ronan Dunklau
rdunklau@gmail.com
In reply to: Szymon Guz (#10)
1 attachment(s)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

It seems like you confused me with steve :)

The patch applies cleanly, and the regression tests pass on python2 when
cdecimal is not installed. When it is, the type info returned for the
converted numeric value is cdecimal.Decimal instead of decimal.Decimal.

The regression tests expected output have not been modified for python3,
and as such they fail on the type conversions.

I am a bit confused with the use of PyModule_GetDict: shouldn't
PyObj_GetAttrString be used directly instead ? Moreover, the reference
count in the current implementation might be off: the reference count for
the decimal module is never decreased, while the reference count to the
module dict is, when the docs say it returns a borrowed reference.

Please find a patch that fixes both issues.

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

Show quoted text

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

Attachments:

plpython_decimal_fix_regression.patchapplication/octet-stream; name=plpython_decimal_fix_regression.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index aaf758d..da27874 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -308,14 +308,19 @@ $$ LANGUAGE plpythonu;
       </para>
      </listitem>
 
+	 <listitem>
+      <para>
+       PostgreSQL <type>real</type> and <type>double</type> are converted to
+       Python <type>float</type>.
+      </para>
+     </listitem>
+
      <listitem>
       <para>
-       PostgreSQL <type>real</type>, <type>double</type>,
-       and <type>numeric</type> are converted to
-       Python <type>float</type>.  Note that for
-       the <type>numeric</type> this loses information and can lead to
-       incorrect results.  This might be fixed in a future
-       release.
+       PostgreSQL <type>numeric</type> is converted to
+       Python <type>Decimal</type>. This type is imported from 
+	   <literal>cdecimal</literal> package if it is available. If cdecimal
+	   cannot be used, then <literal>decimal.Decimal</literal> will be used.
       </para>
      </listitem>
 
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 4641345..30c4455 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -216,31 +216,47 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <type 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <type 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <type 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  (Decimal('-1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <type 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index 511ef5a..d50e7eb 100644
--- a/src/pl/plpython/expected/plpython_types_3.out
+++ b/src/pl/plpython/expected/plpython_types_3.out
@@ -216,31 +216,47 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpython3u;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <class 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <class 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <class 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  (Decimal('-1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <class 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 0dad843..226a5b8 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -77,6 +77,9 @@ static const int plpython_python_version = PY_MAJOR_VERSION;
 /* initialize global variables */
 PyObject   *PLy_interp_globals = NULL;
 
+/* global pointer to decimal.Decimal costructor */
+PyObject   *PLy_decimal_ctor_global = NULL;
+
 /* this doesn't need to be global; use PLy_current_execution_context() */
 static PLyExecutionContext *PLy_execution_contexts = NULL;
 
@@ -136,7 +139,7 @@ void
 PLy_init_interp(void)
 {
 	static PyObject *PLy_interp_safe_globals = NULL;
-	PyObject   *mainmod;
+	PyObject   *mainmod, *decimal, *decimal_dict;
 
 	mainmod = PyImport_AddModule("__main__");
 	if (mainmod == NULL || PyErr_Occurred())
@@ -147,9 +150,22 @@ PLy_init_interp(void)
 	if (PLy_interp_safe_globals == NULL)
 		PLy_elog(ERROR, "could not create globals");
 	PyDict_SetItemString(PLy_interp_globals, "GD", PLy_interp_safe_globals);
+	
 	Py_DECREF(mainmod);
+
 	if (PLy_interp_globals == NULL || PyErr_Occurred())
 		PLy_elog(ERROR, "could not initialize globals");
+	/* Try to import cdecimal, if it doesnt exist, fallback to decimal */
+	decimal = PyImport_ImportModule("cdecimal");
+	if (decimal == NULL)
+	{
+		PyErr_Clear();
+		decimal = PyImport_ImportModule("decimal");
+	}
+	if (decimal == NULL)
+		PLy_elog(ERROR, "could not import module 'decimal'");
+	PLy_decimal_ctor_global = PyObject_GetAttrString(decimal, "Decimal");
+	Py_DECREF(decimal);
 }
 
 Datum
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 6a9a2cb..7da085f 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -18,6 +18,7 @@
 #include "utils/memutils.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
+#include "utils/numeric.h"
 
 #include "plpython.h"
 
@@ -26,6 +27,7 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 
+extern PyObject *PLy_decimal_ctor_global;
 
 /* I/O function caching */
 static void PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup);
@@ -35,7 +37,7 @@ static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup);
 static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
-static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
+static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
@@ -450,7 +452,7 @@ PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup)
 			arg->func = PLyFloat_FromFloat8;
 			break;
 		case NUMERICOID:
-			arg->func = PLyFloat_FromNumeric;
+			arg->func = PLyDecimal_FromNumeric;
 			break;
 		case INT2OID:
 			arg->func = PLyInt_FromInt16;
@@ -516,16 +518,15 @@ PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d)
 }
 
 static PyObject *
-PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
+PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
 {
-	/*
-	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
-	 * it be better to cast to PyString?
-	 */
-	Datum		f = DirectFunctionCall1(numeric_float8, d);
-	double		x = DatumGetFloat8(f);
+	char *x;
+	PyObject *pvalue, *value;
 
-	return PyFloat_FromDouble(x);
+	x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
+	pvalue = PyString_FromString(x);
+	value = PyObject_CallFunctionObjArgs(PLy_decimal_ctor_global, pvalue, NULL);
+	return value;
 }
 
 static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 6a50b42..6bf0c4a 100644
--- a/src/pl/plpython/sql/plpython_types.sql
+++ b/src/pl/plpython/sql/plpython_types.sql
@@ -90,10 +90,12 @@ plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
 
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
 SELECT * FROM test_type_conversion_numeric(-100);
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
 SELECT * FROM test_type_conversion_numeric(null);
 
 
#13Szymon Guz
mabewlun@gmail.com
In reply to: Ronan Dunklau (#12)
1 attachment(s)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

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

It seems like you confused me with steve :)

Hi Ronan,
Oh, yes. I'm sorry for that :)

The patch applies cleanly, and the regression tests pass on python2 when
cdecimal is not installed. When it is, the type info returned for the
converted numeric value is cdecimal.Decimal instead of decimal.Decimal.

The regression tests expected output have not been modified for python3,
and as such they fail on the type conversions.

I am a bit confused with the use of PyModule_GetDict: shouldn't
PyObj_GetAttrString be used directly instead ? Moreover, the reference
count in the current implementation might be off: the reference count for
the decimal module is never decreased, while the reference count to the
module dict is, when the docs say it returns a borrowed reference.

Please find a patch that fixes both issues.

Thanks for the patch. I assume you generated that from clean trunk, and it
includes all the changes (mine and yours) right?

I've checked the patch, everything looks great.
I've attached it to this email with changed name, just for consistent
naming in commitfest app.

thanks,
Szymon

Attachments:

plpython_decimal_v4.patchapplication/octet-stream; name=plpython_decimal_v4.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index aaf758d..da27874 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -308,14 +308,19 @@ $$ LANGUAGE plpythonu;
       </para>
      </listitem>
 
+	 <listitem>
+      <para>
+       PostgreSQL <type>real</type> and <type>double</type> are converted to
+       Python <type>float</type>.
+      </para>
+     </listitem>
+
      <listitem>
       <para>
-       PostgreSQL <type>real</type>, <type>double</type>,
-       and <type>numeric</type> are converted to
-       Python <type>float</type>.  Note that for
-       the <type>numeric</type> this loses information and can lead to
-       incorrect results.  This might be fixed in a future
-       release.
+       PostgreSQL <type>numeric</type> is converted to
+       Python <type>Decimal</type>. This type is imported from 
+	   <literal>cdecimal</literal> package if it is available. If cdecimal
+	   cannot be used, then <literal>decimal.Decimal</literal> will be used.
       </para>
      </listitem>
 
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 4641345..30c4455 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -216,31 +216,47 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <type 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <type 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <type 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  (Decimal('-1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <type 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index 511ef5a..d50e7eb 100644
--- a/src/pl/plpython/expected/plpython_types_3.out
+++ b/src/pl/plpython/expected/plpython_types_3.out
@@ -216,31 +216,47 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpython3u;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <class 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <class 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <class 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  (Decimal('-1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <class 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 0dad843..226a5b8 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -77,6 +77,9 @@ static const int plpython_python_version = PY_MAJOR_VERSION;
 /* initialize global variables */
 PyObject   *PLy_interp_globals = NULL;
 
+/* global pointer to decimal.Decimal costructor */
+PyObject   *PLy_decimal_ctor_global = NULL;
+
 /* this doesn't need to be global; use PLy_current_execution_context() */
 static PLyExecutionContext *PLy_execution_contexts = NULL;
 
@@ -136,7 +139,7 @@ void
 PLy_init_interp(void)
 {
 	static PyObject *PLy_interp_safe_globals = NULL;
-	PyObject   *mainmod;
+	PyObject   *mainmod, *decimal, *decimal_dict;
 
 	mainmod = PyImport_AddModule("__main__");
 	if (mainmod == NULL || PyErr_Occurred())
@@ -147,9 +150,22 @@ PLy_init_interp(void)
 	if (PLy_interp_safe_globals == NULL)
 		PLy_elog(ERROR, "could not create globals");
 	PyDict_SetItemString(PLy_interp_globals, "GD", PLy_interp_safe_globals);
+	
 	Py_DECREF(mainmod);
+
 	if (PLy_interp_globals == NULL || PyErr_Occurred())
 		PLy_elog(ERROR, "could not initialize globals");
+	/* Try to import cdecimal, if it doesnt exist, fallback to decimal */
+	decimal = PyImport_ImportModule("cdecimal");
+	if (decimal == NULL)
+	{
+		PyErr_Clear();
+		decimal = PyImport_ImportModule("decimal");
+	}
+	if (decimal == NULL)
+		PLy_elog(ERROR, "could not import module 'decimal'");
+	PLy_decimal_ctor_global = PyObject_GetAttrString(decimal, "Decimal");
+	Py_DECREF(decimal);
 }
 
 Datum
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 6a9a2cb..7da085f 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -18,6 +18,7 @@
 #include "utils/memutils.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
+#include "utils/numeric.h"
 
 #include "plpython.h"
 
@@ -26,6 +27,7 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 
+extern PyObject *PLy_decimal_ctor_global;
 
 /* I/O function caching */
 static void PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup);
@@ -35,7 +37,7 @@ static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup);
 static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
-static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
+static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
@@ -450,7 +452,7 @@ PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup)
 			arg->func = PLyFloat_FromFloat8;
 			break;
 		case NUMERICOID:
-			arg->func = PLyFloat_FromNumeric;
+			arg->func = PLyDecimal_FromNumeric;
 			break;
 		case INT2OID:
 			arg->func = PLyInt_FromInt16;
@@ -516,16 +518,15 @@ PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d)
 }
 
 static PyObject *
-PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
+PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
 {
-	/*
-	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
-	 * it be better to cast to PyString?
-	 */
-	Datum		f = DirectFunctionCall1(numeric_float8, d);
-	double		x = DatumGetFloat8(f);
+	char *x;
+	PyObject *pvalue, *value;
 
-	return PyFloat_FromDouble(x);
+	x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
+	pvalue = PyString_FromString(x);
+	value = PyObject_CallFunctionObjArgs(PLy_decimal_ctor_global, pvalue, NULL);
+	return value;
 }
 
 static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 6a50b42..6bf0c4a 100644
--- a/src/pl/plpython/sql/plpython_types.sql
+++ b/src/pl/plpython/sql/plpython_types.sql
@@ -90,10 +90,12 @@ plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
 
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
 SELECT * FROM test_type_conversion_numeric(-100);
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
 SELECT * FROM test_type_conversion_numeric(null);
 
 
#14Peter Eisentraut
peter_e@gmx.net
In reply to: Szymon Guz (#13)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 6/26/13 7:03 AM, Szymon Guz wrote:

I've checked the patch, everything looks great.
I've attached it to this email with changed name, just for consistent
naming in commitfest app.

Could the setup of the decimal.Decimal constructor be moved into
PLyDecimal_FromNumeric() and kept in a static pointer? I'd rather not
clutter up the main initialization routine.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15Szymon Guz
mabewlun@gmail.com
In reply to: Peter Eisentraut (#14)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 26 June 2013 21:59, Peter Eisentraut <peter_e@gmx.net> wrote:

On 6/26/13 7:03 AM, Szymon Guz wrote:

I've checked the patch, everything looks great.
I've attached it to this email with changed name, just for consistent
naming in commitfest app.

Could the setup of the decimal.Decimal constructor be moved into
PLyDecimal_FromNumeric() and kept in a static pointer? I'd rather not
clutter up the main initialization routine.

OK, I will.

#16Szymon Guz
mabewlun@gmail.com
In reply to: Szymon Guz (#15)
1 attachment(s)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 26 June 2013 22:08, Szymon Guz <mabewlun@gmail.com> wrote:

On 26 June 2013 21:59, Peter Eisentraut <peter_e@gmx.net> wrote:

On 6/26/13 7:03 AM, Szymon Guz wrote:

I've checked the patch, everything looks great.
I've attached it to this email with changed name, just for consistent
naming in commitfest app.

Could the setup of the decimal.Decimal constructor be moved into
PLyDecimal_FromNumeric() and kept in a static pointer? I'd rather not
clutter up the main initialization routine.

Attached patch has all changes against trunk code.

There is added a function for conversion from Postgres numeric to Python
Decimal. The Decimal type is taken from cdecimal.Decimal, if it is
available. It is an external library, quite fast, but may be not available.
If it is not available, then decimal.Decimal will be used. It is in
standard Python library, however it is rather slow.

The initialization is done in the conversion function, the pointer to a
proper Decimal constructor is stored as static variable inside the function
and is lazy initialized.

The documentation is updated.

Tests for python 2 and 3 have been added. They work only with standard
decimal.Decimal, as the type is printed in the *.out files. I think there
is nothing we can do with that now.

regards,
Szymon

Attachments:

plpython_decimal_v5.patchapplication/octet-stream; name=plpython_decimal_v5.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index aaf758d..da27874 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -308,14 +308,19 @@ $$ LANGUAGE plpythonu;
       </para>
      </listitem>
 
+	 <listitem>
+      <para>
+       PostgreSQL <type>real</type> and <type>double</type> are converted to
+       Python <type>float</type>.
+      </para>
+     </listitem>
+
      <listitem>
       <para>
-       PostgreSQL <type>real</type>, <type>double</type>,
-       and <type>numeric</type> are converted to
-       Python <type>float</type>.  Note that for
-       the <type>numeric</type> this loses information and can lead to
-       incorrect results.  This might be fixed in a future
-       release.
+       PostgreSQL <type>numeric</type> is converted to
+       Python <type>Decimal</type>. This type is imported from 
+	   <literal>cdecimal</literal> package if it is available. If cdecimal
+	   cannot be used, then <literal>decimal.Decimal</literal> will be used.
       </para>
      </listitem>
 
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 4641345..30c4455 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -216,31 +216,47 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <type 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <type 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <type 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  (Decimal('-1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <type 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index 511ef5a..d50e7eb 100644
--- a/src/pl/plpython/expected/plpython_types_3.out
+++ b/src/pl/plpython/expected/plpython_types_3.out
@@ -216,31 +216,47 @@ CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
 plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpython3u;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <class 'float'>)
+INFO:  (Decimal('100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <class 'float'>)
+INFO:  (Decimal('-100'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <class 'float'>)
+INFO:  (Decimal('5000000000.5'), <class 'decimal.Decimal'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  (Decimal('1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  (Decimal('-1234567890.0987654321'), <class 'decimal.Decimal'>)
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
 INFO:  (None, <class 'NoneType'>)
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
diff --git a/src/pl/plpython/plpy_main.c b/src/pl/plpython/plpy_main.c
index 0dad843..54a4caf 100644
--- a/src/pl/plpython/plpy_main.c
+++ b/src/pl/plpython/plpy_main.c
@@ -147,7 +147,9 @@ PLy_init_interp(void)
 	if (PLy_interp_safe_globals == NULL)
 		PLy_elog(ERROR, "could not create globals");
 	PyDict_SetItemString(PLy_interp_globals, "GD", PLy_interp_safe_globals);
+	
 	Py_DECREF(mainmod);
+
 	if (PLy_interp_globals == NULL || PyErr_Occurred())
 		PLy_elog(ERROR, "could not initialize globals");
 }
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 6a9a2cb..6e52ae4 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -18,6 +18,7 @@
 #include "utils/memutils.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
+#include "utils/numeric.h"
 
 #include "plpython.h"
 
@@ -26,7 +27,6 @@
 #include "plpy_elog.h"
 #include "plpy_main.h"
 
-
 /* I/O function caching */
 static void PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup);
 static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup);
@@ -35,7 +35,7 @@ static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup);
 static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
-static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
+static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
@@ -450,7 +450,7 @@ PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup)
 			arg->func = PLyFloat_FromFloat8;
 			break;
 		case NUMERICOID:
-			arg->func = PLyFloat_FromNumeric;
+			arg->func = PLyDecimal_FromNumeric;
 			break;
 		case INT2OID:
 			arg->func = PLyInt_FromInt16;
@@ -516,16 +516,33 @@ PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d)
 }
 
 static PyObject *
-PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
+PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
 {
-	/*
-	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
-	 * it be better to cast to PyString?
-	 */
-	Datum		f = DirectFunctionCall1(numeric_float8, d);
-	double		x = DatumGetFloat8(f);
+	char *x;
+	PyObject *pvalue, *value, *decimal, *decimal_dict;
+	static PyObject *decimal_ctor;
+
+	/* Try to import cdecimal, if it doesnt exist, fallback to decimal */
+	if (decimal_ctor == NULL)
+	{
+		decimal = PyImport_ImportModule("cdecimal");
+		if (decimal == NULL)
+		{
+			PyErr_Clear();
+			decimal = PyImport_ImportModule("decimal");
+		}
+		if (decimal == NULL)
+			PLy_elog(ERROR, "could not import module 'decimal'");
+
+		decimal_dict = PyModule_GetDict(decimal);
+		decimal_ctor = PyDict_GetItemString(decimal_dict, "Decimal");
+		Py_DECREF(decimal_dict);
+	}
 
-	return PyFloat_FromDouble(x);
+	x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
+	pvalue = PyString_FromString(x);
+	value = PyObject_CallFunctionObjArgs(decimal_ctor, pvalue, NULL);
+	return value;
 }
 
 static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 6a50b42..6bf0c4a 100644
--- a/src/pl/plpython/sql/plpython_types.sql
+++ b/src/pl/plpython/sql/plpython_types.sql
@@ -90,10 +90,12 @@ plpy.info(x, type(x))
 return x
 $$ LANGUAGE plpythonu;
 
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to decimal.Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
 SELECT * FROM test_type_conversion_numeric(-100);
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
 SELECT * FROM test_type_conversion_numeric(null);
 
 
#17Steve Singer
steve@ssinger.info
In reply to: Szymon Guz (#16)
1 attachment(s)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 06/26/2013 04:47 PM, Szymon Guz wrote:

Attached patch has all changes against trunk code.

There is added a function for conversion from Postgres numeric to
Python Decimal. The Decimal type is taken from cdecimal.Decimal, if it
is available. It is an external library, quite fast, but may be not
available. If it is not available, then decimal.Decimal will be used.
It is in standard Python library, however it is rather slow.

The initialization is done in the conversion function, the pointer to
a proper Decimal constructor is stored as static variable inside the
function and is lazy initialized.

The documentation is updated.

I've tested this version with python 2.7 with and without cdecimal and
also with 3.3 that has the faster decimal performance. It seems fine.

The v5 version of the patch makes only white-space changes to
plpy_main.c you should excluded that from the patch if your making a new
version (I have done this in the v6 version I'm attaching)

Tests for python 2 and 3 have been added. They work only with standard
decimal.Decimal, as the type is printed in the *.out files. I think
there is nothing we can do with that now.

I think we should make test_type_conversion_numeric to do something
that generates the same output in both cases. ie
py.info(str(x)). I downside of having the test fail on installs with
cdecimal installed is much greater than any benefit we get by ensuring
that the type is really decimal.
I've attached a v6 version of the patch that does this, do you agree
with my thinking?

Steve

Show quoted text

regards,
Szymon

Attachments:

plpython_decimal_v6.patchtext/x-patch; name=plpython_decimal_v6.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
new file mode 100644
index aaf758d..da27874
*** a/doc/src/sgml/plpython.sgml
--- b/doc/src/sgml/plpython.sgml
*************** $$ LANGUAGE plpythonu;
*** 308,321 ****
        </para>
       </listitem>
  
       <listitem>
        <para>
!        PostgreSQL <type>real</type>, <type>double</type>,
!        and <type>numeric</type> are converted to
!        Python <type>float</type>.  Note that for
!        the <type>numeric</type> this loses information and can lead to
!        incorrect results.  This might be fixed in a future
!        release.
        </para>
       </listitem>
  
--- 308,326 ----
        </para>
       </listitem>
  
+ 	 <listitem>
+       <para>
+        PostgreSQL <type>real</type> and <type>double</type> are converted to
+        Python <type>float</type>.
+       </para>
+      </listitem>
+ 
       <listitem>
        <para>
!        PostgreSQL <type>numeric</type> is converted to
!        Python <type>Decimal</type>. This type is imported from 
! 	   <literal>cdecimal</literal> package if it is available. If cdecimal
! 	   cannot be used, then <literal>decimal.Decimal</literal> will be used.
        </para>
       </listitem>
  
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
new file mode 100644
index 4641345..46308ed
*** a/src/pl/plpython/expected/plpython_types.out
--- b/src/pl/plpython/expected/plpython_types.out
*************** CONTEXT:  PL/Python function "test_type_
*** 213,248 ****
  (1 row)
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(x, type(x))
  return x
  $$ LANGUAGE plpythonu;
! /* The current implementation converts numeric to float. */
  SELECT * FROM test_type_conversion_numeric(100);
! INFO:  (100.0, <type 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                         100.0
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(-100);
! INFO:  (-100.0, <type 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                        -100.0
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
! INFO:  (5000000000.5, <type 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
                   5000000000.5
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(null);
! INFO:  (None, <type 'NoneType'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
--- 213,264 ----
  (1 row)
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(str(x))
  return x
  $$ LANGUAGE plpythonu;
! /* The current implementation converts numeric to decimal.Decimal. */
  SELECT * FROM test_type_conversion_numeric(100);
! INFO:  100
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                           100
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(-100);
! INFO:  -100
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                          -100
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
! INFO:  5000000000.5
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
                   5000000000.5
  (1 row)
  
+ SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+ INFO:  1234567890.0987654321
+ CONTEXT:  PL/Python function "test_type_conversion_numeric"
+  test_type_conversion_numeric 
+ ------------------------------
+         1234567890.0987654321
+ (1 row)
+ 
+ SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+ INFO:  -1234567890.0987654321
+ CONTEXT:  PL/Python function "test_type_conversion_numeric"
+  test_type_conversion_numeric 
+ ------------------------------
+        -1234567890.0987654321
+ (1 row)
+ 
  SELECT * FROM test_type_conversion_numeric(null);
! INFO:  None
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
new file mode 100644
index 511ef5a..d791fb6
*** a/src/pl/plpython/expected/plpython_types_3.out
--- b/src/pl/plpython/expected/plpython_types_3.out
*************** CONTEXT:  PL/Python function "test_type_
*** 213,248 ****
  (1 row)
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(x, type(x))
  return x
  $$ LANGUAGE plpython3u;
! /* The current implementation converts numeric to float. */
  SELECT * FROM test_type_conversion_numeric(100);
! INFO:  (100.0, <class 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                         100.0
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(-100);
! INFO:  (-100.0, <class 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                        -100.0
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
! INFO:  (5000000000.5, <class 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
                   5000000000.5
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(null);
! INFO:  (None, <class 'NoneType'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
--- 213,264 ----
  (1 row)
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(str(x))
  return x
  $$ LANGUAGE plpython3u;
! /* The current implementation converts numeric to decimal.Decimal. */
  SELECT * FROM test_type_conversion_numeric(100);
! INFO:  100
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                           100
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(-100);
! INFO:  -100
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                          -100
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
! INFO:  5000000000.5
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
                   5000000000.5
  (1 row)
  
+ SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+ INFO:  1234567890.0987654321
+ CONTEXT:  PL/Python function "test_type_conversion_numeric"
+  test_type_conversion_numeric 
+ ------------------------------
+         1234567890.0987654321
+ (1 row)
+ 
+ SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+ INFO:  -1234567890.0987654321
+ CONTEXT:  PL/Python function "test_type_conversion_numeric"
+  test_type_conversion_numeric 
+ ------------------------------
+        -1234567890.0987654321
+ (1 row)
+ 
  SELECT * FROM test_type_conversion_numeric(null);
! INFO:  None
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
new file mode 100644
index 6a9a2cb..e04c2f9
*** a/src/pl/plpython/plpy_typeio.c
--- b/src/pl/plpython/plpy_typeio.c
***************
*** 18,23 ****
--- 18,24 ----
  #include "utils/memutils.h"
  #include "utils/syscache.h"
  #include "utils/typcache.h"
+ #include "utils/numeric.h"
  
  #include "plpython.h"
  
*************** static void PLy_output_datum_func2(PLyOb
*** 35,41 ****
  static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
! static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
--- 36,42 ----
  static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
! static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
*************** PLy_input_datum_func2(PLyDatumToOb *arg,
*** 450,456 ****
  			arg->func = PLyFloat_FromFloat8;
  			break;
  		case NUMERICOID:
! 			arg->func = PLyFloat_FromNumeric;
  			break;
  		case INT2OID:
  			arg->func = PLyInt_FromInt16;
--- 451,457 ----
  			arg->func = PLyFloat_FromFloat8;
  			break;
  		case NUMERICOID:
! 			arg->func = PLyDecimal_FromNumeric;
  			break;
  		case INT2OID:
  			arg->func = PLyInt_FromInt16;
*************** PLyFloat_FromFloat8(PLyDatumToOb *arg, D
*** 516,531 ****
  }
  
  static PyObject *
! PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
  {
! 	/*
! 	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
! 	 * it be better to cast to PyString?
! 	 */
! 	Datum		f = DirectFunctionCall1(numeric_float8, d);
! 	double		x = DatumGetFloat8(f);
  
! 	return PyFloat_FromDouble(x);
  }
  
  static PyObject *
--- 517,549 ----
  }
  
  static PyObject *
! PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
  {
! 	char *x;
! 	PyObject *pvalue, *value, *decimal, *decimal_dict;
! 	static PyObject *decimal_ctor;
  
! 	/* Try to import cdecimal, if it doesnt exist, fallback to decimal */
! 	if (decimal_ctor == NULL)
! 	{
! 		decimal = PyImport_ImportModule("cdecimal");
! 		if (decimal == NULL)
! 		{
! 			PyErr_Clear();
! 			decimal = PyImport_ImportModule("decimal");
! 		}
! 		if (decimal == NULL)
! 			PLy_elog(ERROR, "could not import module 'decimal'");
! 
! 		decimal_dict = PyModule_GetDict(decimal);
! 		decimal_ctor = PyDict_GetItemString(decimal_dict, "Decimal");
! 		Py_DECREF(decimal_dict);
! 	}
! 
! 	x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
! 	pvalue = PyString_FromString(x);
! 	value = PyObject_CallFunctionObjArgs(decimal_ctor, pvalue, NULL);
! 	return value;
  }
  
  static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
new file mode 100644
index 6a50b42..83a8ef9
*** a/src/pl/plpython/sql/plpython_types.sql
--- b/src/pl/plpython/sql/plpython_types.sql
*************** SELECT * FROM test_type_conversion_int8(
*** 86,99 ****
  
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(x, type(x))
  return x
  $$ LANGUAGE plpythonu;
  
! /* The current implementation converts numeric to float. */
  SELECT * FROM test_type_conversion_numeric(100);
  SELECT * FROM test_type_conversion_numeric(-100);
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
  SELECT * FROM test_type_conversion_numeric(null);
  
  
--- 86,101 ----
  
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(str(x))
  return x
  $$ LANGUAGE plpythonu;
  
! /* The current implementation converts numeric to decimal.Decimal. */
  SELECT * FROM test_type_conversion_numeric(100);
  SELECT * FROM test_type_conversion_numeric(-100);
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
+ SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+ SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
  SELECT * FROM test_type_conversion_numeric(null);
  
  
#18Szymon Guz
mabewlun@gmail.com
In reply to: Steve Singer (#17)
1 attachment(s)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 27 June 2013 05:21, Steve Singer <steve@ssinger.info> wrote:

On 06/26/2013 04:47 PM, Szymon Guz wrote:

Attached patch has all changes against trunk code.

There is added a function for conversion from Postgres numeric to Python
Decimal. The Decimal type is taken from cdecimal.Decimal, if it is
available. It is an external library, quite fast, but may be not available.
If it is not available, then decimal.Decimal will be used. It is in
standard Python library, however it is rather slow.

The initialization is done in the conversion function, the pointer to a
proper Decimal constructor is stored as static variable inside the function
and is lazy initialized.

The documentation is updated.

I've tested this version with python 2.7 with and without cdecimal and
also with 3.3 that has the faster decimal performance. It seems fine.

The v5 version of the patch makes only white-space changes to plpy_main.c
you should excluded that from the patch if your making a new version (I
have done this in the v6 version I'm attaching)

Tests for python 2 and 3 have been added. They work only with standard

decimal.Decimal, as the type is printed in the *.out files. I think there
is nothing we can do with that now.

I think we should make test_type_conversion_numeric to do something that
generates the same output in both cases. ie
py.info(str(x)). I downside of having the test fail on installs with
cdecimal installed is much greater than any benefit we get by ensuring that
the type is really decimal.
I've attached a v6 version of the patch that does this, do you agree with
my thinking?

Hi Steve,
thanks for the changes.

You're idea about common code for decimal and cdecimal is good, however not
good enough. I like the idea of common code for decimal and cdecimal. But
we need class name, not the value.

I've changed the code from str(x) to x.__class__.__name__ so the function
prints class name (which is Decimal for both packages), not the value. We
need to have the class name check. The value is returned by the function
and is a couple of lines lower in the file.

patch is attached.

thanks,
Szymon

Attachments:

plpython_decimal_v7.patchapplication/octet-stream; name=plpython_decimal_v7.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
index aaf758d..da27874 100644
--- a/doc/src/sgml/plpython.sgml
+++ b/doc/src/sgml/plpython.sgml
@@ -308,14 +308,19 @@ $$ LANGUAGE plpythonu;
       </para>
      </listitem>
 
+	 <listitem>
+      <para>
+       PostgreSQL <type>real</type> and <type>double</type> are converted to
+       Python <type>float</type>.
+      </para>
+     </listitem>
+
      <listitem>
       <para>
-       PostgreSQL <type>real</type>, <type>double</type>,
-       and <type>numeric</type> are converted to
-       Python <type>float</type>.  Note that for
-       the <type>numeric</type> this loses information and can lead to
-       incorrect results.  This might be fixed in a future
-       release.
+       PostgreSQL <type>numeric</type> is converted to
+       Python <type>Decimal</type>. This type is imported from 
+	   <literal>cdecimal</literal> package if it is available. If cdecimal
+	   cannot be used, then <literal>decimal.Decimal</literal> will be used.
       </para>
      </listitem>
 
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 4641345..1742114 100644
--- a/src/pl/plpython/expected/plpython_types.out
+++ b/src/pl/plpython/expected/plpython_types.out
@@ -213,36 +213,52 @@ CONTEXT:  PL/Python function "test_type_conversion_int8"
 (1 row)
 
 CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
-plpy.info(x, type(x))
+plpy.info(x.__class__.__name__)
 return x
 $$ LANGUAGE plpythonu;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <type 'float'>)
+INFO:  Decimal
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <type 'float'>)
+INFO:  Decimal
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <type 'float'>)
+INFO:  Decimal
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  Decimal
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  Decimal
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
-INFO:  (None, <type 'NoneType'>)
+INFO:  NoneType
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index 511ef5a..57538b3 100644
--- a/src/pl/plpython/expected/plpython_types_3.out
+++ b/src/pl/plpython/expected/plpython_types_3.out
@@ -213,36 +213,52 @@ CONTEXT:  PL/Python function "test_type_conversion_int8"
 (1 row)
 
 CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
-plpy.info(x, type(x))
+plpy.info(str(x))
 return x
 $$ LANGUAGE plpython3u;
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
-INFO:  (100.0, <class 'float'>)
+INFO:  Decimal
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                        100.0
+                          100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(-100);
-INFO:  (-100.0, <class 'float'>)
+INFO:  Decimal
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
-                       -100.0
+                         -100
 (1 row)
 
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
-INFO:  (5000000000.5, <class 'float'>)
+INFO:  Decimal
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
                  5000000000.5
 (1 row)
 
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+INFO:  Decimal
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+        1234567890.0987654321
+(1 row)
+
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+INFO:  Decimal
+CONTEXT:  PL/Python function "test_type_conversion_numeric"
+ test_type_conversion_numeric 
+------------------------------
+       -1234567890.0987654321
+(1 row)
+
 SELECT * FROM test_type_conversion_numeric(null);
-INFO:  (None, <class 'NoneType'>)
+INFO:  NoneType
 CONTEXT:  PL/Python function "test_type_conversion_numeric"
  test_type_conversion_numeric 
 ------------------------------
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index 6a9a2cb..e04c2f9 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -18,6 +18,7 @@
 #include "utils/memutils.h"
 #include "utils/syscache.h"
 #include "utils/typcache.h"
+#include "utils/numeric.h"
 
 #include "plpython.h"
 
@@ -35,7 +36,7 @@ static void PLy_output_datum_func2(PLyObToDatum *arg, HeapTuple typeTup);
 static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
-static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
+static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
 static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
@@ -450,7 +451,7 @@ PLy_input_datum_func2(PLyDatumToOb *arg, Oid typeOid, HeapTuple typeTup)
 			arg->func = PLyFloat_FromFloat8;
 			break;
 		case NUMERICOID:
-			arg->func = PLyFloat_FromNumeric;
+			arg->func = PLyDecimal_FromNumeric;
 			break;
 		case INT2OID:
 			arg->func = PLyInt_FromInt16;
@@ -516,16 +517,33 @@ PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d)
 }
 
 static PyObject *
-PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
+PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
 {
-	/*
-	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
-	 * it be better to cast to PyString?
-	 */
-	Datum		f = DirectFunctionCall1(numeric_float8, d);
-	double		x = DatumGetFloat8(f);
+	char *x;
+	PyObject *pvalue, *value, *decimal, *decimal_dict;
+	static PyObject *decimal_ctor;
+
+	/* Try to import cdecimal, if it doesnt exist, fallback to decimal */
+	if (decimal_ctor == NULL)
+	{
+		decimal = PyImport_ImportModule("cdecimal");
+		if (decimal == NULL)
+		{
+			PyErr_Clear();
+			decimal = PyImport_ImportModule("decimal");
+		}
+		if (decimal == NULL)
+			PLy_elog(ERROR, "could not import module 'decimal'");
+
+		decimal_dict = PyModule_GetDict(decimal);
+		decimal_ctor = PyDict_GetItemString(decimal_dict, "Decimal");
+		Py_DECREF(decimal_dict);
+	}
 
-	return PyFloat_FromDouble(x);
+	x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
+	pvalue = PyString_FromString(x);
+	value = PyObject_CallFunctionObjArgs(decimal_ctor, pvalue, NULL);
+	return value;
 }
 
 static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 6a50b42..fd7fbca 100644
--- a/src/pl/plpython/sql/plpython_types.sql
+++ b/src/pl/plpython/sql/plpython_types.sql
@@ -86,14 +86,16 @@ SELECT * FROM test_type_conversion_int8(null);
 
 
 CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
-plpy.info(x, type(x))
+plpy.info(x.__class__.__name__)
 return x
 $$ LANGUAGE plpythonu;
 
-/* The current implementation converts numeric to float. */
+/* The current implementation converts numeric to Decimal. */
 SELECT * FROM test_type_conversion_numeric(100);
 SELECT * FROM test_type_conversion_numeric(-100);
 SELECT * FROM test_type_conversion_numeric(5000000000.5);
+SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
 SELECT * FROM test_type_conversion_numeric(null);
 
 
#19Steve Singer
steve@ssinger.info
In reply to: Szymon Guz (#18)
1 attachment(s)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 06/27/2013 05:04 AM, Szymon Guz wrote:

On 27 June 2013 05:21, Steve Singer <steve@ssinger.info
<mailto:steve@ssinger.info>> wrote:

On 06/26/2013 04:47 PM, Szymon Guz wrote:

Hi Steve,
thanks for the changes.

You're idea about common code for decimal and cdecimal is good,
however not good enough. I like the idea of common code for decimal
and cdecimal. But we need class name, not the value.

I've changed the code from str(x) to x.__class__.__name__ so the
function prints class name (which is Decimal for both packages), not
the value. We need to have the class name check. The value is returned
by the function and is a couple of lines lower in the file.

patch is attached.

I think the value is more important than the name, I want to the tests
to make sure that the conversion is actually converting properly. With
your method of getting the class name without the module we can have both.

The attached patch should print the value and the class name but not the
module name.

Steve

Show quoted text

thanks,
Szymon

Attachments:

plpython_decimal_v8.patchtext/x-patch; name=plpython_decimal_v8.patchDownload
diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml
new file mode 100644
index aaf758d..da27874
*** a/doc/src/sgml/plpython.sgml
--- b/doc/src/sgml/plpython.sgml
*************** $$ LANGUAGE plpythonu;
*** 308,321 ****
        </para>
       </listitem>
  
       <listitem>
        <para>
!        PostgreSQL <type>real</type>, <type>double</type>,
!        and <type>numeric</type> are converted to
!        Python <type>float</type>.  Note that for
!        the <type>numeric</type> this loses information and can lead to
!        incorrect results.  This might be fixed in a future
!        release.
        </para>
       </listitem>
  
--- 308,326 ----
        </para>
       </listitem>
  
+ 	 <listitem>
+       <para>
+        PostgreSQL <type>real</type> and <type>double</type> are converted to
+        Python <type>float</type>.
+       </para>
+      </listitem>
+ 
       <listitem>
        <para>
!        PostgreSQL <type>numeric</type> is converted to
!        Python <type>Decimal</type>. This type is imported from 
! 	   <literal>cdecimal</literal> package if it is available. If cdecimal
! 	   cannot be used, then <literal>decimal.Decimal</literal> will be used.
        </para>
       </listitem>
  
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
new file mode 100644
index 4641345..e602336
*** a/src/pl/plpython/expected/plpython_types.out
--- b/src/pl/plpython/expected/plpython_types.out
*************** CONTEXT:  PL/Python function "test_type_
*** 213,248 ****
  (1 row)
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(x, type(x))
  return x
  $$ LANGUAGE plpythonu;
! /* The current implementation converts numeric to float. */
  SELECT * FROM test_type_conversion_numeric(100);
! INFO:  (100.0, <type 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                         100.0
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(-100);
! INFO:  (-100.0, <type 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                        -100.0
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
! INFO:  (5000000000.5, <type 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
                   5000000000.5
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(null);
! INFO:  (None, <type 'NoneType'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
--- 213,264 ----
  (1 row)
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(x,x.__class__.__name__)
  return x
  $$ LANGUAGE plpythonu;
! /* The current implementation converts numeric to Decimal. */
  SELECT * FROM test_type_conversion_numeric(100);
! INFO:  (Decimal('100'), 'Decimal')
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                           100
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(-100);
! INFO:  (Decimal('-100'), 'Decimal')
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                          -100
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
! INFO:  (Decimal('5000000000.5'), 'Decimal')
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
                   5000000000.5
  (1 row)
  
+ SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+ INFO:  (Decimal('1234567890.0987654321'), 'Decimal')
+ CONTEXT:  PL/Python function "test_type_conversion_numeric"
+  test_type_conversion_numeric 
+ ------------------------------
+         1234567890.0987654321
+ (1 row)
+ 
+ SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+ INFO:  (Decimal('-1234567890.0987654321'), 'Decimal')
+ CONTEXT:  PL/Python function "test_type_conversion_numeric"
+  test_type_conversion_numeric 
+ ------------------------------
+        -1234567890.0987654321
+ (1 row)
+ 
  SELECT * FROM test_type_conversion_numeric(null);
! INFO:  (None, 'NoneType')
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
new file mode 100644
index 511ef5a..57538b3
*** a/src/pl/plpython/expected/plpython_types_3.out
--- b/src/pl/plpython/expected/plpython_types_3.out
*************** CONTEXT:  PL/Python function "test_type_
*** 213,248 ****
  (1 row)
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(x, type(x))
  return x
  $$ LANGUAGE plpython3u;
! /* The current implementation converts numeric to float. */
  SELECT * FROM test_type_conversion_numeric(100);
! INFO:  (100.0, <class 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                         100.0
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(-100);
! INFO:  (-100.0, <class 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                        -100.0
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
! INFO:  (5000000000.5, <class 'float'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
                   5000000000.5
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(null);
! INFO:  (None, <class 'NoneType'>)
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
--- 213,264 ----
  (1 row)
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(str(x))
  return x
  $$ LANGUAGE plpython3u;
! /* The current implementation converts numeric to Decimal. */
  SELECT * FROM test_type_conversion_numeric(100);
! INFO:  Decimal
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                           100
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(-100);
! INFO:  Decimal
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
!                          -100
  (1 row)
  
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
! INFO:  Decimal
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
                   5000000000.5
  (1 row)
  
+ SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+ INFO:  Decimal
+ CONTEXT:  PL/Python function "test_type_conversion_numeric"
+  test_type_conversion_numeric 
+ ------------------------------
+         1234567890.0987654321
+ (1 row)
+ 
+ SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
+ INFO:  Decimal
+ CONTEXT:  PL/Python function "test_type_conversion_numeric"
+  test_type_conversion_numeric 
+ ------------------------------
+        -1234567890.0987654321
+ (1 row)
+ 
  SELECT * FROM test_type_conversion_numeric(null);
! INFO:  NoneType
  CONTEXT:  PL/Python function "test_type_conversion_numeric"
   test_type_conversion_numeric 
  ------------------------------
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
new file mode 100644
index 6a9a2cb..e04c2f9
*** a/src/pl/plpython/plpy_typeio.c
--- b/src/pl/plpython/plpy_typeio.c
***************
*** 18,23 ****
--- 18,24 ----
  #include "utils/memutils.h"
  #include "utils/syscache.h"
  #include "utils/typcache.h"
+ #include "utils/numeric.h"
  
  #include "plpython.h"
  
*************** static void PLy_output_datum_func2(PLyOb
*** 35,41 ****
  static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
! static PyObject *PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
--- 36,42 ----
  static PyObject *PLyBool_FromBool(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyFloat_FromFloat4(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyFloat_FromFloat8(PLyDatumToOb *arg, Datum d);
! static PyObject *PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyInt_FromInt16(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyInt_FromInt32(PLyDatumToOb *arg, Datum d);
  static PyObject *PLyLong_FromInt64(PLyDatumToOb *arg, Datum d);
*************** PLy_input_datum_func2(PLyDatumToOb *arg,
*** 450,456 ****
  			arg->func = PLyFloat_FromFloat8;
  			break;
  		case NUMERICOID:
! 			arg->func = PLyFloat_FromNumeric;
  			break;
  		case INT2OID:
  			arg->func = PLyInt_FromInt16;
--- 451,457 ----
  			arg->func = PLyFloat_FromFloat8;
  			break;
  		case NUMERICOID:
! 			arg->func = PLyDecimal_FromNumeric;
  			break;
  		case INT2OID:
  			arg->func = PLyInt_FromInt16;
*************** PLyFloat_FromFloat8(PLyDatumToOb *arg, D
*** 516,531 ****
  }
  
  static PyObject *
! PLyFloat_FromNumeric(PLyDatumToOb *arg, Datum d)
  {
! 	/*
! 	 * Numeric is cast to a PyFloat: This results in a loss of precision Would
! 	 * it be better to cast to PyString?
! 	 */
! 	Datum		f = DirectFunctionCall1(numeric_float8, d);
! 	double		x = DatumGetFloat8(f);
  
! 	return PyFloat_FromDouble(x);
  }
  
  static PyObject *
--- 517,549 ----
  }
  
  static PyObject *
! PLyDecimal_FromNumeric(PLyDatumToOb *arg, Datum d)
  {
! 	char *x;
! 	PyObject *pvalue, *value, *decimal, *decimal_dict;
! 	static PyObject *decimal_ctor;
  
! 	/* Try to import cdecimal, if it doesnt exist, fallback to decimal */
! 	if (decimal_ctor == NULL)
! 	{
! 		decimal = PyImport_ImportModule("cdecimal");
! 		if (decimal == NULL)
! 		{
! 			PyErr_Clear();
! 			decimal = PyImport_ImportModule("decimal");
! 		}
! 		if (decimal == NULL)
! 			PLy_elog(ERROR, "could not import module 'decimal'");
! 
! 		decimal_dict = PyModule_GetDict(decimal);
! 		decimal_ctor = PyDict_GetItemString(decimal_dict, "Decimal");
! 		Py_DECREF(decimal_dict);
! 	}
! 
! 	x = DatumGetCString(DirectFunctionCall1(numeric_out, d));
! 	pvalue = PyString_FromString(x);
! 	value = PyObject_CallFunctionObjArgs(decimal_ctor, pvalue, NULL);
! 	return value;
  }
  
  static PyObject *
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
new file mode 100644
index 6a50b42..0f707cf
*** a/src/pl/plpython/sql/plpython_types.sql
--- b/src/pl/plpython/sql/plpython_types.sql
*************** SELECT * FROM test_type_conversion_int8(
*** 86,99 ****
  
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(x, type(x))
  return x
  $$ LANGUAGE plpythonu;
  
! /* The current implementation converts numeric to float. */
  SELECT * FROM test_type_conversion_numeric(100);
  SELECT * FROM test_type_conversion_numeric(-100);
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
  SELECT * FROM test_type_conversion_numeric(null);
  
  
--- 86,101 ----
  
  
  CREATE FUNCTION test_type_conversion_numeric(x numeric) RETURNS numeric AS $$
! plpy.info(x,x.__class__.__name__)
  return x
  $$ LANGUAGE plpythonu;
  
! /* The current implementation converts numeric to Decimal. */
  SELECT * FROM test_type_conversion_numeric(100);
  SELECT * FROM test_type_conversion_numeric(-100);
  SELECT * FROM test_type_conversion_numeric(5000000000.5);
+ SELECT * FROM test_type_conversion_numeric(1234567890.0987654321);
+ SELECT * FROM test_type_conversion_numeric(-1234567890.0987654321);
  SELECT * FROM test_type_conversion_numeric(null);
  
  
#20Szymon Guz
mabewlun@gmail.com
In reply to: Steve Singer (#19)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 28 June 2013 22:14, Steve Singer <steve@ssinger.info> wrote:

I think the value is more important than the name, I want to the tests to
make sure that the conversion is actually converting properly. With your
method of getting the class name without the module we can have both.

The attached patch should print the value and the class name but not the
module name.

Steve

Hi Steve,
I agree, we can check both. This is quite a nice patch now, I've reviewed
it, all tests pass, works as expected. I think it is ready for committing.

thanks,
Szymon

#21Claudio Freire
klaussfreire@gmail.com
In reply to: Steve Singer (#19)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On Fri, Jun 28, 2013 at 5:14 PM, Steve Singer <steve@ssinger.info> wrote:

On 06/27/2013 05:04 AM, Szymon Guz wrote:

On 27 June 2013 05:21, Steve Singer <steve@ssinger.info
<mailto:steve@ssinger.info>> wrote:

On 06/26/2013 04:47 PM, Szymon Guz wrote:

Hi Steve,
thanks for the changes.

You're idea about common code for decimal and cdecimal is good, however
not good enough. I like the idea of common code for decimal and cdecimal.
But we need class name, not the value.

I've changed the code from str(x) to x.__class__.__name__ so the function
prints class name (which is Decimal for both packages), not the value. We
need to have the class name check. The value is returned by the function and
is a couple of lines lower in the file.

patch is attached.

I think the value is more important than the name, I want to the tests to
make sure that the conversion is actually converting properly. With your
method of getting the class name without the module we can have both.

The attached patch should print the value and the class name but not the
module name.

Why not forego checking of the type, and instead check the interface?

plpy.info(x.as_tuple())

Should do.

d = decimal.Decimal((0,(3,1,4),-2))
d.as_tuple()

DecimalTuple(sign=0, digits=(3, 1, 4), exponent=-2)

d.as_tuple() == (0,(3,1,4),-2)

True

d = decimal.Decimal("3.14")
d.as_tuple()

DecimalTuple(sign=0, digits=(3, 1, 4), exponent=-2)

d.as_tuple() == (0,(3,1,4),-2)

True

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#22Peter Eisentraut
peter_e@gmx.net
In reply to: Szymon Guz (#20)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On Fri, 2013-06-28 at 22:28 +0200, Szymon Guz wrote:

I agree, we can check both. This is quite a nice patch now, I've reviewed
it, all tests pass, works as expected. I think it is ready for committing.

Committed.

Very nice to get that finally fixed.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#23Peter Eisentraut
peter_e@gmx.net
In reply to: Claudio Freire (#21)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On Fri, 2013-06-28 at 17:29 -0300, Claudio Freire wrote:

Why not forego checking of the type, and instead check the interface?

plpy.info(x.as_tuple())

Should do.

d = decimal.Decimal((0,(3,1,4),-2))
d.as_tuple()

DecimalTuple(sign=0, digits=(3, 1, 4), exponent=-2)

I think that potentially exposes us to more version differences and
such, and it doesn't really add much value in the test output.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#24Claudio Freire
klaussfreire@gmail.com
In reply to: Peter Eisentraut (#23)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On Fri, Jul 5, 2013 at 11:47 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On Fri, 2013-06-28 at 17:29 -0300, Claudio Freire wrote:

Why not forego checking of the type, and instead check the interface?

plpy.info(x.as_tuple())

Should do.

d = decimal.Decimal((0,(3,1,4),-2))
d.as_tuple()

DecimalTuple(sign=0, digits=(3, 1, 4), exponent=-2)

I think that potentially exposes us to more version differences and
such, and it doesn't really add much value in the test output.

Why?

That interface is documented, and Python guys aren't likely to change
it, not even in alternative implementations.

Besides, this is the "zen" way of checking. Python uses duck typing,
so checking the actual class of stuff is frowned upon. The "Pythonic"
way to write tests is to check the expected behavior of returned
objects. In this case, that it has an as_tuple that returns the right
thing.

You could also check other interesting behavior if you want, including
its string representation, that it can be converted to float, that two
decimals can be operated upon and that they preserve the right amount
of precision, etc..

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#25Szymon Guz
mabewlun@gmail.com
In reply to: Claudio Freire (#21)
Re: [PATCH] Fix conversion for Decimal arguments in plpython functions

On 28 June 2013 22:29, Claudio Freire <klaussfreire@gmail.com> wrote:

On Fri, Jun 28, 2013 at 5:14 PM, Steve Singer <steve@ssinger.info> wrote:

On 06/27/2013 05:04 AM, Szymon Guz wrote:

On 27 June 2013 05:21, Steve Singer <steve@ssinger.info
<mailto:steve@ssinger.info>> wrote:

On 06/26/2013 04:47 PM, Szymon Guz wrote:

Hi Steve,
thanks for the changes.

You're idea about common code for decimal and cdecimal is good, however
not good enough. I like the idea of common code for decimal and

cdecimal.

But we need class name, not the value.

I've changed the code from str(x) to x.__class__.__name__ so the

function

prints class name (which is Decimal for both packages), not the value.

We

need to have the class name check. The value is returned by the

function and

is a couple of lines lower in the file.

patch is attached.

I think the value is more important than the name, I want to the tests to
make sure that the conversion is actually converting properly. With your
method of getting the class name without the module we can have both.

The attached patch should print the value and the class name but not the
module name.

Why not forego checking of the type, and instead check the interface?

plpy.info(x.as_tuple())

Should do.

d = decimal.Decimal((0,(3,1,4),-2))
d.as_tuple()

DecimalTuple(sign=0, digits=(3, 1, 4), exponent=-2)

d.as_tuple() == (0,(3,1,4),-2)

True

d = decimal.Decimal("3.14")
d.as_tuple()

DecimalTuple(sign=0, digits=(3, 1, 4), exponent=-2)

d.as_tuple() == (0,(3,1,4),-2)

True

Yea, however decimal and cdecimal have different outputs:

For decimal:

! INFO: DecimalTuple(sign=1, digits=(1, 0, 0), exponent=0)

for cdecimal:

! INFO: DecimalTuple(sign=1, digits=(1, 0, 0), exponent=0L)