Skip to content

Commit 18692be

Browse files
evertlammertsevertlammerts
committed
long tail fixes
1 parent 0f57ddf commit 18692be

14 files changed

Lines changed: 121 additions & 76 deletions

File tree

‎src/duckdb_py/arrow/pyarrow_filter_pushdown.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,14 +143,14 @@ py::object MakePyArrowScalar(const Value &constant, const string &timezone_confi
143143
if (arrow_type && arrow_type->GetTypeInfo<ArrowStringInfo>().GetSizeType() == ArrowVariableSizeType::VIEW) {
144144
py::handle binary_view_type = import_cache.pyarrow.binary_view();
145145
{
146-
auto blob = constant.GetValueUnsafe<string>();
147-
return dataset_scalar(scalar(py::bytes(blob.data(), blob.size()), binary_view_type()));
148-
}
146+
auto blob = constant.GetValueUnsafe<string>();
147+
return dataset_scalar(scalar(py::bytes(blob.data(), blob.size()), binary_view_type()));
148+
}
149149
}
150150
{
151-
auto blob = constant.GetValueUnsafe<string>();
152-
return dataset_scalar(py::bytes(blob.data(), blob.size()));
153-
}
151+
auto blob = constant.GetValueUnsafe<string>();
152+
return dataset_scalar(py::bytes(blob.data(), blob.size()));
153+
}
154154
}
155155
case LogicalTypeId::DECIMAL: {
156156
if (!arrow_type) {

‎src/duckdb_py/include/duckdb_python/arrow/arrow_array_stream.hpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,3 @@ class PythonTableArrowArrayStreamFactory {
109109
ArrowStreamParameters &parameters, const ClientProperties &client_properties);
110110
};
111111
} // namespace duckdb
112-

‎src/duckdb_py/include/duckdb_python/pybind11/conversions/enum_string_caster.hpp‎

Lines changed: 52 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -24,62 +24,69 @@
2424
// qualified names for the conversion functions and the enum type.
2525

2626
//! str + int + enum form.
27-
#define DUCKDB_PY_ENUM_STRING_INT_CASTER(EnumType, FromStringFn, FromIntegerFn, NameLiteral) \
27+
#define DUCKDB_PY_ENUM_STRING_INT_CASTER(EnumType, FromStringFn, FromIntegerFn, NameLiteral) \
2828
namespace nanobind { \
2929
namespace detail { \
3030
template <> \
3131
struct type_caster<EnumType> { \
3232
NB_TYPE_CASTER(EnumType, const_name(NameLiteral)) \
33-
bool from_python(handle src, uint8_t, cleanup_list *) noexcept { \
34-
try { \
35-
if (nanobind::isinstance<nanobind::str>(src)) { \
36-
value = FromStringFn(nanobind::cast<std::string>(src)); \
37-
return true; \
38-
} \
39-
if (nanobind::isinstance<nanobind::int_>(src)) { \
40-
value = FromIntegerFn(nanobind::cast<int64_t>(src)); \
41-
return true; \
42-
} \
43-
/* Registered nb::enum_ instances aren't int subclasses (unlike pybind11's), so accept a member */ \
44-
/* of the registered enum by reading its integer .value. */ \
45-
nanobind::handle enum_type = nanobind::type<EnumType>(); \
46-
if (enum_type.is_valid() && PyObject_IsInstance(src.ptr(), enum_type.ptr()) == 1) { \
47-
value = FromIntegerFn(nanobind::cast<int64_t>(src.attr("value"))); \
48-
return true; \
49-
} \
50-
} catch (...) { \
51-
return false; \
52-
} \
53-
return false; \
54-
} \
55-
static handle from_cpp(EnumType src, rv_policy, cleanup_list *) noexcept { \
56-
return nanobind::int_((int64_t)src).release(); \
57-
} \
58-
}; \
59-
} /* namespace detail */ \
33+
bool from_python(handle src, uint8_t, cleanup_list *) noexcept { \
34+
try { \
35+
if (nanobind::isinstance<nanobind::str>(src)) { \
36+
value = FromStringFn(nanobind::cast<std::string>(src)); \
37+
return true; \
38+
} \
39+
if (nanobind::isinstance<nanobind::int_>(src)) { \
40+
value = FromIntegerFn(nanobind::cast<int64_t>(src)); \
41+
return true; \
42+
} \
43+
/* Registered nb::enum_ instances aren't int subclasses (unlike pybind11's), so accept a member */ \
44+
/* of the registered enum by reading its integer .value. */ \
45+
nanobind::handle enum_type = nanobind::type<EnumType>(); \
46+
if (enum_type.is_valid() && PyObject_IsInstance(src.ptr(), enum_type.ptr()) == 1) { \
47+
value = FromIntegerFn(nanobind::cast<int64_t>(src.attr("value"))); \
48+
return true; \
49+
} \
50+
} catch (...) { \
51+
return false; \
52+
} \
53+
return false; \
54+
} \
55+
static handle from_cpp(EnumType src, rv_policy, cleanup_list *) noexcept { \
56+
return nanobind::int_((int64_t)src).release(); \
57+
} \
58+
}; \
59+
} /* namespace detail */ \
6060
} /* namespace nanobind */
6161

6262
//! str + enum form (no integer accepted).
63-
#define DUCKDB_PY_ENUM_STRING_CASTER(EnumType, FromStringFn, NameLiteral) \
63+
#define DUCKDB_PY_ENUM_STRING_CASTER(EnumType, FromStringFn, NameLiteral) \
6464
namespace nanobind { \
6565
namespace detail { \
6666
template <> \
6767
struct type_caster<EnumType> { \
6868
NB_TYPE_CASTER(EnumType, const_name(NameLiteral)) \
69-
bool from_python(handle src, uint8_t, cleanup_list *) noexcept { \
70-
try { \
71-
if (nanobind::isinstance<nanobind::str>(src)) { \
72-
value = FromStringFn(nanobind::cast<std::string>(src)); \
73-
return true; \
74-
} \
75-
} catch (...) { \
76-
return false; \
77-
} \
78-
return false; \
79-
} \
80-
static handle from_cpp(EnumType src, rv_policy, cleanup_list *) noexcept { \
81-
return nanobind::int_((int64_t)src).release(); \
82-
} \
83-
}; \
84-
} /* namespace detail */ \
69+
bool from_python(handle src, uint8_t, cleanup_list *) noexcept { \
70+
try { \
71+
if (nanobind::isinstance<nanobind::str>(src)) { \
72+
value = FromStringFn(nanobind::cast<std::string>(src)); \
73+
return true; \
74+
} \
75+
/* Registered nb::enum_ instances aren't int subclasses; accept a member of the registered enum */ \
76+
/* by reading its integer .value (this enum has no FromInteger, so cast the int directly). */ \
77+
nanobind::handle enum_type = nanobind::type<EnumType>(); \
78+
if (enum_type.is_valid() && PyObject_IsInstance(src.ptr(), enum_type.ptr()) == 1) { \
79+
value = (EnumType)nanobind::cast<int64_t>(src.attr("value")); \
80+
return true; \
81+
} \
82+
} catch (...) { \
83+
return false; \
84+
} \
85+
return false; \
86+
} \
87+
static handle from_cpp(EnumType src, rv_policy, cleanup_list *) noexcept { \
88+
return nanobind::int_((int64_t)src).release(); \
89+
} \
90+
}; \
91+
} /* namespace detail */ \
8592
} /* namespace nanobind */

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <nanobind/stl/vector.h>
1515
#include <nanobind/stl/shared_ptr.h>
1616
#include <nanobind/stl/unique_ptr.h>
17+
#include <nanobind/stl/unordered_set.h>
1718
#include <nanobind/stl/optional.h>
1819
#include <nanobind/stl/detail/nb_list.h>
1920
#include <nanobind/operators.h>

‎src/duckdb_py/include/duckdb_python/pyconnection/pyconnection.hpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,9 @@ struct DuckDBPyConnection : public std::enable_shared_from_this<DuckDBPyConnecti
350350
static identifier_map_t<BoundParameterData> TransformPythonParamDict(ClientContext &context,
351351
const py::dict &params);
352352

353-
void RegisterFilesystem(AbstractFileSystem filesystem);
353+
// Takes py::object (not AbstractFileSystem) so the binding can accept None: nanobind's .none() does not bypass a
354+
// py::object-subclass wrapper's check_(). The body imports fsspec and validates the instance explicitly.
355+
void RegisterFilesystem(py::object filesystem);
354356
void UnregisterFilesystem(const py::str &name);
355357
py::list ListFilesystems();
356358
bool FileSystemIsRegistered(const string &name);

‎src/duckdb_py/include/duckdb_python/pyfilesystem.hpp‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,14 @@ class AbstractFileSystem : public py::object {
2727

2828
public:
2929
static bool check_(const py::handle &object) {
30-
return py::isinstance(object, py::module_::import_("fsspec").attr("AbstractFileSystem"));
30+
// Non-throwing: if fsspec isn't installed, nothing is an AbstractFileSystem. nanobind invokes check_ from
31+
// noexcept contexts (argument casters, isinstance), so a thrown import error would std::terminate rather
32+
// than propagate. register_filesystem() re-imports fsspec in a throwing context to surface ModuleNotFoundError.
33+
try {
34+
return py::isinstance(object, py::module_::import_("fsspec").attr("AbstractFileSystem"));
35+
} catch (...) {
36+
return false;
37+
}
3138
}
3239
};
3340

‎src/duckdb_py/native/python_objects.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,8 +517,7 @@ py::object PythonObject::FromValue(const Value &val, const LogicalType &type,
517517
case LogicalTypeId::VARCHAR:
518518
return py::cast(StringValue::Get(val));
519519
case LogicalTypeId::BLOB:
520-
case LogicalTypeId::GEOMETRY:
521-
{
520+
case LogicalTypeId::GEOMETRY: {
522521
auto &blob = StringValue::Get(val);
523522
return py::bytes(blob.data(), blob.size());
524523
}

‎src/duckdb_py/numpy/array_wrapper.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,8 @@ struct MapConvert {
340340
static py::dict ConvertValue(Vector &input, idx_t chunk_offset, NumpyAppendData &append_data) {
341341
auto &client_properties = append_data.client_properties;
342342
auto val = input.GetValue(chunk_offset);
343-
// FromValue returns a py::object; a MAP value materializes as a Python dict (nulls use NullValue, not this path)
343+
// FromValue returns a py::object; a MAP value materializes as a Python dict (nulls use NullValue, not this
344+
// path)
344345
return py::cast<py::dict>(PythonObject::FromValue(val, input.GetType(), client_properties));
345346
}
346347
};

‎src/duckdb_py/pyconnection.cpp‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,10 @@ std::string DuckDBPyConnection::FormattedPythonVersion() {
149149

150150
static void InitializeConnectionMethods(py::class_<DuckDBPyConnection> &m) {
151151
m.def("cursor", &DuckDBPyConnection::Cursor, "Create a duplicate of the current connection");
152+
// .none() lets None reach RegisterFilesystem's body, which imports fsspec explicitly (surfacing
153+
// ModuleNotFoundError when fsspec is absent) before validating the instance.
152154
m.def("register_filesystem", &DuckDBPyConnection::RegisterFilesystem, "Register a fsspec compliant filesystem",
153-
py::arg("filesystem"));
155+
py::arg("filesystem").none());
154156
m.def("unregister_filesystem", &DuckDBPyConnection::UnregisterFilesystem, "Unregister a filesystem",
155157
py::arg("name"));
156158
m.def("list_filesystems", &DuckDBPyConnection::ListFilesystems,
@@ -281,8 +283,16 @@ static void InitializeConnectionMethods(py::class_<DuckDBPyConnection> &m) {
281283
"Run a SQL query. If it is a SELECT statement, create a relation object from the given SQL query, otherwise "
282284
"run the query as-is.",
283285
py::arg("query"), py::kw_only(), py::arg("alias") = "", py::arg("params") = py::none());
284-
m.def("read_csv", &DuckDBPyConnection::ReadCSV, "Create a relation object from the CSV file in 'name'");
285-
m.def("from_csv_auto", &DuckDBPyConnection::ReadCSV, "Create a relation object from the CSV file in 'name'");
286+
// read_csv takes a positional source plus **kwargs of options. Bind via a py::args lambda so None is accepted as
287+
// the source: a typed py::object param would be rejected by nanobind before ReadCSV's body runs (and .none()
288+
// can't combine with py::kwargs), whereas a py::args tuple element may be None. ReadCSV itself raises the
289+
// "non file-like object" error for a None/invalid source.
290+
auto read_csv_fn = [](DuckDBPyConnection &self, py::args args, py::kwargs kwargs) {
291+
py::object name = args.size() >= 1 ? py::object(args[0]) : py::object(py::none());
292+
return self.ReadCSV(name, kwargs);
293+
};
294+
m.def("read_csv", read_csv_fn, "Create a relation object from the CSV file in 'name'");
295+
m.def("from_csv_auto", read_csv_fn, "Create a relation object from the CSV file in 'name'");
286296
m.def("from_df", &DuckDBPyConnection::FromDF, "Create a relation object from the DataFrame in df", py::arg("df"));
287297
m.def("from_arrow", &DuckDBPyConnection::FromArrow, "Create a relation object from an Arrow object",
288298
py::arg("arrow_object"));
@@ -316,11 +326,14 @@ void DuckDBPyConnection::UnregisterFilesystem(const py::str &name) {
316326
fs.ExtractSubSystem(py::cast<std::string>(name));
317327
}
318328

319-
void DuckDBPyConnection::RegisterFilesystem(AbstractFileSystem filesystem) {
329+
void DuckDBPyConnection::RegisterFilesystem(py::object filesystem) {
320330
PythonGILWrapper gil_wrapper;
321331

322332
auto &database = con.GetDatabase();
323-
if (!py::isinstance<AbstractFileSystem>(filesystem)) {
333+
// Import fsspec here (a normal, throwing context) so a missing install surfaces as ModuleNotFoundError, rather
334+
// than terminating inside the noexcept AbstractFileSystem type check (which nanobind cannot let throw).
335+
auto abstract_filesystem = py::module_::import_("fsspec").attr("AbstractFileSystem");
336+
if (filesystem.is_none() || !py::isinstance(filesystem, abstract_filesystem)) {
324337
throw InvalidInputException("Bad filesystem instance");
325338
}
326339

@@ -340,7 +353,7 @@ void DuckDBPyConnection::RegisterFilesystem(AbstractFileSystem filesystem) {
340353
}
341354
}
342355

343-
fs.RegisterSubSystem(make_uniq<PythonFilesystem>(std::move(protocols), std::move(filesystem)));
356+
fs.RegisterSubSystem(make_uniq<PythonFilesystem>(std::move(protocols), py::borrow<AbstractFileSystem>(filesystem)));
344357
}
345358

346359
py::list DuckDBPyConnection::ListFilesystems() {

0 commit comments

Comments
 (0)