Skip to content

Commit 03550bd

Browse files
committed
Resolve PyObject* conversion warnings
The `Read as arrow` commit introduced a `UTF_DYNAMIC32` type whose value types can't safely be converted to pointers.
1 parent 8b6b5e6 commit 03550bd

File tree

3 files changed

+35
-25
lines changed

3 files changed

+35
-25
lines changed

cpp/arcticdb/column_store/statistics.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ FieldStatsImpl generate_column_statistics(ColumnData column_data) {
187187
const size_t count = block->row_count();
188188
if constexpr (is_numeric_type(TagType::DataTypeTag::data_type)) {
189189
return generate_numeric_statistics<RawType>(std::span{ptr, count});
190-
} else if constexpr (is_dynamic_string_type(TagType::DataTypeTag::data_type) && !is_output_only_type(TagType::DataTypeTag::data_type)) {
190+
} else if constexpr (is_dynamic_string_type(TagType::DataTypeTag::data_type) && !is_arrow_output_only_type(TagType::DataTypeTag::data_type)) {
191191
return generate_string_statistics(std::span{ptr, count});
192192
} else {
193193
util::raise_rte("Cannot generate statistics for data type");
@@ -200,7 +200,7 @@ FieldStatsImpl generate_column_statistics(ColumnData column_data) {
200200
if constexpr (is_numeric_type(TagType::DataTypeTag::data_type)) {
201201
auto local_stats = generate_numeric_statistics<RawType>(std::span{ptr, count});
202202
stats.compose<RawType>(local_stats);
203-
} else if constexpr (is_dynamic_string_type(TagType::DataTypeTag::data_type) && !is_output_only_type(TagType::DataTypeTag::data_type)) {
203+
} else if constexpr (is_dynamic_string_type(TagType::DataTypeTag::data_type) && !is_arrow_output_only_type(TagType::DataTypeTag::data_type)) {
204204
auto local_stats = generate_string_statistics(std::span{ptr, count});
205205
stats.compose<RawType>(local_stats);
206206
} else {

cpp/arcticdb/entity/types.hpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ constexpr bool is_dynamic_string_type(DataType v) {
297297
return is_dynamic_string_type(slice_value_type(v));
298298
}
299299

300-
constexpr bool is_output_only_type(DataType d) {
300+
constexpr bool is_arrow_output_only_type(DataType d) {
301301
return d == DataType::UTF_DYNAMIC32;
302302
}
303303

@@ -500,6 +500,10 @@ constexpr bool is_object_type(TypeDescriptor td) {
500500
|| is_array_type(td);
501501
}
502502

503+
constexpr bool is_arrow_output_only_type(TypeDescriptor td) {
504+
return is_arrow_output_only_type(td.data_type());
505+
}
506+
503507
inline void set_data_type(DataType data_type, TypeDescriptor &type_desc) {
504508
type_desc.data_type_ = data_type;
505509
}

cpp/arcticdb/pipeline/pandas_output_frame.cpp

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -33,32 +33,38 @@ PandasOutputFrame::~PandasOutputFrame() {
3333
if(frame_.is_null())
3434
return;
3535

36-
for (auto &column : frame_.columns()) {
37-
auto dt = column->type().data_type();
38-
if (is_dynamic_string_type(dt) && column->is_inflated()) {
39-
auto column_data = column->data();
40-
column_data.type().visit_tag([&](auto type_desc_tag) {
41-
using TDT = decltype(type_desc_tag);
42-
constexpr auto td = TypeDescriptor(type_desc_tag);
43-
if constexpr (is_object_type(TypeDescriptor(type_desc_tag))) {
44-
if constexpr(is_array_type(td)) {
45-
auto it = column_data.buffer().iterator(sizeof(PyObject*));
46-
while(!it.finished()) {
47-
util::check(reinterpret_cast<PyObject*>(it.value()) != nullptr, "Can't delete null item");
48-
Py_DECREF(reinterpret_cast<PyObject*>(it.value()));
49-
it.next();
50-
}
51-
} else {
52-
while (auto block = column_data.next<TDT>()) {
53-
for (auto item : *block) {
54-
util::check(reinterpret_cast<PyObject *>(item) != nullptr, "Can't delete null item");
55-
Py_DECREF(reinterpret_cast<PyObject *>(item));
36+
try {
37+
for (auto &column : frame_.columns()) {
38+
auto dt = column->type().data_type();
39+
if (is_dynamic_string_type(dt) && column->is_inflated()) {
40+
auto column_data = column->data();
41+
column_data.type().visit_tag([&](auto type_desc_tag) {
42+
using TDT = decltype(type_desc_tag);
43+
constexpr auto td = TypeDescriptor(type_desc_tag);
44+
if constexpr (is_object_type(td)) {
45+
if constexpr(is_array_type(td)) {
46+
auto it = column_data.buffer().iterator(sizeof(PyObject*));
47+
while(!it.finished()) {
48+
util::check(reinterpret_cast<PyObject*>(it.value()) != nullptr, "Can't delete null item");
49+
Py_DECREF(reinterpret_cast<PyObject*>(it.value()));
50+
it.next();
51+
}
52+
} else if constexpr (!is_arrow_output_only_type(td)) {
53+
while (auto block = column_data.next<TDT>()) {
54+
for (auto item : *block) {
55+
util::check(reinterpret_cast<PyObject *>(item) != nullptr, "Can't delete null item");
56+
Py_DECREF(reinterpret_cast<PyObject *>(item));
57+
}
5658
}
59+
} else {
60+
internal::raise<ErrorCode::E_ASSERTION_FAILURE>("Unexpected arrow output format seen in PandasOutputFrame");
5761
}
5862
}
59-
}
60-
});
63+
});
64+
}
6165
}
66+
} catch (const std::exception &e) {
67+
log::version().warn("Exception in PandasOutputFrame destructor: {}", e.what());
6268
}
6369
}
6470

0 commit comments

Comments
 (0)