Skip to content

Commit 664d7e4

Browse files
evertlammertsevertlammerts
authored
Consolidate enum casters into reusable macro (#514)
Shrink the nanobind-porting surface of the custom type casters (pybind11-side prep only; no nanobind introduced). Enum caster consolidation: - Add conversions/enum_string_caster.hpp defining two reusable macros: DUCKDB_PY_ENUM_STRING_INT_CASTER(EnumType, FromStringFn, FromIntegerFn, Name) for the str+int+enum shape, and DUCKDB_PY_ENUM_STRING_CASTER(EnumType, FromStringFn, Name) for the str-only shape (CSV line terminator). - Reduce the 6 hand-written type_caster specializations (explain, render_mode, python_csv_line_terminator, python_udf_type, null_handling, exception_handling) to a single macro invocation each. Behavior is identical; the per-enum FromString/FromInteger free functions are unchanged. The eventual nanobind caster rewrite is now a one-place change. None-nullability audit: - Add an in-code nanobind porting note to pyconnection_default.hpp.
2 parents 4542c30 + 7c26386 commit 664d7e4

8 files changed

Lines changed: 133 additions & 196 deletions

File tree

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

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
#pragma once
2+
3+
#include <pybind11/pybind11.h>
4+
#include <cstdint>
5+
#include <string>
6+
7+
//===----------------------------------------------------------------------===//
8+
// Reusable pybind11 type_caster macros for "string / integer or enum" arguments
9+
//===----------------------------------------------------------------------===//
10+
//
11+
// Several DuckDB enums are exposed to Python so that a binding parameter typed as
12+
// the enum also accepts a string (and, for most, an integer) naming one of its
13+
// values, while still accepting an actual registered enum instance. Every one of
14+
// these casters had an identical shape:
15+
//
16+
// - if the source is a Python str -> value = <Enum>FromString(...)
17+
// - if the source is a Python int -> value = <Enum>FromInteger(...) (optional)
18+
// - otherwise delegate to a *local* type_caster_base<Enum> for the registered
19+
// enum instance.
20+
//
21+
// The macros below collapse that boilerplate into a single invocation per enum so
22+
// the eventual nanobind port is a one-place change. Behavior is intentionally
23+
// identical to the hand-written casters they replace.
24+
//
25+
// IMPORTANT (matches the original per-file notes): these casters own their value
26+
// via PYBIND11_TYPE_CASTER and delegate ONLY the registered-instance case to a
27+
// local base caster -- they do NOT inherit type_caster_base. Inheriting the base
28+
// while also writing custom branches is what historically made a caster accept
29+
// str XOR the enum depending on include visibility. Each specialization must be
30+
// visible in every TU that converts the type (they live under the universally
31+
// included pybind_wrapper.hpp umbrella), otherwise it is UB.
32+
//
33+
// Invoke these macros at GLOBAL scope (outside any namespace); each expands to a
34+
// full `namespace pybind11 { namespace detail { ... } }` specialization. Pass
35+
// fully-qualified names (e.g. duckdb::ExplainTypeFromString) for the conversion
36+
// functions and the enum type.
37+
38+
//! str + int + registered-enum form.
39+
#define DUCKDB_PY_ENUM_STRING_INT_CASTER(EnumType, FromStringFn, FromIntegerFn, NameLiteral) \
40+
namespace PYBIND11_NAMESPACE { \
41+
namespace detail { \
42+
template <> \
43+
struct type_caster<EnumType> { \
44+
PYBIND11_TYPE_CASTER(EnumType, const_name(NameLiteral)); \
45+
\
46+
bool load(handle src, bool convert) { \
47+
if (isinstance<str>(src)) { \
48+
value = FromStringFn(src.cast<std::string>()); \
49+
return true; \
50+
} \
51+
if (isinstance<int_>(src)) { \
52+
value = FromIntegerFn(src.cast<int64_t>()); \
53+
return true; \
54+
} \
55+
type_caster_base<EnumType> base; \
56+
if (!base.load(src, convert)) { \
57+
return false; \
58+
} \
59+
value = *static_cast<EnumType *>(base); \
60+
return true; \
61+
} \
62+
\
63+
static handle cast(EnumType src, return_value_policy policy, handle parent) { \
64+
return type_caster_base<EnumType>::cast(src, policy, parent); \
65+
} \
66+
}; \
67+
} /* namespace detail */ \
68+
} /* namespace PYBIND11_NAMESPACE */
69+
70+
//! str + registered-enum form (no integer accepted).
71+
#define DUCKDB_PY_ENUM_STRING_CASTER(EnumType, FromStringFn, NameLiteral) \
72+
namespace PYBIND11_NAMESPACE { \
73+
namespace detail { \
74+
template <> \
75+
struct type_caster<EnumType> { \
76+
PYBIND11_TYPE_CASTER(EnumType, const_name(NameLiteral)); \
77+
\
78+
bool load(handle src, bool convert) { \
79+
if (isinstance<str>(src)) { \
80+
value = FromStringFn(src.cast<std::string>()); \
81+
return true; \
82+
} \
83+
type_caster_base<EnumType> base; \
84+
if (!base.load(src, convert)) { \
85+
return false; \
86+
} \
87+
value = *static_cast<EnumType *>(base); \
88+
return true; \
89+
} \
90+
\
91+
static handle cast(EnumType src, return_value_policy policy, handle parent) { \
92+
return type_caster_base<EnumType>::cast(src, policy, parent); \
93+
} \
94+
}; \
95+
} /* namespace detail */ \
96+
} /* namespace PYBIND11_NAMESPACE */

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

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "duckdb/common/common.hpp"
44
#include "duckdb/common/exception.hpp"
55
#include "duckdb/common/string_util.hpp"
6+
#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp"
67

78
namespace duckdb {
89

@@ -31,35 +32,6 @@ inline PythonExceptionHandling PythonExceptionHandlingFromInteger(int64_t value)
3132

3233
} // namespace duckdb
3334

34-
namespace PYBIND11_NAMESPACE {
35-
namespace detail {
36-
37-
//! See python_udf_type_enum.hpp for the rationale (composition over inheritance, umbrella visibility).
38-
template <>
39-
struct type_caster<duckdb::PythonExceptionHandling> {
40-
PYBIND11_TYPE_CASTER(duckdb::PythonExceptionHandling, const_name("PythonExceptionHandling"));
41-
42-
bool load(handle src, bool convert) {
43-
if (isinstance<str>(src)) {
44-
value = duckdb::PythonExceptionHandlingFromString(src.cast<std::string>());
45-
return true;
46-
}
47-
if (isinstance<int_>(src)) {
48-
value = duckdb::PythonExceptionHandlingFromInteger(src.cast<int64_t>());
49-
return true;
50-
}
51-
type_caster_base<duckdb::PythonExceptionHandling> base;
52-
if (!base.load(src, convert)) {
53-
return false;
54-
}
55-
value = *static_cast<duckdb::PythonExceptionHandling *>(base);
56-
return true;
57-
}
58-
59-
static handle cast(duckdb::PythonExceptionHandling src, return_value_policy policy, handle parent) {
60-
return type_caster_base<duckdb::PythonExceptionHandling>::cast(src, policy, parent);
61-
}
62-
};
63-
64-
} // namespace detail
65-
} // namespace PYBIND11_NAMESPACE
35+
//! See enum_string_caster.hpp for the rationale (composition over inheritance, umbrella visibility).
36+
DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::PythonExceptionHandling, duckdb::PythonExceptionHandlingFromString,
37+
duckdb::PythonExceptionHandlingFromInteger, "PythonExceptionHandling")

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

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "duckdb/common/common.hpp"
55
#include "duckdb/common/exception.hpp"
66
#include "duckdb/common/string_util.hpp"
7+
#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp"
78

89
namespace duckdb {
910

@@ -30,35 +31,6 @@ inline ExplainType ExplainTypeFromInteger(int64_t value) {
3031

3132
} // namespace duckdb
3233

33-
namespace PYBIND11_NAMESPACE {
34-
namespace detail {
35-
36-
//! See python_udf_type_enum.hpp for the rationale (composition over inheritance, umbrella visibility).
37-
template <>
38-
struct type_caster<duckdb::ExplainType> {
39-
PYBIND11_TYPE_CASTER(duckdb::ExplainType, const_name("ExplainType"));
40-
41-
bool load(handle src, bool convert) {
42-
if (isinstance<str>(src)) {
43-
value = duckdb::ExplainTypeFromString(src.cast<std::string>());
44-
return true;
45-
}
46-
if (isinstance<int_>(src)) {
47-
value = duckdb::ExplainTypeFromInteger(src.cast<int64_t>());
48-
return true;
49-
}
50-
type_caster_base<duckdb::ExplainType> base;
51-
if (!base.load(src, convert)) {
52-
return false;
53-
}
54-
value = *static_cast<duckdb::ExplainType *>(base);
55-
return true;
56-
}
57-
58-
static handle cast(duckdb::ExplainType src, return_value_policy policy, handle parent) {
59-
return type_caster_base<duckdb::ExplainType>::cast(src, policy, parent);
60-
}
61-
};
62-
63-
} // namespace detail
64-
} // namespace PYBIND11_NAMESPACE
34+
//! See enum_string_caster.hpp for the rationale (composition over inheritance, umbrella visibility).
35+
DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::ExplainType, duckdb::ExplainTypeFromString, duckdb::ExplainTypeFromInteger,
36+
"ExplainType")

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

Lines changed: 5 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "duckdb/common/common.hpp"
55
#include "duckdb/common/exception.hpp"
66
#include "duckdb/common/string_util.hpp"
7+
#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp"
78

89
namespace duckdb {
910

@@ -30,37 +31,7 @@ inline FunctionNullHandling FunctionNullHandlingFromInteger(int64_t value) {
3031

3132
} // namespace duckdb
3233

33-
namespace PYBIND11_NAMESPACE {
34-
namespace detail {
35-
36-
//! See python_udf_type_enum.hpp for why this owns its value and delegates the enum case to a local base
37-
//! caster instead of inheriting type_caster_base. Must stay visible in every TU (included from
38-
//! pybind_wrapper.hpp).
39-
template <>
40-
struct type_caster<duckdb::FunctionNullHandling> {
41-
PYBIND11_TYPE_CASTER(duckdb::FunctionNullHandling, const_name("FunctionNullHandling"));
42-
43-
bool load(handle src, bool convert) {
44-
if (isinstance<str>(src)) {
45-
value = duckdb::FunctionNullHandlingFromString(src.cast<std::string>());
46-
return true;
47-
}
48-
if (isinstance<int_>(src)) {
49-
value = duckdb::FunctionNullHandlingFromInteger(src.cast<int64_t>());
50-
return true;
51-
}
52-
type_caster_base<duckdb::FunctionNullHandling> base;
53-
if (!base.load(src, convert)) {
54-
return false;
55-
}
56-
value = *static_cast<duckdb::FunctionNullHandling *>(base);
57-
return true;
58-
}
59-
60-
static handle cast(duckdb::FunctionNullHandling src, return_value_policy policy, handle parent) {
61-
return type_caster_base<duckdb::FunctionNullHandling>::cast(src, policy, parent);
62-
}
63-
};
64-
65-
} // namespace detail
66-
} // namespace PYBIND11_NAMESPACE
34+
//! See enum_string_caster.hpp for why this owns its value and delegates the enum case to a local base caster
35+
//! instead of inheriting type_caster_base. Must stay visible in every TU (included from pybind_wrapper.hpp).
36+
DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::FunctionNullHandling, duckdb::FunctionNullHandlingFromString,
37+
duckdb::FunctionNullHandlingFromInteger, "FunctionNullHandling")

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@ namespace py = pybind11;
1010
namespace PYBIND11_NAMESPACE {
1111
namespace detail {
1212

13+
// NANOBIND PORTING NOTE (None handling):
14+
// This caster maps a Python None (or an omitted `connection=None` argument) to the module-level default
15+
// connection. It works under pybind11 because pybind11 forwards None into a holder/pointer argument's caster
16+
// `load()` by default (argument_record.none defaults to true). nanobind inverts this: it REJECTS None for
17+
// bound-type (shared_ptr / pointer) arguments BEFORE the caster runs, unless the binding annotates the argument
18+
// with `.none()`. So the eventual nanobind port must (1) keep this None -> DefaultConnection() branch AND
19+
// (2) add `.none()` to every `connection` argument that currently defaults to `py::none()` (see
20+
// NANOBIND_NONE_AUDIT.md -- 81 sites in duckdb_python.cpp). Object-family arguments (py::object / Optional<T>)
21+
// do not need this annotation; their value casters accept None directly.
1322
template <>
1423
class type_caster<std::shared_ptr<DuckDBPyConnection>>
1524
: public copyable_holder_caster<DuckDBPyConnection, std::shared_ptr<DuckDBPyConnection>> {

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

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "duckdb/common/common.hpp"
44
#include "duckdb/common/exception.hpp"
55
#include "duckdb/common/string_util.hpp"
6+
#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp"
67

78
namespace duckdb {
89

@@ -41,32 +42,7 @@ struct PythonCSVLineTerminator {
4142

4243
} // namespace duckdb
4344

44-
namespace PYBIND11_NAMESPACE {
45-
namespace detail {
46-
47-
//! See python_udf_type_enum.hpp for the rationale (composition over inheritance, umbrella visibility).
45+
//! See enum_string_caster.hpp for the rationale (composition over inheritance, umbrella visibility).
4846
//! Only a string or the enum itself are accepted (no integer form).
49-
template <>
50-
struct type_caster<duckdb::PythonCSVLineTerminator::Type> {
51-
PYBIND11_TYPE_CASTER(duckdb::PythonCSVLineTerminator::Type, const_name("CSVLineTerminator"));
52-
53-
bool load(handle src, bool convert) {
54-
if (isinstance<str>(src)) {
55-
value = duckdb::PythonCSVLineTerminator::FromString(src.cast<std::string>());
56-
return true;
57-
}
58-
type_caster_base<duckdb::PythonCSVLineTerminator::Type> base;
59-
if (!base.load(src, convert)) {
60-
return false;
61-
}
62-
value = *static_cast<duckdb::PythonCSVLineTerminator::Type *>(base);
63-
return true;
64-
}
65-
66-
static handle cast(duckdb::PythonCSVLineTerminator::Type src, return_value_policy policy, handle parent) {
67-
return type_caster_base<duckdb::PythonCSVLineTerminator::Type>::cast(src, policy, parent);
68-
}
69-
};
70-
71-
} // namespace detail
72-
} // namespace PYBIND11_NAMESPACE
47+
DUCKDB_PY_ENUM_STRING_CASTER(duckdb::PythonCSVLineTerminator::Type, duckdb::PythonCSVLineTerminator::FromString,
48+
"CSVLineTerminator")

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

Lines changed: 7 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include "duckdb/common/common.hpp"
44
#include "duckdb/common/exception.hpp"
55
#include "duckdb/common/string_util.hpp"
6+
#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp"
67

78
namespace duckdb {
89

@@ -31,41 +32,9 @@ inline PythonUDFType PythonUDFTypeFromInteger(int64_t value) {
3132

3233
} // namespace duckdb
3334

34-
namespace PYBIND11_NAMESPACE {
35-
namespace detail {
36-
37-
//! Accepts the registered PythonUDFType enum, or a string / integer naming one. Unlike the previous version,
38-
//! this does NOT inherit type_caster_base: it owns its value (PYBIND11_TYPE_CASTER) and delegates only the
39-
//! enum case to a *local* base caster. Inheriting the base while also writing custom branches is what made
40-
//! the old version accept str XOR the enum depending on include visibility. This specialization must be
41-
//! visible in every TU that converts PythonUDFType (it is included from pybind_wrapper.hpp), otherwise it is
42-
//! UB. Keeping the binding parameter typed as the enum preserves the type + default in help()/stubs.
43-
template <>
44-
struct type_caster<duckdb::PythonUDFType> {
45-
PYBIND11_TYPE_CASTER(duckdb::PythonUDFType, const_name("PythonUDFType"));
46-
47-
bool load(handle src, bool convert) {
48-
if (isinstance<str>(src)) {
49-
value = duckdb::PythonUDFTypeFromString(src.cast<std::string>());
50-
return true;
51-
}
52-
if (isinstance<int_>(src)) {
53-
value = duckdb::PythonUDFTypeFromInteger(src.cast<int64_t>());
54-
return true;
55-
}
56-
// Otherwise it must be an actual (registered) PythonUDFType instance.
57-
type_caster_base<duckdb::PythonUDFType> base;
58-
if (!base.load(src, convert)) {
59-
return false;
60-
}
61-
value = *static_cast<duckdb::PythonUDFType *>(base);
62-
return true;
63-
}
64-
65-
static handle cast(duckdb::PythonUDFType src, return_value_policy policy, handle parent) {
66-
return type_caster_base<duckdb::PythonUDFType>::cast(src, policy, parent);
67-
}
68-
};
69-
70-
} // namespace detail
71-
} // namespace PYBIND11_NAMESPACE
35+
//! Accepts the registered PythonUDFType enum, or a string / integer naming one. See enum_string_caster.hpp for
36+
//! the rationale (this owns its value via PYBIND11_TYPE_CASTER and delegates only the registered-enum case to a
37+
//! local base caster instead of inheriting type_caster_base). Keeping the binding parameter typed as the enum
38+
//! preserves the type + default in help()/stubs.
39+
DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::PythonUDFType, duckdb::PythonUDFTypeFromString,
40+
duckdb::PythonUDFTypeFromInteger, "PythonUDFType")

0 commit comments

Comments
 (0)