fix pybind type casters for enums

Author: evertlammerts

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

@@ -4,67 +4,60 @@
 #include "duckdb/common/exception.hpp"
 #include "duckdb/common/string_util.hpp"
 
-using duckdb::InvalidInputException;
-using duckdb::string;
-using duckdb::StringUtil;
-
 namespace duckdb {
 
 enum class PythonExceptionHandling : uint8_t { FORWARD_ERROR, RETURN_NULL };
 
-} // namespace duckdb
-
-using duckdb::PythonExceptionHandling;
-
-namespace py = pybind11;
-
-static PythonExceptionHandling PythonExceptionHandlingFromString(const string &type) {
+inline PythonExceptionHandling PythonExceptionHandlingFromString(const string &type) {
 	auto ltype = StringUtil::Lower(type);
 	if (ltype.empty() || ltype == "default") {
 		return PythonExceptionHandling::FORWARD_ERROR;
-	} else if (ltype == "return_null") {
+	}
+	if (ltype == "return_null") {
 		return PythonExceptionHandling::RETURN_NULL;
-	} else {
-		throw InvalidInputException("'%s' is not a recognized type for 'exception_handling'", type);
 	}
+	throw InvalidInputException("'%s' is not a recognized type for 'exception_handling'", type);
 }
 
-static PythonExceptionHandling PythonExceptionHandlingFromInteger(int64_t value) {
+inline PythonExceptionHandling PythonExceptionHandlingFromInteger(int64_t value) {
 	if (value == 0) {
 		return PythonExceptionHandling::FORWARD_ERROR;
-	} else if (value == 1) {
+	}
+	if (value == 1) {
 		return PythonExceptionHandling::RETURN_NULL;
-	} else {
-		throw InvalidInputException("'%d' is not a recognized type for 'exception_handling'", value);
 	}
+	throw InvalidInputException("'%d' is not a recognized type for 'exception_handling'", 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<PythonExceptionHandling> : public type_caster_base<PythonExceptionHandling> {
-	using base = type_caster_base<PythonExceptionHandling>;
-	PythonExceptionHandling tmp;
+struct type_caster<duckdb::PythonExceptionHandling> {
+	PYBIND11_TYPE_CASTER(duckdb::PythonExceptionHandling, const_name("PythonExceptionHandling"));
 
-public:
 	bool load(handle src, bool convert) {
-		if (base::load(src, convert)) {
+		if (isinstance<str>(src)) {
+			value = duckdb::PythonExceptionHandlingFromString(src.cast<std::string>());
 			return true;
-		} else if (py::isinstance<py::str>(src)) {
-			tmp = PythonExceptionHandlingFromString(py::str(src));
-			value = &tmp;
-			return true;
-		} else if (py::isinstance<py::int_>(src)) {
-			tmp = PythonExceptionHandlingFromInteger(src.cast<int64_t>());
-			value = &tmp;
+		}
+		if (isinstance<int_>(src)) {
+			value = duckdb::PythonExceptionHandlingFromInteger(src.cast<int64_t>());
 			return true;
 		}
-		return false;
+		type_caster_base<duckdb::PythonExceptionHandling> base;
+		if (!base.load(src, convert)) {
+			return false;
+		}
+		value = *static_cast<duckdb::PythonExceptionHandling *>(base);
+		return true;
 	}
 
-	static handle cast(PythonExceptionHandling src, return_value_policy policy, handle parent) {
-		return base::cast(src, policy, parent);
+	static handle cast(duckdb::PythonExceptionHandling src, return_value_policy policy, handle parent) {
+		return type_caster_base<duckdb::PythonExceptionHandling>::cast(src, policy, parent);
 	}
 };
 

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

@@ -5,46 +5,60 @@
 #include "duckdb/common/exception.hpp"
 #include "duckdb/common/string_util.hpp"
 
-using duckdb::ExplainType;
-using duckdb::InvalidInputException;
-using duckdb::string;
-using duckdb::StringUtil;
+namespace duckdb {
 
-namespace py = pybind11;
-
-static ExplainType ExplainTypeFromString(const string &type) {
+inline ExplainType ExplainTypeFromString(const string &type) {
 	auto ltype = StringUtil::Lower(type);
 	if (ltype.empty() || ltype == "standard") {
 		return ExplainType::EXPLAIN_STANDARD;
-	} else if (ltype == "analyze") {
+	}
+	if (ltype == "analyze") {
 		return ExplainType::EXPLAIN_ANALYZE;
-	} else {
-		throw InvalidInputException("Unrecognized type for 'explain'");
 	}
+	throw InvalidInputException("Unrecognized type for 'explain'");
 }
 
-static ExplainType ExplainTypeFromInteger(int64_t value) {
+inline ExplainType ExplainTypeFromInteger(int64_t value) {
 	if (value == 0) {
 		return ExplainType::EXPLAIN_STANDARD;
-	} else if (value == 1) {
+	}
+	if (value == 1) {
 		return ExplainType::EXPLAIN_ANALYZE;
-	} else {
-		throw InvalidInputException("Unrecognized type for 'explain'");
 	}
+	throw InvalidInputException("Unrecognized type for 'explain'");
 }
 
-//! Resolve a Python explain-type argument (ExplainType enum, str, or int) to an ExplainType.
-//! NOTE: deliberately NOT a pybind type_caster. A custom caster inheriting type_caster_base shadows the
-//! registered py::enum_ inconsistently across translation units - it ends up accepting str/int XOR the enum
-//! instance, never both, depending on which TU sees the specialization. Explicit dispatch at the call site is
-//! robust regardless of include order.
-static ExplainType ExplainTypeFromPython(const py::object &obj) {
-	if (py::isinstance<py::str>(obj)) {
-		return ExplainTypeFromString(py::str(obj));
+} // 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<duckdb::ExplainType> {
+	PYBIND11_TYPE_CASTER(duckdb::ExplainType, const_name("ExplainType"));
+
+	bool load(handle src, bool convert) {
+		if (isinstance<str>(src)) {
+			value = duckdb::ExplainTypeFromString(src.cast<std::string>());
+			return true;
+		}
+		if (isinstance<int_>(src)) {
+			value = duckdb::ExplainTypeFromInteger(src.cast<int64_t>());
+			return true;
+		}
+		type_caster_base<duckdb::ExplainType> base;
+		if (!base.load(src, convert)) {
+			return false;
+		}
+		value = *static_cast<duckdb::ExplainType *>(base);
+		return true;
 	}
-	if (py::isinstance<py::int_>(obj)) {
-		return ExplainTypeFromInteger(obj.cast<int64_t>());
+
+	static handle cast(duckdb::ExplainType src, return_value_policy policy, handle parent) {
+		return type_caster_base<duckdb::ExplainType>::cast(src, policy, parent);
 	}
-	// Fall through to the registered py::enum_ caster (handles an actual ExplainType, throws otherwise).
-	return obj.cast<ExplainType>();
-}
+};
+
+} // namespace detail
+} // namespace PYBIND11_NAMESPACE

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

@@ -5,60 +5,60 @@
 #include "duckdb/common/exception.hpp"
 #include "duckdb/common/string_util.hpp"
 
-using duckdb::FunctionNullHandling;
-using duckdb::InvalidInputException;
-using duckdb::string;
-using duckdb::StringUtil;
+namespace duckdb {
 
-namespace py = pybind11;
-
-static FunctionNullHandling FunctionNullHandlingFromString(const string &type) {
+inline FunctionNullHandling FunctionNullHandlingFromString(const string &type) {
 	auto ltype = StringUtil::Lower(type);
 	if (ltype.empty() || ltype == "default") {
 		return FunctionNullHandling::DEFAULT_NULL_HANDLING;
-	} else if (ltype == "special") {
+	}
+	if (ltype == "special") {
 		return FunctionNullHandling::SPECIAL_HANDLING;
-	} else {
-		throw InvalidInputException("'%s' is not a recognized type for 'null_handling'", type);
 	}
+	throw InvalidInputException("'%s' is not a recognized type for 'null_handling'", type);
 }
 
-static FunctionNullHandling FunctionNullHandlingFromInteger(int64_t value) {
+inline FunctionNullHandling FunctionNullHandlingFromInteger(int64_t value) {
 	if (value == 0) {
 		return FunctionNullHandling::DEFAULT_NULL_HANDLING;
-	} else if (value == 1) {
+	}
+	if (value == 1) {
 		return FunctionNullHandling::SPECIAL_HANDLING;
-	} else {
-		throw InvalidInputException("'%d' is not a recognized type for 'null_handling'", value);
 	}
+	throw InvalidInputException("'%d' is not a recognized type for 'null_handling'", 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<FunctionNullHandling> : public type_caster_base<FunctionNullHandling> {
-	using base = type_caster_base<FunctionNullHandling>;
-	FunctionNullHandling tmp;
+struct type_caster<duckdb::FunctionNullHandling> {
+	PYBIND11_TYPE_CASTER(duckdb::FunctionNullHandling, const_name("FunctionNullHandling"));
 
-public:
 	bool load(handle src, bool convert) {
-		if (base::load(src, convert)) {
+		if (isinstance<str>(src)) {
+			value = duckdb::FunctionNullHandlingFromString(src.cast<std::string>());
 			return true;
-		} else if (py::isinstance<py::str>(src)) {
-			tmp = FunctionNullHandlingFromString(py::str(src));
-			value = &tmp;
-			return true;
-		} else if (py::isinstance<py::int_>(src)) {
-			tmp = FunctionNullHandlingFromInteger(src.cast<int64_t>());
-			value = &tmp;
+		}
+		if (isinstance<int_>(src)) {
+			value = duckdb::FunctionNullHandlingFromInteger(src.cast<int64_t>());
 			return true;
 		}
-		return false;
+		type_caster_base<duckdb::FunctionNullHandling> base;
+		if (!base.load(src, convert)) {
+			return false;
+		}
+		value = *static_cast<duckdb::FunctionNullHandling *>(base);
+		return true;
 	}
 
-	static handle cast(FunctionNullHandling src, return_value_policy policy, handle parent) {
-		return base::cast(src, policy, parent);
+	static handle cast(duckdb::FunctionNullHandling src, return_value_policy policy, handle parent) {
+		return type_caster_base<duckdb::FunctionNullHandling>::cast(src, policy, parent);
 	}
 };
 

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

@@ -4,10 +4,6 @@
 #include "duckdb/common/exception.hpp"
 #include "duckdb/common/string_util.hpp"
 
-using duckdb::InvalidInputException;
-using duckdb::string;
-using duckdb::StringUtil;
-
 namespace duckdb {
 
 struct PythonCSVLineTerminator {
@@ -45,32 +41,30 @@ struct PythonCSVLineTerminator {
 
 } // namespace duckdb
 
-using duckdb::PythonCSVLineTerminator;
-
-namespace py = pybind11;
-
 namespace PYBIND11_NAMESPACE {
 namespace detail {
 
+//! See python_udf_type_enum.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<PythonCSVLineTerminator::Type> : public type_caster_base<PythonCSVLineTerminator::Type> {
-	using base = type_caster_base<PythonCSVLineTerminator::Type>;
-	PythonCSVLineTerminator::Type tmp;
+struct type_caster<duckdb::PythonCSVLineTerminator::Type> {
+	PYBIND11_TYPE_CASTER(duckdb::PythonCSVLineTerminator::Type, const_name("CSVLineTerminator"));
 
-public:
 	bool load(handle src, bool convert) {
-		if (base::load(src, convert)) {
-			return true;
-		} else if (py::isinstance<py::str>(src)) {
-			tmp = duckdb::PythonCSVLineTerminator::FromString(py::str(src));
-			value = &tmp;
+		if (isinstance<str>(src)) {
+			value = duckdb::PythonCSVLineTerminator::FromString(src.cast<std::string>());
 			return true;
 		}
-		return false;
+		type_caster_base<duckdb::PythonCSVLineTerminator::Type> base;
+		if (!base.load(src, convert)) {
+			return false;
+		}
+		value = *static_cast<duckdb::PythonCSVLineTerminator::Type *>(base);
+		return true;
 	}
 
-	static handle cast(PythonCSVLineTerminator::Type src, return_value_policy policy, handle parent) {
-		return base::cast(src, policy, parent);
+	static handle cast(duckdb::PythonCSVLineTerminator::Type src, return_value_policy policy, handle parent) {
+		return type_caster_base<duckdb::PythonCSVLineTerminator::Type>::cast(src, policy, parent);
 	}
 };