Skip to content

Commit eb9f87a

Browse files
committed
nanobind: custom shared_ptr<DuckDBPyType> caster (keep convert flag for implicit conversions); guard numpy ctypes eager-compute
1 parent 06bb706 commit eb9f87a

2 files changed

Lines changed: 68 additions & 1 deletion

File tree

src/duckdb_py/include/duckdb_python/numpy/numpy_array.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,10 @@ class NumpyArray {
9898
//! cached and an array is held. `ctypes.data` is dtype-agnostic (works for the `object` dtype
9999
//! too). Only ever called with the GIL held (construction / Resize).
100100
void EnsurePointer() {
101-
if (!cached_data_ && array.ptr() != nullptr) {
101+
// Only numpy ndarrays expose `ctypes`; some NumpyArray wrappers hold other objects (e.g. a pandas Index)
102+
// whose buffer pointer is never read. Guard the eager compute so constructing such a wrapper doesn't raise
103+
// (the original lazy code only touched `ctypes` if Data()/MutableData() was actually called).
104+
if (!cached_data_ && array.ptr() != nullptr && py::hasattr(array, "ctypes")) {
102105
cached_data_ = reinterpret_cast<void *>(py::cast<uintptr_t>(array.attr("ctypes").attr("data")));
103106
}
104107
}

src/duckdb_py/include/duckdb_python/pytype.hpp

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,67 @@ class DuckDBPyType : public std::enable_shared_from_this<DuckDBPyType> {
5050
};
5151

5252
} // namespace duckdb
53+
54+
namespace nanobind {
55+
namespace detail {
56+
57+
// Custom type caster for std::shared_ptr<duckdb::DuckDBPyType>.
58+
//
59+
// nanobind's default std::shared_ptr<T> caster strips cast_flags::convert before delegating to the inner caster,
60+
// which disables implicit conversions for shared_ptr-typed arguments. DuckDBPyType, however, is routinely passed
61+
// as a string ("VARCHAR"), a Python type object (int), a typing generic, or a dict, relying on its registered
62+
// implicit conversions (as it did under pybind11). Those conversions construct brand-new, fully-owned
63+
// DuckDBPyType objects, so they carry no dangling risk -- we therefore mirror nanobind's shared_ptr caster but
64+
// KEEP the convert flag. (This specialization is visible in every TU that converts the type, since such TUs use
65+
// DuckDBPyType and thus include this header.)
66+
template <>
67+
struct type_caster<std::shared_ptr<duckdb::DuckDBPyType>> {
68+
using T = duckdb::DuckDBPyType;
69+
using Caster = make_caster<T>;
70+
NB_TYPE_CASTER(std::shared_ptr<T>, Caster::Name)
71+
72+
bool from_python(handle src, uint8_t flags, cleanup_list *cleanup) noexcept {
73+
// NOTE: deliberately do NOT clear cast_flags::convert (see header comment).
74+
Caster caster;
75+
if (!caster.from_python(src, flags, cleanup)) {
76+
return false;
77+
}
78+
T *ptr = caster.operator T *();
79+
if (ptr) {
80+
ft_object_guard guard(src);
81+
if (auto sp = ptr->weak_from_this().lock()) {
82+
value = std::static_pointer_cast<T>(std::move(sp));
83+
return true;
84+
}
85+
value = shared_from_python(ptr, src);
86+
return true;
87+
}
88+
value = shared_from_python(ptr, src);
89+
return true;
90+
}
91+
92+
static handle from_cpp(const std::shared_ptr<T> &value, rv_policy, cleanup_list *cleanup) noexcept {
93+
bool is_new = false;
94+
handle result;
95+
T *ptr = value.get();
96+
const std::type_info *type = &typeid(T);
97+
constexpr bool has_type_hook = !std::is_base_of_v<std::false_type, type_hook<T>>;
98+
if constexpr (has_type_hook) {
99+
type = type_hook<T>::get(ptr);
100+
}
101+
if constexpr (!std::is_polymorphic_v<T>) {
102+
result = nb_type_put(type, ptr, rv_policy::reference, cleanup, &is_new);
103+
} else {
104+
const std::type_info *type_p = (!has_type_hook && ptr) ? &typeid(*ptr) : nullptr;
105+
result = nb_type_put_p(type, type_p, ptr, rv_policy::reference, cleanup, &is_new);
106+
}
107+
if (is_new) {
108+
auto pp = std::static_pointer_cast<void>(value);
109+
shared_from_cpp(std::move(pp), result.ptr());
110+
}
111+
return result;
112+
}
113+
};
114+
115+
} // namespace detail
116+
} // namespace nanobind

0 commit comments

Comments
 (0)