ptupitsyn commented on code in PR #4401: URL: https://github.com/apache/ignite-3/pull/4401#discussion_r1762449851
########## modules/platforms/python/cpp_module/type_conversion.h: ########## @@ -171,8 +332,39 @@ static void submit_pyobject(ignite::binary_tuple_builder &builder, PyObject *obj return; } + if (PyBytes_Check(obj)) { + auto *data = reinterpret_cast<std::byte*>(PyBytes_AsString(obj)); + auto len = PyBytes_Size(obj); + ignite::bytes_view view(data, len); + + if (claim) { + ignite::protocol::claim_type_and_scale(builder, ignite::ignite_type::BYTE_ARRAY); + builder.claim_varlen(view); + } else { + ignite::protocol::append_type_and_scale(builder, ignite::ignite_type::BYTE_ARRAY); + builder.append_varlen(view); + } + return; + } + + if (PyBool_Check(obj)) { + bool val = (obj == Py_True); + if (claim) { + ignite::protocol::claim_type_and_scale(builder, ignite::ignite_type::BOOLEAN); + builder.claim_bool(val); + } else { + ignite::protocol::append_type_and_scale(builder, ignite::ignite_type::BOOLEAN); + builder.append_bool(val); + } + return; + } + if (PyFloat_Check(obj)) { double val = PyFloat_AsDouble(obj); + if (PyErr_Occurred()) { + throw ignite::ignite_error("Can not convert FLOAT to a double"); Review Comment: How is that possible? ########## modules/platforms/python/cpp_module/type_conversion.h: ########## @@ -185,6 +377,10 @@ static void submit_pyobject(ignite::binary_tuple_builder &builder, PyObject *obj if (PyLong_Check(obj)) { auto val = PyLong_AsLongLong(obj); + if (PyErr_Occurred()) { + throw ignite::ignite_error("Can not convert INT to a long long"); Review Comment: Same as above - is that possible in practice? ########## modules/platforms/python/cpp_module/type_conversion.h: ########## @@ -122,40 +161,162 @@ static void submit_pyobject(ignite::binary_tuple_builder &builder, PyObject *obj return; } - if (PyBool_Check(obj)) { - bool val = (obj == Py_True); + if (PyObject_IsInstance(obj, py_get_module_timestamp_class())) { + auto double_value = PyFloat_AsDouble(obj); + if (PyErr_Occurred()) { + throw ignite::ignite_error("Can not get ticks count from a TIMESTAMP value"); Review Comment: Is that possible in practice? Should we add the original error message from `PyErr_Occurred`? ########## modules/platforms/python/cpp_module/type_conversion.h: ########## @@ -122,40 +161,162 @@ static void submit_pyobject(ignite::binary_tuple_builder &builder, PyObject *obj return; } - if (PyBool_Check(obj)) { - bool val = (obj == Py_True); + if (PyObject_IsInstance(obj, py_get_module_timestamp_class())) { + auto double_value = PyFloat_AsDouble(obj); + if (PyErr_Occurred()) { + throw ignite::ignite_error("Can not get ticks count from a TIMESTAMP value"); + } + + auto secs = std::int64_t(std::trunc(double_value)); + auto nanos = std::lround((double_value - secs) * 1'000'000'000); + ignite::ignite_timestamp value(secs, nanos); if (claim) { - ignite::protocol::claim_type_and_scale(builder, ignite::ignite_type::BOOLEAN); - builder.claim_bool(val); + ignite::protocol::claim_type_and_scale(builder, ignite::ignite_type::TIMESTAMP); + builder.claim_timestamp(value); } else { - ignite::protocol::append_type_and_scale(builder, ignite::ignite_type::BOOLEAN); - builder.append_bool(val); + ignite::protocol::append_type_and_scale(builder, ignite::ignite_type::TIMESTAMP); + builder.append_timestamp(value); } return; } - if (PyBytes_Check(obj)) { - auto *data = reinterpret_cast<std::byte*>(PyBytes_AsString(obj)); - auto len = PyBytes_Size(obj); - ignite::bytes_view view(data, len); + if (PyObject_IsInstance(obj, py_get_module_number_class())) { + auto obj_str = PyObject_Str(obj); + if (!obj_str) { + throw ignite::ignite_error("Can not convert NUMBER to str"); Review Comment: Here and other places - if a conversion fails, we should provide more details: - The value that we could not convert - The original error from converter, if applicable ########## modules/platforms/python/cpp_module/utils.cpp: ########## @@ -0,0 +1,395 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "utils.h" + +#include <ignite/odbc/diagnostic/diagnosable.h> +#include <ignite/common/detail/defer.h> + +#include <Python.h> + +#define LAZY_INIT_MODULE_CLASS(class_name) \ + static PyObject* instance{nullptr}; \ + if (!instance) \ + instance = py_get_module_class(class_name); \ + return instance + +bool check_errors(ignite::diagnosable& diag) { + auto &records = diag.get_diagnostic_records(); + if (records.is_successful()) + return true; + + std::string err_msg; + switch (records.get_return_code()) { + case SQL_INVALID_HANDLE: + err_msg = "Invalid object handle"; + break; + + case SQL_NO_DATA: + err_msg = "No data available"; + break; + + case SQL_ERROR: + auto record = records.get_status_record(1); + err_msg = record.get_message_text(); + break; + } + + // TODO: IGNITE-22226 Set a proper error here, not a standard one. + PyErr_SetString(PyExc_RuntimeError, err_msg.c_str()); + + return false; +} + +const char* py_object_get_typename(PyObject* obj) { + if (!obj || !obj->ob_type || !obj->ob_type->tp_name) { + return "Unknown"; + } + + return obj->ob_type->tp_name; +} + +PyObject* py_get_module() { + static PyObject* instance{nullptr}; + // No need for sync here - Python is single-threaded + if (!instance) { + instance = PyImport_ImportModule(MODULE_NAME); + } + return instance; +} + +PyObject* py_get_class(const char* module_name, const char* class_name) { + auto module_obj = PyImport_ImportModule(module_name); + if (!module_obj) + return nullptr; + + auto class_obj = PyObject_GetAttrString(module_obj, class_name); + Py_DECREF(module_obj); + + return class_obj; +} + +PyObject* py_get_module_class(const char* class_name) { + auto pyignite3_mod = py_get_module(); + if (!pyignite3_mod) + return nullptr; + + return PyObject_GetAttrString(pyignite3_mod, class_name); +} + +PyObject* py_call_method_no_arg(PyObject* obj, const char* method_name) { + PyObject* py_method_name = PyUnicode_FromString(method_name); + if (!py_method_name) + return nullptr; + auto py_method_name_guard = ignite::detail::defer([&]{ Py_DECREF(py_method_name); }); + + return PyObject_CallMethodObjArgs(obj, py_method_name, nullptr); +} + +std::int64_t py_get_attr_int(PyObject* obj, const char* attr_name) { + auto attr_obj = PyObject_GetAttrString(obj, attr_name); + if (!attr_obj) { + throw ignite::ignite_error("Can not get an Object's attribute: " + std::string(attr_name)); + } + auto attr_obj_guard = ignite::detail::defer([&] { Py_DECREF(attr_obj); }); + if (PyErr_Occurred()) { + throw ignite::ignite_error("An Object's attribute is not an integer : " + std::string(attr_name)); + } + return PyLong_AsLongLong(attr_obj); +} + +PyObject* py_get_module_uuid_class() { + LAZY_INIT_MODULE_CLASS("UUID"); +} + +PyObject* py_get_module_date_class() { + LAZY_INIT_MODULE_CLASS("DATE"); +} + +PyObject* py_get_module_time_class() { + LAZY_INIT_MODULE_CLASS("TIME"); +} + +PyObject* py_get_module_datetime_class() { + LAZY_INIT_MODULE_CLASS("DATETIME"); +} + +PyObject* py_get_module_timestamp_class() { + LAZY_INIT_MODULE_CLASS("TIMESTAMP"); +} + +PyObject* py_get_module_number_class() { + LAZY_INIT_MODULE_CLASS("NUMBER"); +} + +PyObject* py_get_module_duration_class() { + LAZY_INIT_MODULE_CLASS("DURATION"); +} + +PyObject* py_create_uuid(ignite::bytes_view bytes) { + auto uuid_class = py_get_module_uuid_class(); + if (!uuid_class) + return nullptr; + + auto args = PyTuple_New(0); + if (!args) + return nullptr; + auto args_guard = ignite::detail::defer([&]{ Py_DECREF(args); }); + + auto kwargs = PyDict_New(); + if (!kwargs) + return nullptr; + auto kwargs_guard = ignite::detail::defer([&]{ Py_DECREF(kwargs); }); + + PyObject* py_bytes = PyBytes_FromStringAndSize(reinterpret_cast<const char*>(bytes.data()), bytes.size()); + if (!py_bytes) + return nullptr; + + if (PyDict_SetItemString(kwargs, "bytes", py_bytes) < 0) { + Py_DECREF(py_bytes); + return nullptr; + } + + return PyObject_Call(uuid_class, args, kwargs); +} + +PyObject* py_create_date(const ignite::ignite_date &value) { + auto date_class = py_get_module_date_class(); + if (!date_class) + return nullptr; + + PyObject* year = PyLong_FromLong(value.get_year()); + if (!year) + return nullptr; + auto year_guard = ignite::detail::defer([&]{ Py_DECREF(year); }); + + PyObject* month = PyLong_FromLong(value.get_month()); + if (!month) + return nullptr; + auto month_guard = ignite::detail::defer([&]{ Py_DECREF(month); }); + + PyObject* day = PyLong_FromLong(value.get_day_of_month()); + if (!day) + return nullptr; + auto day_guard = ignite::detail::defer([&]{ Py_DECREF(day); }); + + auto args = PyTuple_Pack(3, year, month, day); + if (!args) + return nullptr; + auto args_guard = ignite::detail::defer([&]{ Py_DECREF(args); }); + + year_guard.release(); + month_guard.release(); + day_guard.release(); + + auto kwargs = PyDict_New(); + if (!kwargs) + return nullptr; + auto kwargs_guard = ignite::detail::defer([&]{ Py_DECREF(kwargs); }); + + return PyObject_Call(date_class, args, kwargs); +} + +PyObject* py_create_time(const ignite::ignite_time &value) { + auto time_class = py_get_module_time_class(); + if (!time_class) + return nullptr; + + PyObject* hour = PyLong_FromLong(value.get_hour()); + if (!hour) + return nullptr; + auto hour_guard = ignite::detail::defer([&]{ Py_DECREF(hour); }); + + PyObject* minute = PyLong_FromLong(value.get_minute()); + if (!minute) + return nullptr; + auto minute_guard = ignite::detail::defer([&]{ Py_DECREF(minute); }); + + PyObject* second = PyLong_FromLong(value.get_second()); + if (!second) + return nullptr; + auto second_guard = ignite::detail::defer([&]{ Py_DECREF(second); }); + + PyObject* u_second = PyLong_FromLong(value.get_nano() / 1000); + if (!u_second) + return nullptr; + auto u_second_guard = ignite::detail::defer([&]{ Py_DECREF(u_second); }); + + auto args = PyTuple_Pack(4, hour, minute, second, u_second); + if (!args) + return nullptr; + auto args_guard = ignite::detail::defer([&]{ Py_DECREF(args); }); + + hour_guard.release(); + minute_guard.release(); + second_guard.release(); + u_second_guard.release(); + + auto kwargs = PyDict_New(); + if (!kwargs) + return nullptr; + auto kwargs_guard = ignite::detail::defer([&]{ Py_DECREF(kwargs); }); + + return PyObject_Call(time_class, args, kwargs); +} + +PyObject* py_create_datetime(const ignite::ignite_date_time &value) { + auto datetime_class = py_get_module_datetime_class(); + if (!datetime_class) + return nullptr; + auto class_guard = ignite::detail::defer([&]{ Py_DECREF(datetime_class); }); + + PyObject* year = PyLong_FromLong(value.get_year()); + if (!year) + return nullptr; + auto year_guard = ignite::detail::defer([&]{ Py_DECREF(year); }); + + PyObject* month = PyLong_FromLong(value.get_month()); + if (!month) + return nullptr; + auto month_guard = ignite::detail::defer([&]{ Py_DECREF(month); }); + + PyObject* day = PyLong_FromLong(value.get_day_of_month()); + if (!day) + return nullptr; + auto day_guard = ignite::detail::defer([&]{ Py_DECREF(day); }); + + PyObject* hour = PyLong_FromLong(value.get_hour()); + if (!hour) + return nullptr; + auto hour_guard = ignite::detail::defer([&]{ Py_DECREF(hour); }); + + PyObject* minute = PyLong_FromLong(value.get_minute()); + if (!minute) + return nullptr; + auto minute_guard = ignite::detail::defer([&]{ Py_DECREF(minute); }); + + PyObject* second = PyLong_FromLong(value.get_second()); + if (!second) + return nullptr; + auto second_guard = ignite::detail::defer([&]{ Py_DECREF(second); }); + + PyObject* u_second = PyLong_FromLong(value.get_nano() / 1000); + if (!u_second) + return nullptr; + auto u_second_guard = ignite::detail::defer([&]{ Py_DECREF(u_second); }); + + auto args = PyTuple_Pack(7, year, month, day, hour, minute, second, u_second); + if (!args) + return nullptr; + auto args_guard = ignite::detail::defer([&]{ Py_DECREF(args); }); + + year_guard.release(); + month_guard.release(); + day_guard.release(); + hour_guard.release(); + minute_guard.release(); + second_guard.release(); + u_second_guard.release(); + + auto kwargs = PyDict_New(); + if (!kwargs) + return nullptr; + auto kwargs_guard = ignite::detail::defer([&]{ Py_DECREF(kwargs); }); + + return PyObject_Call(datetime_class, args, kwargs); +} + +PyObject* py_create_datetime(const ignite::ignite_timestamp &value) { + // TODO: Cache classes and functions for re-use Review Comment: Ticket for TODO? ########## modules/platforms/python/cpp_module/type_conversion.h: ########## @@ -85,23 +88,59 @@ static PyObject* primitive_to_pyobject(ignite::primitive value) { } case ignite_type::BYTE_ARRAY: { - auto &blob_val = value.get<std::vector<std::byte>>(); + auto &blob_val = value.get<std::vector<std::byte>>(); return PyBytes_FromStringAndSize((const char*)blob_val.data(), blob_val.size()); } - case ignite_type::UUID: - case ignite_type::DATE: - case ignite_type::TIMESTAMP: - case ignite_type::TIME: - case ignite_type::DATETIME: - case ignite_type::BITMASK: - case ignite_type::DECIMAL: - case ignite_type::PERIOD: - case ignite_type::DURATION: - case ignite_type::NUMBER: + case ignite_type::UUID: { + auto &uuid_val = value.get<ignite::uuid>(); + std::byte buf[16]; + ignite::detail::bytes::store<ignite::detail::endian::BIG>(buf, uuid_val.get_most_significant_bits()); + ignite::detail::bytes::store<ignite::detail::endian::BIG>(buf + 8, uuid_val.get_least_significant_bits()); + return py_create_uuid({buf, sizeof(buf)}); + } + + case ignite_type::DATE: { + auto &date_val = value.get<ignite::ignite_date>(); + return py_create_date(date_val); + } + + case ignite_type::TIME: { + auto &time_val = value.get<ignite::ignite_time>(); + return py_create_time(time_val); + } + + case ignite_type::DATETIME: { + auto &datetime_val = value.get<ignite::ignite_date_time>(); + return py_create_datetime(datetime_val); + } + + case ignite_type::TIMESTAMP: { + auto ×tamp_val = value.get<ignite::ignite_timestamp>(); + auto double_val = timestamp_val.get_epoch_second() + timestamp_val.get_nano() * 1.0E-9; + return PyFloat_FromDouble(double_val); + } + + case ignite_type::DECIMAL: { + auto &decimal_val = value.get<ignite::big_decimal>(); + std::stringstream converter; + converter << decimal_val; + auto str = converter.str(); + return py_create_number(str); + } + + case ignite_type::DURATION: { + auto &duration_val = value.get<ignite::ignite_duration>(); + return py_create_timedelta(duration_val); + } + + case ignite_type::PERIOD:{ + PyErr_SetString(PyExc_RuntimeError, "PERIOD data type is not supported"); Review Comment: Do we need a ticket for this? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org