Skip to content

Commit 6324191

Browse files
committed
tuple field assignment wrapper
1 parent cd43e38 commit 6324191

4 files changed

Lines changed: 48 additions & 33 deletions

File tree

src/duckdb_py/include/duckdb_python/pybind11/pybind_wrapper.hpp

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <nanobind/stl/optional.h>
1919
#include <nanobind/stl/detail/nb_list.h>
2020
#include <nanobind/operators.h>
21+
#include <cassert>
2122

2223
// nanobind has no PYBIND11_NAMESPACE; the custom type_caster specializations below (and in the
2324
// conversion headers) live in `namespace nanobind`. Point the legacy macro at it so those headers
@@ -108,13 +109,8 @@ bool try_cast(const handle &object, T &result) {
108109
return true;
109110
}
110111

111-
// pybind11's std::string caster accepted str (as-is) and bytes (decoded UTF-8) and stringified scalars; nanobind's
112-
// nb::cast<std::string> is stricter and surfaces a raw std::bad_cast for non-str input. This helper restores the
113-
// lenient behavior for the identifier / parameter-key / separator sites that relied on it:
114-
// str -> the string as-is
115-
// bytes -> UTF-8 decoded (so read_csv(sep=b"|") and byte-string identifiers keep working)
116-
// else -> str(obj) (so e.g. an int parameter-dict key stringifies to "1", matching pybind11)
117-
// It never throws std::bad_cast.
112+
// Lenient string conversion matching pybind11 (nanobind's cast<std::string> rejects bytes/scalars with std::bad_cast):
113+
// str stays as is, bytes are UTF-8 decoded, anything else goes through str(). For identifier/param-key/separator sites.
118114
inline std::string cast_to_string(handle obj) {
119115
// Use check_ directly: an unqualified isinstance<> here is ambiguous between this namespace's override and
120116
// nanobind's (pulled in by the using-directive above).
@@ -127,6 +123,36 @@ inline std::string cast_to_string(handle obj) {
127123
return cast<std::string>(str(obj));
128124
}
129125

126+
// Fills a tuple of known size via PyTuple_SET_ITEM (nanobind's py::tuple is immutable). Cheaper than building a
127+
// py::list then copying it to a tuple. Fill every slot with append()/set_item(), then take().
128+
class tuple_builder {
129+
public:
130+
explicit tuple_builder(size_t size)
131+
: tuple_(steal<tuple>(PyTuple_New(static_cast<Py_ssize_t>(size)))), size_(size) {
132+
}
133+
// Append to the next slot (steals item's ref).
134+
void append(object item) {
135+
assert(index_ < size_);
136+
PyTuple_SET_ITEM(tuple_.ptr(), static_cast<Py_ssize_t>(index_++), item.release().ptr());
137+
}
138+
// Set slot `index` (steals item's ref).
139+
void set_item(size_t index, object item) {
140+
assert(index < size_);
141+
PyTuple_SET_ITEM(tuple_.ptr(), static_cast<Py_ssize_t>(index), item.release().ptr());
142+
}
143+
size_t size() const {
144+
return size_;
145+
}
146+
tuple take() {
147+
return std::move(tuple_);
148+
}
149+
150+
private:
151+
tuple tuple_;
152+
size_t size_;
153+
size_t index_ = 0;
154+
};
155+
130156
// pybind11 compatibility shim: pybind11's py::register_exception<T>(scope, name[, base]) maps to nanobind's
131157
// nb::exception<T>(scope, name[, base]) (which both creates the Python exception type and registers a C++->Python
132158
// translator). Returns the exception object so callers can set .attr()/.doc().

src/duckdb_py/native/python_objects.cpp

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -393,17 +393,14 @@ py::object PythonObject::FromStruct(const Value &val, const LogicalType &type,
393393

394394
auto &child_types = StructType::GetChildTypes(type);
395395
if (StructType::IsUnnamed(type)) {
396-
// nanobind tuples are immutable; build the pre-sized tuple via the raw CPython API (SET_ITEM steals
397-
// the reference) to keep the hot FromValue path allocation-light.
398-
auto py_tuple = py::steal<py::tuple>(PyTuple_New((Py_ssize_t)struct_values.size()));
396+
py::tuple_builder py_tuple(struct_values.size());
399397
for (idx_t i = 0; i < struct_values.size(); i++) {
400398
auto &child_entry = child_types[i];
401399
D_ASSERT(child_entry.first.empty());
402400
auto &child_type = child_entry.second;
403-
PyTuple_SET_ITEM(py_tuple.ptr(), (Py_ssize_t)i,
404-
FromValue(struct_values[i], child_type, client_properties).release().ptr());
401+
py_tuple.append(FromValue(struct_values[i], child_type, client_properties));
405402
}
406-
return std::move(py_tuple);
403+
return py_tuple.take();
407404
} else {
408405
py::dict py_struct;
409406
for (idx_t i = 0; i < struct_values.size(); i++) {
@@ -672,13 +669,11 @@ py::object PythonObject::FromValue(const Value &val, const LogicalType &type,
672669
// because the return type of ArrayType::GetSize is idx_t,
673670
// which is typedef'd to uint64_t and ssize_t is 4 bytes with Emscripten
674671
// and pybind11 requires that the input be castable to ssize_t
675-
auto arr = py::steal<py::tuple>(PyTuple_New(static_cast<Py_ssize_t>(array_size)));
676-
672+
py::tuple_builder arr(array_size);
677673
for (idx_t elem_idx = 0; elem_idx < array_size; elem_idx++) {
678-
PyTuple_SET_ITEM(arr.ptr(), (Py_ssize_t)elem_idx,
679-
FromValue(array_values[elem_idx], child_type, client_properties).release().ptr());
674+
arr.append(FromValue(array_values[elem_idx], child_type, client_properties));
680675
}
681-
return std::move(arr);
676+
return arr.take();
682677
}
683678
case LogicalTypeId::MAP: {
684679
auto &list_values = ListValue::GetChildren(val);

src/duckdb_py/pyresult.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -136,21 +136,18 @@ Optional<py::tuple> DuckDBPyResult::Fetchone() {
136136
if (!current_chunk || current_chunk->size() == 0) {
137137
return py::none();
138138
}
139-
// nanobind tuples are immutable (no pre-sized ctor / indexed assignment); build a list sequentially
140-
// and convert to a tuple at the end. Only py-object refcounts move here, no heavy C++ data is copied.
141-
py::list res;
142-
139+
py::tuple_builder row(result->types.size());
143140
for (idx_t col_idx = 0; col_idx < result->types.size(); col_idx++) {
144141
auto &mask = FlatVector::Validity(current_chunk->data[col_idx]);
145142
if (!mask.RowIsValid(chunk_offset)) {
146-
res.append(py::none());
147-
continue;
143+
row.append(py::none());
144+
} else {
145+
auto val = current_chunk->data[col_idx].GetValue(chunk_offset);
146+
row.append(PythonObject::FromValue(val, result->types[col_idx], result->client_properties));
148147
}
149-
auto val = current_chunk->data[col_idx].GetValue(chunk_offset);
150-
res.append(PythonObject::FromValue(val, result->types[col_idx], result->client_properties));
151148
}
152149
chunk_offset++;
153-
return py::tuple(res);
150+
return row.take();
154151
}
155152

156153
py::list DuckDBPyResult::Fetchmany(idx_t size) {

src/duckdb_py/python_udf.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,9 +331,7 @@ static scalar_function_t CreateNativeFunction(PyObject *function, PythonExceptio
331331

332332
py::object ret;
333333
if (input.ColumnCount() > 0) {
334-
// nanobind tuples are immutable; build a pre-sized tuple with the raw CPython API (SET_ITEM steals a
335-
// reference) so the per-row UDF path keeps pybind11's allocation profile (no list-then-convert copy).
336-
auto bundled_parameters = py::steal<py::tuple>(PyTuple_New((Py_ssize_t)input.ColumnCount()));
334+
py::tuple_builder parameter_builder(input.ColumnCount());
337335
bool contains_null = false;
338336
for (idx_t i = 0; i < input.ColumnCount(); i++) {
339337
// Fill the tuple with the arguments for this row
@@ -343,16 +341,15 @@ static scalar_function_t CreateNativeFunction(PyObject *function, PythonExceptio
343341
contains_null = true;
344342
break;
345343
}
346-
PyTuple_SET_ITEM(
347-
bundled_parameters.ptr(), (Py_ssize_t)i,
348-
PythonObject::FromValue(value, column.GetType(), client_properties).release().ptr());
344+
parameter_builder.append(PythonObject::FromValue(value, column.GetType(), client_properties));
349345
}
350346
if (contains_null) {
351347
// Immediately insert None, no need to call the function
352348
FlatVector::SetNull(result, row, true);
353349
continue;
354350
}
355351
// Call the function
352+
auto bundled_parameters = parameter_builder.take();
356353
ret = py::steal<py::object>(PyObject_CallObject(function, bundled_parameters.ptr()));
357354
} else {
358355
ret = py::steal<py::object>(PyObject_CallObject(function, nullptr));

0 commit comments

Comments
 (0)