From 9e3e4013ff40a045ec626d8550779b007232ac87 Mon Sep 17 00:00:00 2001
From: Dan Goodliffe <dan@randomdan.homeip.net>
Date: Sat, 20 Oct 2018 15:25:07 +0100
Subject: Fix assignable/convertible check in extract

Don't allow magic conversion between non-arithmetic and arithmetic types though.
Double to std::string is not sane in this context
---
 libdbpp/column.h                | 23 ++++++++++++++---------
 libdbpp/error.h                 |  2 +-
 libdbpp/unittests/testUtils.cpp | 19 +++++++++++++++++++
 3 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/libdbpp/column.h b/libdbpp/column.h
index 3cc974e..8989d17 100644
--- a/libdbpp/column.h
+++ b/libdbpp/column.h
@@ -48,11 +48,13 @@ namespace DB {
 			template<typename T>
 			class Extract : public DB::HandleField {
 				public:
-					template <typename> struct is_optional {
+					template <typename X> struct is_optional {
 						static constexpr bool value = false;
+						static constexpr bool is_arithmetic = std::is_arithmetic<X>::value;
 					};
 					template <typename X> struct is_optional<std::optional<X>> {
 						static constexpr bool value = true;
+						static constexpr bool is_arithmetic = std::is_arithmetic<X>::value;
 					};
 
 					Extract(T & t) : target(t) { }
@@ -78,15 +80,18 @@ namespace DB {
 					inline
 					void operator()(const D & v)
 					{
-						if constexpr (std::is_assignable<D, T>::value) {
-							target = v;
-						}
-						else if constexpr (std::is_convertible<D, T>::value) {
-							target = (T)v;
-						}
-						else {
-							throw InvalidConversion(typeid(D).name(), typeid(T).name());
+
+						if constexpr (is_optional<T>::is_arithmetic == std::is_arithmetic<D>::value) {
+							if constexpr (std::is_assignable<T, D>::value) {
+								target = v;
+								return;
+							}
+							if constexpr (std::is_convertible<T, D>::value) {
+								target = (T)v;
+								return;
+							}
 						}
+						throw InvalidConversion(typeid(D).name(), typeid(T).name());
 					}
 
 					T & target;
diff --git a/libdbpp/error.h b/libdbpp/error.h
index 99a28cb..05f0a39 100644
--- a/libdbpp/error.h
+++ b/libdbpp/error.h
@@ -37,7 +37,7 @@ namespace DB {
 	};
 
 	class DLL_PUBLIC UnexpectedNullValue : public AdHoc::Exception<Error> {
-		public:	
+		public:
 			UnexpectedNullValue(const char * const from);
 
 		private:
diff --git a/libdbpp/unittests/testUtils.cpp b/libdbpp/unittests/testUtils.cpp
index 6ff9edd..1911d9f 100644
--- a/libdbpp/unittests/testUtils.cpp
+++ b/libdbpp/unittests/testUtils.cpp
@@ -92,6 +92,25 @@ BOOST_AUTO_TEST_CASE( stdforOverRowsStructuredBinding )
 	BOOST_REQUIRE_EQUAL(totalOfc, "Some textSome text");
 }
 
+BOOST_AUTO_TEST_CASE( stdforOverRowsStructuredBindingOptional )
+{
+	auto db = DB::MockDatabase::openConnectionTo("pqmock");
+	unsigned int count = 0;
+	int64_t totalOfa = 0;
+	std::string totalOfc;
+	auto sel = db->select("SELECT a, c FROM forEachRow ORDER BY a DESC");
+	for (const auto [ a, c ] : sel->as<std::optional<int64_t>, std::optional<std::string>>()) {
+		count += 1;
+		BOOST_REQUIRE(a);
+		totalOfa += *a;
+		BOOST_REQUIRE(c);
+		totalOfc += *c;
+	}
+	BOOST_REQUIRE_EQUAL(count, 2);
+	BOOST_REQUIRE_EQUAL(totalOfa, 3);
+	BOOST_REQUIRE_EQUAL(totalOfc, "Some textSome text");
+}
+
 BOOST_AUTO_TEST_CASE( execute )
 {
 	auto db = DB::ConnectionPtr(DB::MockDatabase::openConnectionTo("pqmock"));
-- 
cgit v1.2.3