From 5effedf2e269eb5652baab9d93832c458ab93f55 Mon Sep 17 00:00:00 2001 From: BarneyNearby Date: Fri, 25 Oct 2024 16:39:45 +0300 Subject: [PATCH] [fix] Rework code according to Valgrind's recommendations (#14) * [fix] Rework code according to Valgrind's recommendations * [fix] Fix build pyreindexer with reindexer v.4.15 --------- Co-authored-by: Alexander.A.Utkin --- pyreindexer/lib/include/pyobjtools.cc | 148 ++++++------ pyreindexer/lib/include/pyobjtools.h | 11 +- .../lib/include/queryresults_wrapper.h | 4 +- pyreindexer/lib/src/rawpyreindexer.cc | 217 ++++++++---------- pyreindexer/lib/src/rawpyreindexer.h | 60 +++-- pyreindexer/lib/src/reindexerinterface.h | 4 +- pyreindexer/query_results.py | 6 +- pyreindexer/tests/tests/test_sql.py | 4 +- setup.py | 2 +- 9 files changed, 211 insertions(+), 245 deletions(-) diff --git a/pyreindexer/lib/include/pyobjtools.cc b/pyreindexer/lib/include/pyobjtools.cc index 39e63da..975d017 100644 --- a/pyreindexer/lib/include/pyobjtools.cc +++ b/pyreindexer/lib/include/pyobjtools.cc @@ -4,57 +4,30 @@ namespace pyreindexer { -void pyValueSerialize(PyObject **value, reindexer::WrSerializer &wrSer) { - if (*value == Py_None) { - wrSer << "null"; - } else if (PyBool_Check(*value)) { - bool v = PyLong_AsLong(*value) != 0; - wrSer << v; - } else if (PyFloat_Check(*value)) { - double v = PyFloat_AsDouble(*value); - double intpart; - if (std::modf(v, &intpart) == 0.0) { - wrSer << int64_t(v); - } else { - wrSer << v; - } - } else if (PyLong_Check(*value)) { - long v = PyLong_AsLong(*value); - wrSer << v; - } else if (PyUnicode_Check(*value)) { - const char *v = PyUnicode_AsUTF8(*value); - wrSer.PrintJsonString(v); - } else if (PyList_Check(*value)) { - pyListSerialize(value, wrSer); - } else if (PyDict_Check(*value)) { - pyDictSerialize(value, wrSer); - } else { - throw reindexer::Error(errParseJson, std::string("Unable to parse value of type ") + Py_TYPE(*value)->tp_name); - } -} +void pyValueSerialize(PyObject** value, reindexer::WrSerializer& wrSer); -void pyListSerialize(PyObject **list, reindexer::WrSerializer &wrSer) { +void pyListSerialize(PyObject** list, reindexer::WrSerializer& wrSer) { if (!PyList_Check(*list)) { throw reindexer::Error(errParseJson, std::string("List expected, got ") + Py_TYPE(*list)->tp_name); } wrSer << '['; - unsigned sz = PyList_Size(*list); - for (unsigned i = 0; i < sz; i++) { - PyObject *value = PyList_GetItem(*list, i); - - pyValueSerialize(&value, wrSer); - - if (i < sz - 1) { + Py_ssize_t sz = PyList_Size(*list); + for (Py_ssize_t i = 0; i < sz; ++i) { + if (i > 0) { wrSer << ','; } + + PyObject* value = PyList_GetItem(*list, i); + + pyValueSerialize(&value, wrSer); } wrSer << ']'; } -void pyDictSerialize(PyObject **dict, reindexer::WrSerializer &wrSer) { +void pyDictSerialize(PyObject** dict, reindexer::WrSerializer& wrSer) { if (!PyDict_Check(*dict)) { throw reindexer::Error(errParseJson, std::string("Dictionary expected, got ") + Py_TYPE(*dict)->tp_name); } @@ -62,47 +35,72 @@ void pyDictSerialize(PyObject **dict, reindexer::WrSerializer &wrSer) { wrSer << '{'; Py_ssize_t sz = PyDict_Size(*dict); - if (!sz) { - wrSer << '}'; + if (sz) { + PyObject *key = nullptr, *value = nullptr; + Py_ssize_t pos = 0; - return; - } - - PyObject *key, *value; - Py_ssize_t pos = 0; - - while (PyDict_Next(*dict, &pos, &key, &value)) { - const char *k = PyUnicode_AsUTF8(key); - wrSer.PrintJsonString(k); - wrSer << ':'; + while (PyDict_Next(*dict, &pos, &key, &value)) { + if (pos > 1) { + wrSer << ','; + } - pyValueSerialize(&value, wrSer); + const char* k = PyUnicode_AsUTF8(key); + wrSer.PrintJsonString(k); + wrSer << ':'; - if (pos != sz) { - wrSer << ','; + pyValueSerialize(&value, wrSer); } } wrSer << '}'; } -void PyObjectToJson(PyObject **obj, reindexer::WrSerializer &wrSer) { +void pyValueSerialize(PyObject** value, reindexer::WrSerializer& wrSer) { + if (*value == Py_None) { + wrSer << "null"; + } else if (PyBool_Check(*value)) { + bool v = PyLong_AsLong(*value) != 0; + wrSer << v; + } else if (PyFloat_Check(*value)) { + double v = PyFloat_AsDouble(*value); + double intpart; + if (std::modf(v, &intpart) == 0.0) { + wrSer << int64_t(v); + } else { + wrSer << v; + } + } else if (PyLong_Check(*value)) { + long v = PyLong_AsLong(*value); + wrSer << v; + } else if (PyUnicode_Check(*value)) { + const char* v = PyUnicode_AsUTF8(*value); + wrSer.PrintJsonString(v); + } else if (PyList_Check(*value)) { + pyListSerialize(value, wrSer); + } else if (PyDict_Check(*value)) { + pyDictSerialize(value, wrSer); + } else { + throw reindexer::Error(errParseJson, std::string("Unable to parse value of type ") + Py_TYPE(*value)->tp_name); + } +} + +void PyObjectToJson(PyObject** obj, reindexer::WrSerializer& wrSer) { if (PyDict_Check(*obj)) { pyDictSerialize(obj, wrSer); } else if (PyList_Check(*obj) ) { pyListSerialize(obj, wrSer); } else { - throw reindexer::Error(errParseJson, + throw reindexer::Error(errParseJson, std::string("PyObject must be a dictionary or a list for JSON serializing, got ") + Py_TYPE(*obj)->tp_name); } } -std::vector ParseListToStrVec(PyObject **list) { +std::vector ParseListToStrVec(PyObject** list) { std::vector vec; Py_ssize_t sz = PyList_Size(*list); for (Py_ssize_t i = 0; i < sz; i++) { - PyObject *item = PyList_GetItem(*list, i); + PyObject* item = PyList_GetItem(*list, i); if (!PyUnicode_Check(item)) { throw reindexer::Error(errParseJson, std::string("String expected, got ") + Py_TYPE(item)->tp_name); @@ -114,40 +112,49 @@ std::vector ParseListToStrVec(PyObject **list) { return vec; } -PyObject *pyValueFromJsonValue(const gason::JsonValue &value) { - PyObject *pyValue = nullptr; +PyObject* pyValueFromJsonValue(const gason::JsonValue& value) { + PyObject* pyValue = nullptr; switch (value.getTag()) { case gason::JSON_NUMBER: - pyValue = PyLong_FromSize_t(value.toNumber()); + pyValue = PyLong_FromSize_t(value.toNumber()); // new ref break; case gason::JSON_DOUBLE: - pyValue = PyFloat_FromDouble(value.toDouble()); + pyValue = PyFloat_FromDouble(value.toDouble()); // new ref break; case gason::JSON_STRING: { auto sv = value.toString(); - pyValue = PyUnicode_FromStringAndSize(sv.data(), sv.size()); + pyValue = PyUnicode_FromStringAndSize(sv.data(), sv.size()); // new ref break; } case gason::JSON_NULL: pyValue = Py_None; + Py_INCREF(pyValue); // new ref break; case gason::JSON_TRUE: pyValue = Py_True; + Py_INCREF(pyValue); // new ref break; case gason::JSON_FALSE: pyValue = Py_False; + Py_INCREF(pyValue); // new ref break; case gason::JSON_ARRAY: - pyValue = PyList_New(0); - for (const auto &v : value) { - PyList_Append(pyValue, pyValueFromJsonValue(v.value)); + pyValue = PyList_New(0); // new ref + for (const auto& v : value) { + PyObject* dictFromJson = pyValueFromJsonValue(v.value); // stolen ref + PyList_Append(pyValue, dictFromJson); // new ref + Py_XDECREF(dictFromJson); } break; case gason::JSON_OBJECT: - pyValue = PyDict_New(); - for (const auto &v : value) { - PyDict_SetItem(pyValue, PyUnicode_FromStringAndSize(v.key.data(), v.key.size()), pyValueFromJsonValue(v.value)); + pyValue = PyDict_New(); // new ref + for (const auto& v : value) { + PyObject* dictFromJson = pyValueFromJsonValue(v.value); // stolen ref + PyObject* pyKey = PyUnicode_FromStringAndSize(v.key.data(), v.key.size()); // new ref + PyDict_SetItem(pyValue, pyKey, dictFromJson); // new refs + Py_XDECREF(pyKey); + Py_XDECREF(dictFromJson); } break; } @@ -155,13 +162,12 @@ PyObject *pyValueFromJsonValue(const gason::JsonValue &value) { return pyValue; } -PyObject *PyObjectFromJson(reindexer::span json) { +PyObject* PyObjectFromJson(reindexer::span json) { try { gason::JsonParser parser; auto root = parser.Parse(json); - // new ref - return pyValueFromJsonValue(root.value); - } catch (const gason::Exception &ex) { + return pyValueFromJsonValue(root.value); // stolen ref + } catch (const gason::Exception& ex) { throw reindexer::Error(errParseJson, std::string("PyObjectFromJson: ") + ex.what()); } } diff --git a/pyreindexer/lib/include/pyobjtools.h b/pyreindexer/lib/include/pyobjtools.h index d45bbfe..89e98e2 100644 --- a/pyreindexer/lib/include/pyobjtools.h +++ b/pyreindexer/lib/include/pyobjtools.h @@ -8,14 +8,9 @@ namespace pyreindexer { -void pyValueSerialize(PyObject **item, reindexer::WrSerializer &wrSer); -void pyListSerialize(PyObject **list, reindexer::WrSerializer &wrSer); -void pyDictSerialize(PyObject **dict, reindexer::WrSerializer &wrSer); -PyObject *pyValueFromJsonValue(const gason::JsonValue &value); +std::vector ParseListToStrVec(PyObject** dict); -std::vector ParseListToStrVec(PyObject **dict); - -void PyObjectToJson(PyObject **dict, reindexer::WrSerializer &wrSer); -PyObject *PyObjectFromJson(reindexer::span json); +void PyObjectToJson(PyObject** dict, reindexer::WrSerializer& wrSer); +PyObject* PyObjectFromJson(reindexer::span json); } // namespace pyreindexer diff --git a/pyreindexer/lib/include/queryresults_wrapper.h b/pyreindexer/lib/include/queryresults_wrapper.h index 37d7214..e3f3295 100644 --- a/pyreindexer/lib/include/queryresults_wrapper.h +++ b/pyreindexer/lib/include/queryresults_wrapper.h @@ -29,8 +29,8 @@ class QueryResultsWrapper { db_->FetchResults(*this); } - const std::vector& GetAggregationResults() const& { return qresPtr.GetAggregationResults(); } - const std::vector& GetAggregationResults() const&& = delete; + const std::vector& GetAggregationResults() & { return qresPtr.GetAggregationResults(); } + const std::vector& GetAggregationResults() && = delete; private: friend DBInterface; diff --git a/pyreindexer/lib/src/rawpyreindexer.cc b/pyreindexer/lib/src/rawpyreindexer.cc index b20d87c..525cea7 100644 --- a/pyreindexer/lib/src/rawpyreindexer.cc +++ b/pyreindexer/lib/src/rawpyreindexer.cc @@ -3,33 +3,34 @@ namespace pyreindexer { static void queryResultsWrapperDelete(uintptr_t qresWrapperAddr) { - QueryResultsWrapper *qresWrapperPtr = getQueryResultsWrapper(qresWrapperAddr); - + QueryResultsWrapper* qresWrapperPtr = getQueryResultsWrapper(qresWrapperAddr); delete qresWrapperPtr; } -static PyObject *queryResultsWrapperIterate(uintptr_t qresWrapperAddr) { - QueryResultsWrapper *qresWrapperPtr = getQueryResultsWrapper(qresWrapperAddr); +static PyObject* queryResultsWrapperIterate(uintptr_t qresWrapperAddr) { + QueryResultsWrapper* qresWrapperPtr = getQueryResultsWrapper(qresWrapperAddr); WrSerializer wrSer; qresWrapperPtr->GetItemJSON(wrSer, false); qresWrapperPtr->Next(); - PyObject *dictFromJson = nullptr; + PyObject* dictFromJson = nullptr; try { dictFromJson = PyObjectFromJson(reindexer::giftStr(wrSer.Slice())); // stolen ref - } catch (const Error &err) { + } catch (const Error& err) { Py_XDECREF(dictFromJson); return Py_BuildValue("is{}", err.code(), err.what().c_str()); } - return Py_BuildValue("isO", errOK, "", dictFromJson); + PyObject* res = Py_BuildValue("isO", errOK, "", dictFromJson); // new ref + Py_DECREF(dictFromJson); + + return res; } -static PyObject *Init(PyObject *self, PyObject *args) { +static PyObject* Init(PyObject* self, PyObject* args) { uintptr_t rx = initReindexer(); - if (rx == 0) { PyErr_SetString(PyExc_RuntimeError, "Initialization error"); @@ -39,9 +40,8 @@ static PyObject *Init(PyObject *self, PyObject *args) { return Py_BuildValue("k", rx); } -static PyObject *Destroy(PyObject *self, PyObject *args) { +static PyObject* Destroy(PyObject* self, PyObject* args) { uintptr_t rx = 0; - if (!PyArg_ParseTuple(args, "k", &rx)) { return nullptr; } @@ -51,63 +51,54 @@ static PyObject *Destroy(PyObject *self, PyObject *args) { Py_RETURN_NONE; } -static PyObject *Connect(PyObject *self, PyObject *args) { +static PyObject* Connect(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *dsn = nullptr; - + char* dsn = nullptr; if (!PyArg_ParseTuple(args, "ks", &rx, &dsn)) { return nullptr; } Error err = getDB(rx)->Connect(dsn); - return pyErr(err); } -static PyObject *NamespaceOpen(PyObject *self, PyObject *args) { +static PyObject* NamespaceOpen(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - + char* ns = nullptr; if (!PyArg_ParseTuple(args, "ks", &rx, &ns)) { return nullptr; } Error err = getDB(rx)->OpenNamespace(ns); - return pyErr(err); } -static PyObject *NamespaceClose(PyObject *self, PyObject *args) { +static PyObject* NamespaceClose(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - + char* ns = nullptr; if (!PyArg_ParseTuple(args, "ks", &rx, &ns)) { return nullptr; } Error err = getDB(rx)->CloseNamespace(ns); - return pyErr(err); } -static PyObject *NamespaceDrop(PyObject *self, PyObject *args) { +static PyObject* NamespaceDrop(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - + char* ns = nullptr; if (!PyArg_ParseTuple(args, "ks", &rx, &ns)) { return nullptr; } Error err = getDB(rx)->DropNamespace(ns); - return pyErr(err); } -static PyObject *IndexAdd(PyObject *self, PyObject *args) { +static PyObject* IndexAdd(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - PyObject *indexDefDict = nullptr; // borrowed ref after ParseTuple - + char* ns = nullptr; + PyObject* indexDefDict = nullptr; // borrowed ref after ParseTuple if (!PyArg_ParseTuple(args, "ksO!", &rx, &ns, &PyDict_Type, &indexDefDict)) { return nullptr; } @@ -118,7 +109,7 @@ static PyObject *IndexAdd(PyObject *self, PyObject *args) { try { PyObjectToJson(&indexDefDict, wrSer); - } catch (const Error &err) { + } catch (const Error& err) { Py_DECREF(indexDefDict); return pyErr(err); @@ -128,20 +119,16 @@ static PyObject *IndexAdd(PyObject *self, PyObject *args) { IndexDef indexDef; Error err = indexDef.FromJSON(reindexer::giftStr(wrSer.Slice())); - if (!err.ok()) { - return pyErr(err); + if (err.ok()) { + err = getDB(rx)->AddIndex(ns, indexDef); } - - err = getDB(rx)->AddIndex(ns, indexDef); - return pyErr(err); } -static PyObject *IndexUpdate(PyObject *self, PyObject *args) { +static PyObject* IndexUpdate(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - PyObject *indexDefDict = nullptr; // borrowed ref after ParseTuple - + char* ns = nullptr; + PyObject* indexDefDict = nullptr; // borrowed ref after ParseTuple if (!PyArg_ParseTuple(args, "ksO!", &rx, &ns, &PyDict_Type, &indexDefDict)) { return nullptr; } @@ -152,7 +139,7 @@ static PyObject *IndexUpdate(PyObject *self, PyObject *args) { try { PyObjectToJson(&indexDefDict, wrSer); - } catch (const Error &err) { + } catch (const Error& err) { Py_DECREF(indexDefDict); return pyErr(err); @@ -162,35 +149,28 @@ static PyObject *IndexUpdate(PyObject *self, PyObject *args) { IndexDef indexDef; Error err = indexDef.FromJSON(reindexer::giftStr(wrSer.Slice())); - if (!err.ok()) { - return pyErr(err); + if (err.ok()) { + err = getDB(rx)->UpdateIndex(ns, indexDef); } - - err = getDB(rx)->UpdateIndex(ns, indexDef); - return pyErr(err); } -static PyObject *IndexDrop(PyObject *self, PyObject *args) { +static PyObject* IndexDrop(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - char *indexName = nullptr; - + char *ns = nullptr, *indexName = nullptr; if (!PyArg_ParseTuple(args, "kss", &rx, &ns, &indexName)) { return nullptr; } Error err = getDB(rx)->DropIndex(ns, IndexDef(indexName)); - return pyErr(err); } -static PyObject *itemModify(PyObject *self, PyObject *args, ItemModifyMode mode) { +static PyObject* itemModify(PyObject* self, PyObject* args, ItemModifyMode mode) { uintptr_t rx = 0; - char *ns = nullptr; - PyObject *itemDefDict = nullptr; // borrowed ref after ParseTuple - PyObject *preceptsList = nullptr; // borrowed ref after ParseTuple if passed - + char* ns = nullptr; + PyObject* itemDefDict = nullptr; // borrowed ref after ParseTuple + PyObject* preceptsList = nullptr; // borrowed ref after ParseTuple if passed if (!PyArg_ParseTuple(args, "ksO!|O!", &rx, &ns, &PyDict_Type, &itemDefDict, &PyList_Type, &preceptsList)) { return nullptr; } @@ -208,7 +188,7 @@ static PyObject *itemModify(PyObject *self, PyObject *args, ItemModifyMode mode) try { PyObjectToJson(&itemDefDict, wrSer); - } catch (const Error &err) { + } catch (const Error& err) { Py_DECREF(itemDefDict); Py_XDECREF(preceptsList); @@ -217,11 +197,10 @@ static PyObject *itemModify(PyObject *self, PyObject *args, ItemModifyMode mode) Py_DECREF(itemDefDict); - char *json = const_cast(wrSer.c_str()); - + char* json = const_cast(wrSer.c_str()); err = item.Unsafe().FromJSON(json, 0, mode == ModeDelete); if (!err.ok()) { - Py_XDECREF(preceptsList); + Py_XDECREF(preceptsList); return pyErr(err); } @@ -231,7 +210,7 @@ static PyObject *itemModify(PyObject *self, PyObject *args, ItemModifyMode mode) try { itemPrecepts = ParseListToStrVec(&preceptsList); - } catch (const Error &err) { + } catch (const Error& err) { Py_DECREF(preceptsList); return pyErr(err); @@ -263,78 +242,67 @@ static PyObject *itemModify(PyObject *self, PyObject *args, ItemModifyMode mode) return pyErr(err); } -static PyObject *ItemInsert(PyObject *self, PyObject *args) { return itemModify(self, args, ModeInsert); } - -static PyObject *ItemUpdate(PyObject *self, PyObject *args) { return itemModify(self, args, ModeUpdate); } - -static PyObject *ItemUpsert(PyObject *self, PyObject *args) { return itemModify(self, args, ModeUpsert); } - -static PyObject *ItemDelete(PyObject *self, PyObject *args) { return itemModify(self, args, ModeDelete); } +static PyObject* ItemInsert(PyObject* self, PyObject* args) { return itemModify(self, args, ModeInsert); } +static PyObject* ItemUpdate(PyObject* self, PyObject* args) { return itemModify(self, args, ModeUpdate); } +static PyObject* ItemUpsert(PyObject* self, PyObject* args) { return itemModify(self, args, ModeUpsert); } +static PyObject* ItemDelete(PyObject* self, PyObject* args) { return itemModify(self, args, ModeDelete); } -static PyObject *PutMeta(PyObject *self, PyObject *args) { +static PyObject* PutMeta(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - char *key = nullptr; - char *value = nullptr; - + char *ns = nullptr, *key = nullptr, *value = nullptr; if (!PyArg_ParseTuple(args, "ksss", &rx, &ns, &key, &value)) { return nullptr; } Error err = getDB(rx)->PutMeta(ns, key, value); - return pyErr(err); } -static PyObject *GetMeta(PyObject *self, PyObject *args) { +static PyObject* GetMeta(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - char *key = nullptr; - + char *ns = nullptr, *key = nullptr; if (!PyArg_ParseTuple(args, "kss", &rx, &ns, &key)) { return nullptr; } std::string value; Error err = getDB(rx)->GetMeta(ns, key, value); - return Py_BuildValue("iss", err.code(), err.what().c_str(), value.c_str()); } -static PyObject *DeleteMeta(PyObject *self, PyObject *args) { +static PyObject* DeleteMeta(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - char *key = nullptr; - + char *ns = nullptr, *key = nullptr; if (!PyArg_ParseTuple(args, "kss", &rx, &ns, &key)) { return nullptr; } Error err = getDB(rx)->DeleteMeta(ns, key); - return pyErr(err); } -static PyObject *Select(PyObject *self, PyObject *args) { +static PyObject* Select(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *query = nullptr; - + char* query = nullptr; if (!PyArg_ParseTuple(args, "ks", &rx, &query)) { return nullptr; } - auto db = getDB(rx); - QueryResultsWrapper *qresWrapper = new QueryResultsWrapper(); + auto qresWrapper = new QueryResultsWrapper(); + Error err = getDB(rx)->Select(query, *qresWrapper); + + if (!err.ok()) { + delete qresWrapper; - Error err = db->Select(query, *qresWrapper); + return Py_BuildValue("iskI", err.code(), err.what().c_str(), 0, 0); + } return Py_BuildValue("iskI", err.code(), err.what().c_str(), reinterpret_cast(qresWrapper), qresWrapper->Count()); } -static PyObject *EnumMeta(PyObject *self, PyObject *args) { +static PyObject* EnumMeta(PyObject* self, PyObject* args) { uintptr_t rx = 0; - char *ns = nullptr; - + char* ns = nullptr; if (!PyArg_ParseTuple(args, "ks", &rx, &ns)) { return nullptr; } @@ -345,27 +313,27 @@ static PyObject *EnumMeta(PyObject *self, PyObject *args) { return Py_BuildValue("is[]", err.code(), err.what().c_str()); } - PyObject *list = PyList_New(keys.size()); // new ref + PyObject* list = PyList_New(keys.size()); // new ref if (!list) { return nullptr; } - for (auto it = keys.begin(); it != keys.end(); it++) { - unsigned pos = std::distance(keys.begin(), it); - - PyList_SetItem(list, pos, Py_BuildValue("s", it->c_str())); // stolen ref + Py_ssize_t pos = 0; + for (const auto& key : keys) { + PyObject* pyKey = PyUnicode_FromStringAndSize(key.data(), key.size()); // new ref + PyList_SetItem(list, pos, pyKey); // stolen ref + ++pos; } - PyObject *res = Py_BuildValue("isO", err.code(), err.what().c_str(), list); + PyObject* res = Py_BuildValue("isO", err.code(), err.what().c_str(), list); Py_DECREF(list); return res; } -static PyObject *EnumNamespaces(PyObject *self, PyObject *args) { +static PyObject* EnumNamespaces(PyObject* self, PyObject* args) { uintptr_t rx = 0; unsigned enumAll = 0; - if (!PyArg_ParseTuple(args, "kI", &rx, &enumAll)) { return nullptr; } @@ -376,22 +344,21 @@ static PyObject *EnumNamespaces(PyObject *self, PyObject *args) { return Py_BuildValue("is[]", err.code(), err.what().c_str()); } - PyObject *list = PyList_New(nsDefs.size()); // new ref + PyObject* list = PyList_New(nsDefs.size()); // new ref if (!list) { return nullptr; } WrSerializer wrSer; - for (auto it = nsDefs.begin(); it != nsDefs.end(); it++) { - unsigned pos = std::distance(nsDefs.begin(), it); - + Py_ssize_t pos = 0; + for (const auto& ns : nsDefs) { wrSer.Reset(); - it->GetJSON(wrSer, false); + ns.GetJSON(wrSer, false); - PyObject *dictFromJson = nullptr; + PyObject* dictFromJson = nullptr; try { dictFromJson = PyObjectFromJson(reindexer::giftStr(wrSer.Slice())); // stolen ref - } catch (const Error &err) { + } catch (const Error& err) { Py_XDECREF(dictFromJson); Py_DECREF(list); @@ -399,17 +366,17 @@ static PyObject *EnumNamespaces(PyObject *self, PyObject *args) { } PyList_SetItem(list, pos, dictFromJson); // stolen ref + ++pos; } - PyObject *res = Py_BuildValue("isO", err.code(), err.what().c_str(), list); + PyObject* res = Py_BuildValue("isO", err.code(), err.what().c_str(), list); Py_DECREF(list); return res; } -static PyObject *QueryResultsWrapperIterate(PyObject *self, PyObject *args) { +static PyObject* QueryResultsWrapperIterate(PyObject* self, PyObject* args) { uintptr_t qresWrapperAddr = 0; - if (!PyArg_ParseTuple(args, "k", &qresWrapperAddr)) { return nullptr; } @@ -417,9 +384,8 @@ static PyObject *QueryResultsWrapperIterate(PyObject *self, PyObject *args) { return queryResultsWrapperIterate(qresWrapperAddr); } -static PyObject *QueryResultsWrapperDelete(PyObject *self, PyObject *args) { +static PyObject* QueryResultsWrapperDelete(PyObject* self, PyObject* args) { uintptr_t qresWrapperAddr = 0; - if (!PyArg_ParseTuple(args, "k", &qresWrapperAddr)) { return nullptr; } @@ -429,36 +395,39 @@ static PyObject *QueryResultsWrapperDelete(PyObject *self, PyObject *args) { Py_RETURN_NONE; } -static PyObject *GetAggregationResults(PyObject *self, PyObject *args) { - uintptr_t qresWrapperAddr; - +static PyObject* GetAggregationResults(PyObject* self, PyObject* args) { + uintptr_t qresWrapperAddr = 0; if (!PyArg_ParseTuple(args, "k", &qresWrapperAddr)) { - return NULL; + return nullptr; } - QueryResultsWrapper *qresWrapper = getQueryResultsWrapper(qresWrapperAddr); + QueryResultsWrapper* qresWrapper = getQueryResultsWrapper(qresWrapperAddr); - const auto &aggResults = qresWrapper->GetAggregationResults(); + const auto& aggResults = qresWrapper->GetAggregationResults(); WrSerializer wrSer; wrSer << "["; for (size_t i = 0; i < aggResults.size(); ++i) { if (i > 0) { wrSer << ','; } + aggResults[i].GetJSON(wrSer); } wrSer << "]"; - PyObject *dictFromJson = nullptr; + PyObject* dictFromJson = nullptr; try { - dictFromJson = PyObjectFromJson(reindexer::giftStr(wrSer.Slice())); // stolen ref - } catch (const Error &err) { + dictFromJson = PyObjectFromJson(reindexer::giftStr(wrSer.Slice())); // stolen ref + } catch (const Error& err) { Py_XDECREF(dictFromJson); return Py_BuildValue("is{}", err.code(), err.what().c_str()); } - return Py_BuildValue("isO", errOK, "", dictFromJson); + PyObject* res = Py_BuildValue("isO", errOK, "", dictFromJson); // new ref + Py_DECREF(dictFromJson); + + return res; } } // namespace pyreindexer diff --git a/pyreindexer/lib/src/rawpyreindexer.h b/pyreindexer/lib/src/rawpyreindexer.h index 6b8e676..83ab10f 100644 --- a/pyreindexer/lib/src/rawpyreindexer.h +++ b/pyreindexer/lib/src/rawpyreindexer.h @@ -30,50 +30,48 @@ using reindexer::NamespaceDef; using reindexer::WrSerializer; inline static uintptr_t initReindexer() { - DBInterface *db = new DBInterface(); - + DBInterface* db = new DBInterface(); return reinterpret_cast(db); } -inline static DBInterface *getDB(uintptr_t rx) { return reinterpret_cast(rx); } +inline static DBInterface* getDB(uintptr_t rx) { return reinterpret_cast(rx); } inline static void destroyReindexer(uintptr_t rx) { - DBInterface *db = getDB(rx); - + DBInterface* db = getDB(rx); delete db; } -inline static PyObject *pyErr(const Error &err) { return Py_BuildValue("is", err.code(), err.what().c_str()); } +inline static PyObject* pyErr(const Error& err) { return Py_BuildValue("is", err.code(), err.what().c_str()); } -inline static QueryResultsWrapper *getQueryResultsWrapper(uintptr_t qresWrapperAddr) { - return reinterpret_cast(qresWrapperAddr); +inline static QueryResultsWrapper* getQueryResultsWrapper(uintptr_t qresWrapperAddr) { + return reinterpret_cast(qresWrapperAddr); } static void queryResultsWrapperDelete(uintptr_t qresWrapperAddr); -static PyObject *Init(PyObject *self, PyObject *args); -static PyObject *Destroy(PyObject *self, PyObject *args); -static PyObject *Connect(PyObject *self, PyObject *args); -static PyObject *NamespaceOpen(PyObject *self, PyObject *args); -static PyObject *NamespaceClose(PyObject *self, PyObject *args); -static PyObject *NamespaceDrop(PyObject *self, PyObject *args); -static PyObject *IndexAdd(PyObject *self, PyObject *args); -static PyObject *IndexUpdate(PyObject *self, PyObject *args); -static PyObject *IndexDrop(PyObject *self, PyObject *args); -static PyObject *ItemInsert(PyObject *self, PyObject *args); -static PyObject *ItemUpdate(PyObject *self, PyObject *args); -static PyObject *ItemUpsert(PyObject *self, PyObject *args); -static PyObject *ItemDelete(PyObject *self, PyObject *args); -static PyObject *PutMeta(PyObject *self, PyObject *args); -static PyObject *GetMeta(PyObject *self, PyObject *args); -static PyObject *DeleteMeta(PyObject *self, PyObject *args); -static PyObject *Select(PyObject *self, PyObject *args); -static PyObject *EnumMeta(PyObject *self, PyObject *args); -static PyObject *EnumNamespaces(PyObject *self, PyObject *args); - -static PyObject *QueryResultsWrapperIterate(PyObject *self, PyObject *args); -static PyObject *QueryResultsWrapperDelete(PyObject *self, PyObject *args); -static PyObject *GetAggregationResults(PyObject *self, PyObject *args); +static PyObject* Init(PyObject* self, PyObject* args); +static PyObject* Destroy(PyObject* self, PyObject* args); +static PyObject* Connect(PyObject* self, PyObject* args); +static PyObject* NamespaceOpen(PyObject* self, PyObject* args); +static PyObject* NamespaceClose(PyObject* self, PyObject* args); +static PyObject* NamespaceDrop(PyObject* self, PyObject* args); +static PyObject* IndexAdd(PyObject* self, PyObject* args); +static PyObject* IndexUpdate(PyObject* self, PyObject* args); +static PyObject* IndexDrop(PyObject* self, PyObject* args); +static PyObject* ItemInsert(PyObject* self, PyObject* args); +static PyObject* ItemUpdate(PyObject* self, PyObject* args); +static PyObject* ItemUpsert(PyObject* self, PyObject* args); +static PyObject* ItemDelete(PyObject* self, PyObject* args); +static PyObject* PutMeta(PyObject* self, PyObject* args); +static PyObject* GetMeta(PyObject* self, PyObject* args); +static PyObject* DeleteMeta(PyObject* self, PyObject* args); +static PyObject* Select(PyObject* self, PyObject* args); +static PyObject* EnumMeta(PyObject* self, PyObject* args); +static PyObject* EnumNamespaces(PyObject* self, PyObject* args); + +static PyObject* QueryResultsWrapperIterate(PyObject* self, PyObject* args); +static PyObject* QueryResultsWrapperDelete(PyObject* self, PyObject* args); +static PyObject* GetAggregationResults(PyObject* self, PyObject* args); // clang-format off static PyMethodDef module_methods[] = { diff --git a/pyreindexer/lib/src/reindexerinterface.h b/pyreindexer/lib/src/reindexerinterface.h index 435d857..1e31e58 100644 --- a/pyreindexer/lib/src/reindexerinterface.h +++ b/pyreindexer/lib/src/reindexerinterface.h @@ -104,7 +104,7 @@ class ReindexerInterface { Error EnumMeta(std::string_view ns, std::vector& keys) { return execute([this, ns, &keys] { return enumMeta(ns, keys); }); } - Error Select(const std::string &query, QueryResultsWrapper& result); + Error Select(const std::string& query, QueryResultsWrapper& result); Error EnumNamespaces(std::vector& defs, EnumNamespacesOpts opts) { return execute([this, &defs, &opts] { return enumNamespaces(defs, opts); }); } @@ -129,7 +129,7 @@ class ReindexerInterface { Error getMeta(std::string_view ns, const std::string& key, std::string& data) { return db_.GetMeta({ns.data(), ns.size()}, key, data); } Error deleteMeta(std::string_view ns, const std::string& key) { return db_.DeleteMeta({ns.data(), ns.size()}, key); } Error enumMeta(std::string_view ns, std::vector& keys) { return db_.EnumMeta({ns.data(), ns.size()}, keys); } - Error select(const std::string &query, typename DBT::QueryResultsT& result) { return db_.Select(query, result); } + Error select(const std::string& query, typename DBT::QueryResultsT& result) { return db_.Select(query, result); } Error enumNamespaces(std::vector& defs, EnumNamespacesOpts opts) { return db_.EnumNamespaces(defs, opts); } Error stop(); diff --git a/pyreindexer/query_results.py b/pyreindexer/query_results.py index be70d93..decb257 100644 --- a/pyreindexer/query_results.py +++ b/pyreindexer/query_results.py @@ -77,14 +77,12 @@ def _close_iterator(self): self.qres_iter_count = 0 self.api.query_results_delete(self.qres_wrapper_ptr) - def get_agg_results(self): """Returns aggregation results for the current query """ - self.err_code, self.err_msg, res = self.api.get_agg_results( - self.qres_wrapper_ptr) + self.err_code, self.err_msg, res = self.api.get_agg_results(self.qres_wrapper_ptr) if self.err_code: raise Exception(self.err_msg) - return res \ No newline at end of file + return res diff --git a/pyreindexer/tests/tests/test_sql.py b/pyreindexer/tests/tests/test_sql.py index 08c96ce..cf9d0bf 100644 --- a/pyreindexer/tests/tests/test_sql.py +++ b/pyreindexer/tests/tests/test_sql.py @@ -58,7 +58,7 @@ def test_sql_delete(self, namespace, index, item): def test_sql_select_with_syntax_error(self, namespace, index, item): # Given("Create namespace with item") # When ("Execute SQL query SELECT with incorrect syntax") - query = f'SELECT *' + query = 'SELECT *' # Then ("Check that selected item is in result") assert_that(calling(sql_query).with_args(namespace, query), raises(Exception, matching=has_string(string_contains_in_order( @@ -79,4 +79,4 @@ def test_sql_select_with_aggregations(self, namespace, index, items): # Then ("Check that returned agg results are correct") for agg in select_result: assert_that(agg['value'], equal_to(expected_values[agg['type']]), - f"Incorrect aggregation result for {agg['type']}") \ No newline at end of file + f"Incorrect aggregation result for {agg['type']}") diff --git a/setup.py b/setup.py index f2cae2b..adff914 100644 --- a/setup.py +++ b/setup.py @@ -52,7 +52,7 @@ def build_cmake(self, ext): setup(name=PACKAGE_NAME, - version='0.2.37', + version='0.2.38', description='A connector that allows to interact with Reindexer', author='Igor Tulmentyev', author_email='igtulm@gmail.com',