Support for bool, std::filesystem::path, std::unique_ptr, std::shared_ptr

[e1y4s]

• Add support (i.e. type_conversion specializations) for:
  • bool
  • std::filesystem::path
  • std::unique_ptr
  • std::shared_ptr

[vadz]

Thanks for the PR!

I think pointer specializations are useful, although I must admit I had somehow never have any need for them personally, and std::fs::path one might be as well, although it's not totally clear how much advantage it brings compared to just using strings.

But the bool one is probably -- and unless I'm missing something -- not 100% correct and I think we should add "true" boolean support instead.

[vadz]

The headers here are sorted in alphabetical order except the very first one which needs to come first, so this line should be moved above.

[e1y4s]

Ok thanks, I will make the change.

[vadz]

I'm really not sure about this one, there is boolean SQL type and I think we ought to use it instead of storing bool as short.

[zann1x]

I feel like adding this specialization creates more confusion than anything else. At least the postgres backend has a mapping from its SQL boolean type to a C++ int type already.

[e1y4s]

The objective behind this addition was to be able to use the C++ bool type directly in my model and use it with soci

C++ bool <-> (internally) C++ short (or maybe int ?) <-> SQL bool (or equivalent)

bool value1 = true;	// or a model with a bool member
bool value2{}; 		// ...

connection << "DELETE FROM AAA WHERE value1 = :value1 RETURNING value2", use(value1), into(value2);

However, using a type_conversion specialization for this purpose might not be the most appropriate approach after all (I'll try to integrate bool support directly into SOCI, like other primitive types already are).

[vadz]

I think we need to draw a line somewhere and not include all standard library headers from here. I was already not sure about std::optional but was convinced to add it here because boost::optional had been already included, but pulling in std::fs into all files using SOCI really doesn't seem right.

Maybe we could have an opt-in way to do it, but the simplest would be to just provide the header and document that it should be included explicitly.

[e1y4s]

Yes, I understand. In that case, maybe I should only include the headers for smart pointers? Because even though both smart pointers and std::filesystem::path are "convenient objects", std::filesystem::path seems to me more optional and project-specific than smart pointers.

[vadz]

We're not using C++98 any more.

Suggested change

	struct type_conversion<std::shared_ptr<T> >
	struct type_conversion<std::shared_ptr<T>>

[e1y4s]

You're welcome!

I personally use pointer specializations when the presence or absence of a value can be ambiguous in the context of a "NOT NULL" field. For example, with std::string, this allows me to differentiate between a field left as null and a field set to empty.

I think the specialization for std::filesystem::path is useful when the data stored is clearly a path. This allows for clarification of the data type in the model and enables direct use of the std::filesystem::path API, without having to repeatedly perform conversions between std::filesystem::path and std::string.

Yes, using type_conversion seemed simpler to me at first, but I also agree that it should be handled "natively" without being a kind of "plugin". I will look into changing this as soon as possible.

Thanks for the PR!

I think pointer specializations are useful, although I must admit I had somehow never have any need for them personally, and std::fs::path one might be as well, although it's not totally clear how much advantage it brings compared to just using strings.

But the bool one is probably -- and unless I'm missing something -- not 100% correct and I think we should add "true" boolean support instead.