diff --git a/src/duckdb_py/include/duckdb_python/pybind11/conversions/enum_string_caster.hpp b/src/duckdb_py/include/duckdb_python/pybind11/conversions/enum_string_caster.hpp new file mode 100644 index 00000000..0bb72026 --- /dev/null +++ b/src/duckdb_py/include/duckdb_python/pybind11/conversions/enum_string_caster.hpp @@ -0,0 +1,96 @@ +#pragma once + +#include +#include +#include + +//===----------------------------------------------------------------------===// +// Reusable pybind11 type_caster macros for "string / integer or enum" arguments +//===----------------------------------------------------------------------===// +// +// Several DuckDB enums are exposed to Python so that a binding parameter typed as +// the enum also accepts a string (and, for most, an integer) naming one of its +// values, while still accepting an actual registered enum instance. Every one of +// these casters had an identical shape: +// +// - if the source is a Python str -> value = FromString(...) +// - if the source is a Python int -> value = FromInteger(...) (optional) +// - otherwise delegate to a *local* type_caster_base for the registered +// enum instance. +// +// The macros below collapse that boilerplate into a single invocation per enum so +// the eventual nanobind port is a one-place change. Behavior is intentionally +// identical to the hand-written casters they replace. +// +// IMPORTANT (matches the original per-file notes): these casters own their value +// via PYBIND11_TYPE_CASTER and delegate ONLY the registered-instance case to a +// local base caster -- they do NOT inherit type_caster_base. Inheriting the base +// while also writing custom branches is what historically made a caster accept +// str XOR the enum depending on include visibility. Each specialization must be +// visible in every TU that converts the type (they live under the universally +// included pybind_wrapper.hpp umbrella), otherwise it is UB. +// +// Invoke these macros at GLOBAL scope (outside any namespace); each expands to a +// full `namespace pybind11 { namespace detail { ... } }` specialization. Pass +// fully-qualified names (e.g. duckdb::ExplainTypeFromString) for the conversion +// functions and the enum type. + +//! str + int + registered-enum form. +#define DUCKDB_PY_ENUM_STRING_INT_CASTER(EnumType, FromStringFn, FromIntegerFn, NameLiteral) \ + namespace PYBIND11_NAMESPACE { \ + namespace detail { \ + template <> \ + struct type_caster { \ + PYBIND11_TYPE_CASTER(EnumType, const_name(NameLiteral)); \ + \ + bool load(handle src, bool convert) { \ + if (isinstance(src)) { \ + value = FromStringFn(src.cast()); \ + return true; \ + } \ + if (isinstance(src)) { \ + value = FromIntegerFn(src.cast()); \ + return true; \ + } \ + type_caster_base base; \ + if (!base.load(src, convert)) { \ + return false; \ + } \ + value = *static_cast(base); \ + return true; \ + } \ + \ + static handle cast(EnumType src, return_value_policy policy, handle parent) { \ + return type_caster_base::cast(src, policy, parent); \ + } \ + }; \ + } /* namespace detail */ \ + } /* namespace PYBIND11_NAMESPACE */ + +//! str + registered-enum form (no integer accepted). +#define DUCKDB_PY_ENUM_STRING_CASTER(EnumType, FromStringFn, NameLiteral) \ + namespace PYBIND11_NAMESPACE { \ + namespace detail { \ + template <> \ + struct type_caster { \ + PYBIND11_TYPE_CASTER(EnumType, const_name(NameLiteral)); \ + \ + bool load(handle src, bool convert) { \ + if (isinstance(src)) { \ + value = FromStringFn(src.cast()); \ + return true; \ + } \ + type_caster_base base; \ + if (!base.load(src, convert)) { \ + return false; \ + } \ + value = *static_cast(base); \ + return true; \ + } \ + \ + static handle cast(EnumType src, return_value_policy policy, handle parent) { \ + return type_caster_base::cast(src, policy, parent); \ + } \ + }; \ + } /* namespace detail */ \ + } /* namespace PYBIND11_NAMESPACE */ diff --git a/src/duckdb_py/include/duckdb_python/pybind11/conversions/exception_handling_enum.hpp b/src/duckdb_py/include/duckdb_python/pybind11/conversions/exception_handling_enum.hpp index 583cd7b9..94adf3d7 100644 --- a/src/duckdb_py/include/duckdb_python/pybind11/conversions/exception_handling_enum.hpp +++ b/src/duckdb_py/include/duckdb_python/pybind11/conversions/exception_handling_enum.hpp @@ -3,6 +3,7 @@ #include "duckdb/common/common.hpp" #include "duckdb/common/exception.hpp" #include "duckdb/common/string_util.hpp" +#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp" namespace duckdb { @@ -31,35 +32,6 @@ inline PythonExceptionHandling PythonExceptionHandlingFromInteger(int64_t value) } // namespace duckdb -namespace PYBIND11_NAMESPACE { -namespace detail { - -//! See python_udf_type_enum.hpp for the rationale (composition over inheritance, umbrella visibility). -template <> -struct type_caster { - PYBIND11_TYPE_CASTER(duckdb::PythonExceptionHandling, const_name("PythonExceptionHandling")); - - bool load(handle src, bool convert) { - if (isinstance(src)) { - value = duckdb::PythonExceptionHandlingFromString(src.cast()); - return true; - } - if (isinstance(src)) { - value = duckdb::PythonExceptionHandlingFromInteger(src.cast()); - return true; - } - type_caster_base base; - if (!base.load(src, convert)) { - return false; - } - value = *static_cast(base); - return true; - } - - static handle cast(duckdb::PythonExceptionHandling src, return_value_policy policy, handle parent) { - return type_caster_base::cast(src, policy, parent); - } -}; - -} // namespace detail -} // namespace PYBIND11_NAMESPACE +//! See enum_string_caster.hpp for the rationale (composition over inheritance, umbrella visibility). +DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::PythonExceptionHandling, duckdb::PythonExceptionHandlingFromString, + duckdb::PythonExceptionHandlingFromInteger, "PythonExceptionHandling") diff --git a/src/duckdb_py/include/duckdb_python/pybind11/conversions/explain_enum.hpp b/src/duckdb_py/include/duckdb_python/pybind11/conversions/explain_enum.hpp index 466e533d..e88f0c02 100644 --- a/src/duckdb_py/include/duckdb_python/pybind11/conversions/explain_enum.hpp +++ b/src/duckdb_py/include/duckdb_python/pybind11/conversions/explain_enum.hpp @@ -4,6 +4,7 @@ #include "duckdb/common/common.hpp" #include "duckdb/common/exception.hpp" #include "duckdb/common/string_util.hpp" +#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp" namespace duckdb { @@ -30,35 +31,6 @@ inline ExplainType ExplainTypeFromInteger(int64_t value) { } // namespace duckdb -namespace PYBIND11_NAMESPACE { -namespace detail { - -//! See python_udf_type_enum.hpp for the rationale (composition over inheritance, umbrella visibility). -template <> -struct type_caster { - PYBIND11_TYPE_CASTER(duckdb::ExplainType, const_name("ExplainType")); - - bool load(handle src, bool convert) { - if (isinstance(src)) { - value = duckdb::ExplainTypeFromString(src.cast()); - return true; - } - if (isinstance(src)) { - value = duckdb::ExplainTypeFromInteger(src.cast()); - return true; - } - type_caster_base base; - if (!base.load(src, convert)) { - return false; - } - value = *static_cast(base); - return true; - } - - static handle cast(duckdb::ExplainType src, return_value_policy policy, handle parent) { - return type_caster_base::cast(src, policy, parent); - } -}; - -} // namespace detail -} // namespace PYBIND11_NAMESPACE +//! See enum_string_caster.hpp for the rationale (composition over inheritance, umbrella visibility). +DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::ExplainType, duckdb::ExplainTypeFromString, duckdb::ExplainTypeFromInteger, + "ExplainType") diff --git a/src/duckdb_py/include/duckdb_python/pybind11/conversions/null_handling_enum.hpp b/src/duckdb_py/include/duckdb_python/pybind11/conversions/null_handling_enum.hpp index 5d0a3fcc..e5172706 100644 --- a/src/duckdb_py/include/duckdb_python/pybind11/conversions/null_handling_enum.hpp +++ b/src/duckdb_py/include/duckdb_python/pybind11/conversions/null_handling_enum.hpp @@ -4,6 +4,7 @@ #include "duckdb/common/common.hpp" #include "duckdb/common/exception.hpp" #include "duckdb/common/string_util.hpp" +#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp" namespace duckdb { @@ -30,37 +31,7 @@ inline FunctionNullHandling FunctionNullHandlingFromInteger(int64_t value) { } // namespace duckdb -namespace PYBIND11_NAMESPACE { -namespace detail { - -//! See python_udf_type_enum.hpp for why this owns its value and delegates the enum case to a local base -//! caster instead of inheriting type_caster_base. Must stay visible in every TU (included from -//! pybind_wrapper.hpp). -template <> -struct type_caster { - PYBIND11_TYPE_CASTER(duckdb::FunctionNullHandling, const_name("FunctionNullHandling")); - - bool load(handle src, bool convert) { - if (isinstance(src)) { - value = duckdb::FunctionNullHandlingFromString(src.cast()); - return true; - } - if (isinstance(src)) { - value = duckdb::FunctionNullHandlingFromInteger(src.cast()); - return true; - } - type_caster_base base; - if (!base.load(src, convert)) { - return false; - } - value = *static_cast(base); - return true; - } - - static handle cast(duckdb::FunctionNullHandling src, return_value_policy policy, handle parent) { - return type_caster_base::cast(src, policy, parent); - } -}; - -} // namespace detail -} // namespace PYBIND11_NAMESPACE +//! See enum_string_caster.hpp for why this owns its value and delegates the enum case to a local base caster +//! instead of inheriting type_caster_base. Must stay visible in every TU (included from pybind_wrapper.hpp). +DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::FunctionNullHandling, duckdb::FunctionNullHandlingFromString, + duckdb::FunctionNullHandlingFromInteger, "FunctionNullHandling") diff --git a/src/duckdb_py/include/duckdb_python/pybind11/conversions/pyconnection_default.hpp b/src/duckdb_py/include/duckdb_python/pybind11/conversions/pyconnection_default.hpp index e07a9b2e..ed35dc7e 100644 --- a/src/duckdb_py/include/duckdb_python/pybind11/conversions/pyconnection_default.hpp +++ b/src/duckdb_py/include/duckdb_python/pybind11/conversions/pyconnection_default.hpp @@ -10,6 +10,15 @@ namespace py = pybind11; namespace PYBIND11_NAMESPACE { namespace detail { +// NANOBIND PORTING NOTE (None handling): +// This caster maps a Python None (or an omitted `connection=None` argument) to the module-level default +// connection. It works under pybind11 because pybind11 forwards None into a holder/pointer argument's caster +// `load()` by default (argument_record.none defaults to true). nanobind inverts this: it REJECTS None for +// bound-type (shared_ptr / pointer) arguments BEFORE the caster runs, unless the binding annotates the argument +// with `.none()`. So the eventual nanobind port must (1) keep this None -> DefaultConnection() branch AND +// (2) add `.none()` to every `connection` argument that currently defaults to `py::none()` (see +// NANOBIND_NONE_AUDIT.md -- 81 sites in duckdb_python.cpp). Object-family arguments (py::object / Optional) +// do not need this annotation; their value casters accept None directly. template <> class type_caster> : public copyable_holder_caster> { diff --git a/src/duckdb_py/include/duckdb_python/pybind11/conversions/python_csv_line_terminator_enum.hpp b/src/duckdb_py/include/duckdb_python/pybind11/conversions/python_csv_line_terminator_enum.hpp index 12d807d0..34325262 100644 --- a/src/duckdb_py/include/duckdb_python/pybind11/conversions/python_csv_line_terminator_enum.hpp +++ b/src/duckdb_py/include/duckdb_python/pybind11/conversions/python_csv_line_terminator_enum.hpp @@ -3,6 +3,7 @@ #include "duckdb/common/common.hpp" #include "duckdb/common/exception.hpp" #include "duckdb/common/string_util.hpp" +#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp" namespace duckdb { @@ -41,32 +42,7 @@ struct PythonCSVLineTerminator { } // namespace duckdb -namespace PYBIND11_NAMESPACE { -namespace detail { - -//! See python_udf_type_enum.hpp for the rationale (composition over inheritance, umbrella visibility). +//! See enum_string_caster.hpp for the rationale (composition over inheritance, umbrella visibility). //! Only a string or the enum itself are accepted (no integer form). -template <> -struct type_caster { - PYBIND11_TYPE_CASTER(duckdb::PythonCSVLineTerminator::Type, const_name("CSVLineTerminator")); - - bool load(handle src, bool convert) { - if (isinstance(src)) { - value = duckdb::PythonCSVLineTerminator::FromString(src.cast()); - return true; - } - type_caster_base base; - if (!base.load(src, convert)) { - return false; - } - value = *static_cast(base); - return true; - } - - static handle cast(duckdb::PythonCSVLineTerminator::Type src, return_value_policy policy, handle parent) { - return type_caster_base::cast(src, policy, parent); - } -}; - -} // namespace detail -} // namespace PYBIND11_NAMESPACE +DUCKDB_PY_ENUM_STRING_CASTER(duckdb::PythonCSVLineTerminator::Type, duckdb::PythonCSVLineTerminator::FromString, + "CSVLineTerminator") diff --git a/src/duckdb_py/include/duckdb_python/pybind11/conversions/python_udf_type_enum.hpp b/src/duckdb_py/include/duckdb_python/pybind11/conversions/python_udf_type_enum.hpp index c012b2f5..13799ba0 100644 --- a/src/duckdb_py/include/duckdb_python/pybind11/conversions/python_udf_type_enum.hpp +++ b/src/duckdb_py/include/duckdb_python/pybind11/conversions/python_udf_type_enum.hpp @@ -3,6 +3,7 @@ #include "duckdb/common/common.hpp" #include "duckdb/common/exception.hpp" #include "duckdb/common/string_util.hpp" +#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp" namespace duckdb { @@ -31,41 +32,9 @@ inline PythonUDFType PythonUDFTypeFromInteger(int64_t value) { } // namespace duckdb -namespace PYBIND11_NAMESPACE { -namespace detail { - -//! Accepts the registered PythonUDFType enum, or a string / integer naming one. Unlike the previous version, -//! this does NOT inherit type_caster_base: it owns its value (PYBIND11_TYPE_CASTER) and delegates only the -//! enum case to a *local* base caster. Inheriting the base while also writing custom branches is what made -//! the old version accept str XOR the enum depending on include visibility. This specialization must be -//! visible in every TU that converts PythonUDFType (it is included from pybind_wrapper.hpp), otherwise it is -//! UB. Keeping the binding parameter typed as the enum preserves the type + default in help()/stubs. -template <> -struct type_caster { - PYBIND11_TYPE_CASTER(duckdb::PythonUDFType, const_name("PythonUDFType")); - - bool load(handle src, bool convert) { - if (isinstance(src)) { - value = duckdb::PythonUDFTypeFromString(src.cast()); - return true; - } - if (isinstance(src)) { - value = duckdb::PythonUDFTypeFromInteger(src.cast()); - return true; - } - // Otherwise it must be an actual (registered) PythonUDFType instance. - type_caster_base base; - if (!base.load(src, convert)) { - return false; - } - value = *static_cast(base); - return true; - } - - static handle cast(duckdb::PythonUDFType src, return_value_policy policy, handle parent) { - return type_caster_base::cast(src, policy, parent); - } -}; - -} // namespace detail -} // namespace PYBIND11_NAMESPACE +//! Accepts the registered PythonUDFType enum, or a string / integer naming one. See enum_string_caster.hpp for +//! the rationale (this owns its value via PYBIND11_TYPE_CASTER and delegates only the registered-enum case to a +//! local base caster instead of inheriting type_caster_base). Keeping the binding parameter typed as the enum +//! preserves the type + default in help()/stubs. +DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::PythonUDFType, duckdb::PythonUDFTypeFromString, + duckdb::PythonUDFTypeFromInteger, "PythonUDFType") diff --git a/src/duckdb_py/include/duckdb_python/pybind11/conversions/render_mode_enum.hpp b/src/duckdb_py/include/duckdb_python/pybind11/conversions/render_mode_enum.hpp index bd45e2d8..a6e0e6ea 100644 --- a/src/duckdb_py/include/duckdb_python/pybind11/conversions/render_mode_enum.hpp +++ b/src/duckdb_py/include/duckdb_python/pybind11/conversions/render_mode_enum.hpp @@ -5,6 +5,7 @@ #include "duckdb/common/string_util.hpp" #include "duckdb/common/box_renderer.hpp" #include "duckdb/common/enum_util.hpp" +#include "duckdb_python/pybind11/conversions/enum_string_caster.hpp" namespace duckdb { @@ -24,35 +25,6 @@ inline RenderMode RenderModeFromInteger(int64_t value) { } // namespace duckdb -namespace PYBIND11_NAMESPACE { -namespace detail { - -//! See python_udf_type_enum.hpp for the rationale (composition over inheritance, umbrella visibility). -template <> -struct type_caster { - PYBIND11_TYPE_CASTER(duckdb::RenderMode, const_name("RenderMode")); - - bool load(handle src, bool convert) { - if (isinstance(src)) { - value = duckdb::RenderModeFromString(src.cast()); - return true; - } - if (isinstance(src)) { - value = duckdb::RenderModeFromInteger(src.cast()); - return true; - } - type_caster_base base; - if (!base.load(src, convert)) { - return false; - } - value = *static_cast(base); - return true; - } - - static handle cast(duckdb::RenderMode src, return_value_policy policy, handle parent) { - return type_caster_base::cast(src, policy, parent); - } -}; - -} // namespace detail -} // namespace PYBIND11_NAMESPACE +//! See enum_string_caster.hpp for the rationale (composition over inheritance, umbrella visibility). +DUCKDB_PY_ENUM_STRING_INT_CASTER(duckdb::RenderMode, duckdb::RenderModeFromString, duckdb::RenderModeFromInteger, + "RenderMode")