On 01.02.2018 19:06, Peter Eisentraut wrote:
On 1/12/18 10:43, Aleksander Alekseev wrote:The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: tested, passed Documentation: tested, passedLGTM. The new status of this patch is: Ready for CommitterI've been working on polishing this a bit. I'll keep working on it. It should be ready to commit soon.
Hi. I have reviewed this patch. Attached new 6th version of the patch with v5-v6 delta-patch. * Added out of memory checks after the following function calls: - PyList_New() - PyDict_New() - PyString_FromStringAndSize() (added PyString_FromJsonbValue()) * Added Py_XDECREF() for key-value pairs and list elements after theirs appending because PyDict_SetItem() and PyList_Append() do not steal references (see also hstore_plpython code). * Removed unnecessary JsonbValue heap-allocations in PyObject_ToJsonbValue(). * Added iterating to the end of iterator in PyObject_FromJsonb() for correct freeing of JsonbIterators. * Passed JsonbParseState ** to PyXxx_ToJsonbValue() functions. * Added transformation of Python tuples into JSON arrays because standard Python JSONEncoder encoder does the same. (See https://docs.python.org/2/library/json.html#py-to-json-table) -- Nikita Glukhov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
0001-jsonb_plpython-extension-v6.patch.gz
Description: application/gzip
diff --git a/contrib/jsonb_plpython/expected/jsonb_plpython2u.out b/contrib/jsonb_plpython/expected/jsonb_plpython2u.out
index 7073d52..7d29589 100644
--- a/contrib/jsonb_plpython/expected/jsonb_plpython2u.out
+++ b/contrib/jsonb_plpython/expected/jsonb_plpython2u.out
@@ -341,8 +341,22 @@ SELECT testDecimal();
255
(1 row)
+-- tuple -> jsonb
+CREATE FUNCTION testTuple() RETURNS jsonb
+LANGUAGE plpython2u
+TRANSFORM FOR TYPE jsonb
+AS $$
+x = (1, 'String', None)
+return x
+$$;
+SELECT testTuple();
+ testtuple
+---------------------
+ [1, "String", null]
+(1 row)
+
DROP EXTENSION plpython2u CASCADE;
-NOTICE: drop cascades to 17 other objects
+NOTICE: drop cascades to 18 other objects
DETAIL: drop cascades to extension jsonb_plpython2u
drop cascades to function test1(jsonb)
drop cascades to function test1complex(jsonb)
@@ -360,6 +374,7 @@ drop cascades to function test1set()
drop cascades to function testcomplexnumbers()
drop cascades to function testrange()
drop cascades to function testdecimal()
+drop cascades to function testtuple()
-- test jsonb int -> python int
CREATE EXTENSION jsonb_plpythonu CASCADE;
NOTICE: installing required extension "plpythonu"
diff --git a/contrib/jsonb_plpython/expected/jsonb_plpython3u.out b/contrib/jsonb_plpython/expected/jsonb_plpython3u.out
index 5acd45c..2492858 100644
--- a/contrib/jsonb_plpython/expected/jsonb_plpython3u.out
+++ b/contrib/jsonb_plpython/expected/jsonb_plpython3u.out
@@ -340,8 +340,22 @@ SELECT testDecimal();
255
(1 row)
+-- tuple -> jsonb
+CREATE FUNCTION testTuple() RETURNS jsonb
+LANGUAGE plpython2u
+TRANSFORM FOR TYPE jsonb
+AS $$
+x = (1, 'String', None)
+return x
+$$;
+SELECT testTuple();
+ testtuple
+---------------------
+ [1, "String", null]
+(1 row)
+
DROP EXTENSION plpython3u CASCADE;
-NOTICE: drop cascades to 17 other objects
+NOTICE: drop cascades to 18 other objects
DETAIL: drop cascades to extension jsonb_plpython3u
drop cascades to function test1(jsonb)
drop cascades to function test1complex(jsonb)
@@ -359,3 +373,4 @@ drop cascades to function test1set()
drop cascades to function testcomplexnumbers()
drop cascades to function testrange()
drop cascades to function testdecimal()
+drop cascades to function testtuple()
diff --git a/contrib/jsonb_plpython/jsonb_plpython.c b/contrib/jsonb_plpython/jsonb_plpython.c
index 28f7a3f..705e82f 100644
--- a/contrib/jsonb_plpython/jsonb_plpython.c
+++ b/contrib/jsonb_plpython/jsonb_plpython.c
@@ -21,7 +21,7 @@ static PyObject *decimal_constructor;
static PyObject *PyObject_FromJsonb(JsonbContainer *jsonb);
static JsonbValue *PyObject_ToJsonbValue(PyObject *obj,
- JsonbParseState *jsonb_state);
+ JsonbParseState **jsonb_state, bool is_elem);
#if PY_MAJOR_VERSION >= 3
typedef PyObject *(*PLyUnicode_FromStringAndSize_t)
@@ -53,6 +53,43 @@ _PG_init(void)
#define PLyUnicode_FromStringAndSize (PLyUnicode_FromStringAndSize_p)
/*
+ * PyString_FromJsonbValue
+ *
+ * Transform string JsonbValue into python string
+ */
+static PyObject *
+PyString_FromJsonbValue(JsonbValue *jbv)
+{
+ PyObject *str;
+
+ Assert(jbv->type == jbvString);
+
+ str = PyString_FromStringAndSize(jbv->val.string.val, jbv->val.string.len);
+
+ if (!str)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ return str;
+}
+
+/*
+ * PyString_ToJsonbValue
+ *
+ * Transform python string to JsonbValue
+ */
+static JsonbValue *
+PyString_ToJsonbValue(PyObject *obj, JsonbValue *jbvElem)
+{
+ jbvElem->type = jbvString;
+ jbvElem->val.string.val = PLyObject_AsString(obj);
+ jbvElem->val.string.len = strlen(jbvElem->val.string.val);
+
+ return jbvElem;
+}
+
+/*
* PyObject_FromJsonbValue
*
* Transform JsonbValue into PyObject.
@@ -60,16 +97,14 @@ _PG_init(void)
static PyObject *
PyObject_FromJsonbValue(JsonbValue *jsonbValue)
{
- PyObject *result;
-
switch (jsonbValue->type)
{
case jbvNull:
- result = Py_None;
- break;
+ return Py_None;
+
case jbvBinary:
- result = PyObject_FromJsonb(jsonbValue->val.binary.data);
- break;
+ return PyObject_FromJsonb(jsonbValue->val.binary.data);
+
case jbvNumeric:
{
Datum num;
@@ -77,22 +112,20 @@ PyObject_FromJsonbValue(JsonbValue *jsonbValue)
num = NumericGetDatum(jsonbValue->val.numeric);
str = DatumGetCString(DirectFunctionCall1(numeric_out, num));
- result = PyObject_CallFunction(decimal_constructor, "s", str);
- break;
+
+ return PyObject_CallFunction(decimal_constructor, "s", str);
}
+
case jbvString:
- result = PyString_FromStringAndSize(jsonbValue->val.string.val,
- jsonbValue->val.string.len);
- break;
+ return PyString_FromJsonbValue(jsonbValue);
+
case jbvBool:
- result = jsonbValue->val.boolean ? Py_True : Py_False;
- break;
- case jbvArray:
- case jbvObject:
- result = PyObject_FromJsonb(jsonbValue->val.binary.data);
- break;
+ return jsonbValue->val.boolean ? Py_True : Py_False;
+
+ default:
+ elog(ERROR, "unexpected jsonb value type: %d", jsonbValue->type);
+ return NULL;
}
- return result;
}
/*
@@ -106,10 +139,7 @@ PyObject_FromJsonb(JsonbContainer *jsonb)
JsonbIteratorToken r;
JsonbValue v;
JsonbIterator *it;
-
- PyObject *result,
- *key,
- *value;
+ PyObject *result;
it = JsonbIteratorInit(jsonb);
r = JsonbIteratorNext(&it, &v, true);
@@ -119,47 +149,80 @@ PyObject_FromJsonb(JsonbContainer *jsonb)
case WJB_BEGIN_ARRAY:
if (v.val.array.rawScalar)
{
- r = JsonbIteratorNext(&it, &v, true);
+ JsonbValue tmp;
+
+ if ((r = JsonbIteratorNext(&it, &v, true)) != WJB_ELEM ||
+ (r = JsonbIteratorNext(&it, &tmp, true)) != WJB_END_ARRAY ||
+ (r = JsonbIteratorNext(&it, &tmp, true)) != WJB_DONE)
+ elog(ERROR, "unexpected jsonb token: %d", r);
+
result = PyObject_FromJsonbValue(&v);
}
else
{
/* array in v */
result = PyList_New(0);
- while ((r = JsonbIteratorNext(&it, &v, true)) == WJB_ELEM)
- PyList_Append(result, PyObject_FromJsonbValue(&v));
+ if (!result)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ while ((r = JsonbIteratorNext(&it, &v, true)) != WJB_DONE)
+ {
+ if (r == WJB_ELEM)
+ {
+ PyObject *elem = PyObject_FromJsonbValue(&v);
+
+ PyList_Append(result, elem);
+ Py_XDECREF(elem);
+ }
+ }
}
break;
+
case WJB_BEGIN_OBJECT:
result = PyDict_New();
- while ((r = JsonbIteratorNext(&it, &v, true)) == WJB_KEY)
+ if (!result)
+ ereport(ERROR,
+ (errcode(ERRCODE_OUT_OF_MEMORY),
+ errmsg("out of memory")));
+
+ while ((r = JsonbIteratorNext(&it, &v, true)) != WJB_DONE)
{
- key = PyString_FromStringAndSize(v.val.string.val,
- v.val.string.len);
- r = JsonbIteratorNext(&it, &v, true);
- value = PyObject_FromJsonbValue(&v);
- PyDict_SetItem(result, key, value);
+ if (r == WJB_KEY)
+ {
+ PyObject *key = PyString_FromJsonbValue(&v);
+
+ r = JsonbIteratorNext(&it, &v, true);
+
+ if (r == WJB_VALUE)
+ {
+ PyObject *value = PyObject_FromJsonbValue(&v);
+
+ PyDict_SetItem(result, key, value);
+ Py_XDECREF(value);
+ }
+
+ Py_XDECREF(key);
+ }
}
break;
- case WJB_END_OBJECT:
- pg_unreachable();
- break;
+
default:
- result = PyObject_FromJsonbValue(&v);
- break;
+ elog(ERROR, "unexpected jsonb token: %d", r);
+ return NULL;
}
+
return result;
}
-
-
/*
* PyMapping_ToJsonbValue
*
* Transform python dict to JsonbValue.
*/
static JsonbValue *
-PyMapping_ToJsonbValue(PyObject *obj, JsonbParseState *jsonb_state)
+PyMapping_ToJsonbValue(PyObject *obj, JsonbParseState **jsonb_state)
{
int32 pcount;
JsonbValue *out = NULL;
@@ -174,21 +237,17 @@ PyMapping_ToJsonbValue(PyObject *obj, JsonbParseState *jsonb_state)
{
int32 i;
PyObject *items;
- JsonbValue *jbvValue;
- JsonbValue jbvKey;
items = (PyObject *) items_v;
- pushJsonbValue(&jsonb_state, WJB_BEGIN_OBJECT, NULL);
+
+ pushJsonbValue(jsonb_state, WJB_BEGIN_OBJECT, NULL);
for (i = 0; i < pcount; i++)
{
- PyObject *tuple,
- *key,
- *value;
-
- tuple = PyList_GetItem(items, i);
- key = PyTuple_GetItem(tuple, 0);
- value = PyTuple_GetItem(tuple, 1);
+ JsonbValue jbvKey;
+ PyObject *tuple = PyList_GetItem(items, i),
+ *key = PyTuple_GetItem(tuple, 0),
+ *value = PyTuple_GetItem(tuple, 1);
/* Python dictionary can have None as key */
if (key == Py_None)
@@ -200,17 +259,14 @@ PyMapping_ToJsonbValue(PyObject *obj, JsonbParseState *jsonb_state)
else
{
/* All others types of keys we serialize to string */
- jbvKey.type = jbvString;
- jbvKey.val.string.val = PLyObject_AsString(key);
- jbvKey.val.string.len = strlen(jbvKey.val.string.val);
+ (void) PyString_ToJsonbValue(key, &jbvKey);
}
- pushJsonbValue(&jsonb_state, WJB_KEY, &jbvKey);
- jbvValue = PyObject_ToJsonbValue(value, jsonb_state);
- if (IsAJsonbScalar(jbvValue))
- pushJsonbValue(&jsonb_state, WJB_VALUE, jbvValue);
+ (void) pushJsonbValue(jsonb_state, WJB_KEY, &jbvKey);
+ (void) PyObject_ToJsonbValue(value, jsonb_state, false);
}
- out = pushJsonbValue(&jsonb_state, WJB_END_OBJECT, NULL);
+
+ out = pushJsonbValue(jsonb_state, WJB_END_OBJECT, NULL);
}
PG_CATCH();
{
@@ -218,25 +274,8 @@ PyMapping_ToJsonbValue(PyObject *obj, JsonbParseState *jsonb_state)
PG_RE_THROW();
}
PG_END_TRY();
- return out;
-}
-
-/*
- * PyString_ToJsonbValue
- *
- * Transform python string to JsonbValue
- */
-static JsonbValue *
-PyString_ToJsonbValue(PyObject *obj)
-{
- JsonbValue *jbvElem;
- jbvElem = palloc(sizeof(JsonbValue));
- jbvElem->type = jbvString;
- jbvElem->val.string.val = PLyObject_AsString(obj);
- jbvElem->val.string.len = strlen(jbvElem->val.string.val);
-
- return jbvElem;
+ return out;
}
/*
@@ -246,27 +285,23 @@ PyString_ToJsonbValue(PyObject *obj)
* a state required for jsonb construction.
*/
static JsonbValue *
-PySequence_ToJsonbValue(PyObject *obj, JsonbParseState *jsonb_state)
+PySequence_ToJsonbValue(PyObject *obj, JsonbParseState **jsonb_state)
{
- JsonbValue *jbvElem;
Size i,
pcount;
- JsonbValue *out = NULL;
pcount = PySequence_Size(obj);
- pushJsonbValue(&jsonb_state, WJB_BEGIN_ARRAY, NULL);
+
+ pushJsonbValue(jsonb_state, WJB_BEGIN_ARRAY, NULL);
for (i = 0; i < pcount; i++)
{
- PyObject *value;
+ PyObject *value = PySequence_GetItem(obj, i);
- value = PySequence_GetItem(obj, i);
- jbvElem = PyObject_ToJsonbValue(value, jsonb_state);
- if (IsAJsonbScalar(jbvElem))
- pushJsonbValue(&jsonb_state, WJB_ELEM, jbvElem);
+ (void) PyObject_ToJsonbValue(value, jsonb_state, true);
}
- out = pushJsonbValue(&jsonb_state, WJB_END_ARRAY, NULL);
- return (out);
+
+ return pushJsonbValue(jsonb_state, WJB_END_ARRAY, NULL);
}
/*
@@ -275,11 +310,9 @@ PySequence_ToJsonbValue(PyObject *obj, JsonbParseState *jsonb_state)
* Transform python number to JsonbValue.
*/
static JsonbValue *
-PyNumeric_ToJsonbValue(PyObject *obj)
+PyNumeric_ToJsonbValue(PyObject *obj, JsonbValue *jbvNum)
{
- volatile bool failed = false;
Numeric num;
- JsonbValue *jbvInt;
char *str = PLyObject_AsString(obj);
PG_TRY();
@@ -289,21 +322,20 @@ PyNumeric_ToJsonbValue(PyObject *obj)
}
PG_CATCH();
{
- failed = true;
- }
- PG_END_TRY();
-
- if (failed)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
(errmsg("plpython transformation error"),
- errdetail("the value \"%s\" cannot be transformed to jsonb", str))));
+ errdetail("the value \"%s\" cannot be transformed to jsonb",
+ str))));
+ }
+ PG_END_TRY();
+
+ pfree(str);
- jbvInt = palloc(sizeof(JsonbValue));
- jbvInt->type = jbvNumeric;
- jbvInt->val.numeric = num;
+ jbvNum->type = jbvNumeric;
+ jbvNum->val.numeric = num;
- return jbvInt;
+ return jbvNum;
}
/*
@@ -312,39 +344,54 @@ PyNumeric_ToJsonbValue(PyObject *obj)
* Transform python object to JsonbValue.
*/
static JsonbValue *
-PyObject_ToJsonbValue(PyObject *obj, JsonbParseState *jsonb_state)
+PyObject_ToJsonbValue(PyObject *obj, JsonbParseState **jsonb_state, bool is_elem)
{
- JsonbValue *out = NULL;
+ JsonbValue buf;
+ JsonbValue *out;
if (PyDict_Check(obj))
- out = PyMapping_ToJsonbValue(obj, jsonb_state);
- else if (PyString_Check(obj) || PyUnicode_Check(obj))
- out = PyString_ToJsonbValue(obj);
- else if (PyList_Check(obj))
- out = PySequence_ToJsonbValue(obj, jsonb_state);
- else if ((obj == Py_True) || (obj == Py_False))
+ return PyMapping_ToJsonbValue(obj, jsonb_state);
+ else if (PyList_Check(obj) || PyTuple_Check(obj))
+ return PySequence_ToJsonbValue(obj, jsonb_state);
+
+ /* Allocate JsonbValue in heap only if it is raw scalar value. */
+ out = *jsonb_state ? &buf : palloc(sizeof(JsonbValue));
+
+ if (PyString_Check(obj) || PyUnicode_Check(obj))
{
- out = palloc(sizeof(JsonbValue));
- out->type = jbvBool;
- out->val.boolean = (obj == Py_True);
+ PyString_ToJsonbValue(obj, out);
}
else if (obj == Py_None)
{
- out = palloc(sizeof(JsonbValue));
out->type = jbvNull;
}
+ /*
+ * PyNumber_Check() returns true for booleans, so boolean conversion
+ * should be placed before numeric conversion.
+ */
+ else if (obj == Py_True || obj == Py_False)
+ {
+ out->type = jbvBool;
+ out->val.boolean = (obj == Py_True);
+ }
else if (PyNumber_Check(obj))
- out = PyNumeric_ToJsonbValue(obj);
+ {
+ out = PyNumeric_ToJsonbValue(obj, out);
+ }
else
{
- PyObject* repr = PyObject_Type(obj);
+ PyObject *repr = PyObject_Type(obj);
+
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
(errmsg("plpython transformation error"),
- errdetail("\"%s\" type cannot be transformed to jsonb", PLyObject_AsString(repr)))));
+ errdetail("\"%s\" type cannot be transformed to jsonb",
+ PLyObject_AsString(repr)))));
}
- return out;
+ /* Push result into 'jsonb_state' unless it is raw scalar value. */
+ return *jsonb_state ?
+ pushJsonbValue(jsonb_state, is_elem ? WJB_ELEM : WJB_VALUE, out) : out;
}
/*
@@ -361,7 +408,7 @@ plpython_to_jsonb(PG_FUNCTION_ARGS)
JsonbParseState *jsonb_state = NULL;
obj = (PyObject *) PG_GETARG_POINTER(0);
- out = PyObject_ToJsonbValue(obj, jsonb_state);
+ out = PyObject_ToJsonbValue(obj, &jsonb_state, true);
PG_RETURN_POINTER(JsonbValueToJsonb(out));
}
diff --git a/contrib/jsonb_plpython/sql/jsonb_plpython2u.sql b/contrib/jsonb_plpython/sql/jsonb_plpython2u.sql
index b6f5fb9..81d10b4 100644
--- a/contrib/jsonb_plpython/sql/jsonb_plpython2u.sql
+++ b/contrib/jsonb_plpython/sql/jsonb_plpython2u.sql
@@ -212,6 +212,16 @@ $$;
SELECT testDecimal();
+-- tuple -> jsonb
+CREATE FUNCTION testTuple() RETURNS jsonb
+LANGUAGE plpython2u
+TRANSFORM FOR TYPE jsonb
+AS $$
+x = (1, 'String', None)
+return x
+$$;
+
+SELECT testTuple();
DROP EXTENSION plpython2u CASCADE;
diff --git a/contrib/jsonb_plpython/sql/jsonb_plpython3u.sql b/contrib/jsonb_plpython/sql/jsonb_plpython3u.sql
index ff8a28a..a467fa4 100644
--- a/contrib/jsonb_plpython/sql/jsonb_plpython3u.sql
+++ b/contrib/jsonb_plpython/sql/jsonb_plpython3u.sql
@@ -212,4 +212,15 @@ $$;
SELECT testDecimal();
+-- tuple -> jsonb
+CREATE FUNCTION testTuple() RETURNS jsonb
+LANGUAGE plpython2u
+TRANSFORM FOR TYPE jsonb
+AS $$
+x = (1, 'String', None)
+return x
+$$;
+
+SELECT testTuple();
+
DROP EXTENSION plpython3u CASCADE;
