Allow to row::get blob into suitable containers #1189
Conversation
"Suitable" means that the container uses contiguous storage in memory and holds entries of type T where sizeof(T) == 1. If this is given, then the blob's data can be copied into a new instance of such a container, which is then returned in the row::get<>() call. This restores the previous possibility of reading blobs into objects of type std::string but also generalizes the concept to any suitable container (such as std::vector<unsigned char>). Note: The check for whether the given container object uses contiguous object storage is based on its iterators being classified as random-access iterators. This is not a perfect check as in principle there could be random-access iterators that don't iterate a contiguous memory region. However, for typical containers this is very unexpected as this would typically lead to rather inefficient access patterns when making use of the random-access capability. A container that is wrongly determined to be contiguous will likely result in a program crash due to a segfault. Fixes SOCI#1173
There was a problem hiding this comment.
Thanks for fixing this but is checking for arbitrary random access iterators really needed? I'd say that string and vector are the only containers that you may ever want to read BLOBs into but if we want to be more generic, we could just specialize is_contiguous_byte_container for them by default but document that it be specialized for custom types too: it seems like it would be simpler and safer.
It would be safer, yes, but also much more hassle (compared to "it just works") to get working. What I have in mind is someone using e.g. Boost containers. The current implementation relies on any container type being passed in to be defined sensibly, that is in analogy with the standard containers. I think as long as this is true, then the assumptions in this PR hold (including the implicitly assumed availability member functions If you know of even a single commonly-ish used container that would pass my heuristic without having contiguous storage in memory, then we obviously can't use this. However, I was unable to come up with one 👀 |
|
I think any legacy custom container wouldn't have I'd really like to keep the code as simple as possible while still avoiding regressions (such as #1173) and just allowing reading a BLOB into a string would be good enough for me. Allowing reading it into a vector is even more enough, I'm not even sure we need a trait at all — but I'm ready to have it if you think it's worth it, just let's maximally simplify it. OTOH I thought of something else: why don't we specialize |
|
I'll add a trait and a default implementation for an accessor. This way, the code can be more or less fully customized if needed, but the logic on SOCI's side is minimal.
BLOBs are inherently non-copyable and I believe they always have been. Making Blobs copyable might be possible but I think this would involve a significant amount of work as different backends have different semantics of what a Blob actually represents and how they have to be accessed. At the very least the Blob handle (where present) would have to have shared_ptr like semantic to ensure that we don't have double-frees. However, copying Blobs would then only copy the SOCI Blob instance. Yet both instances would still point to the same "physical" Blob in the DBs. That's because for most backends, SOCI Blobs have more of a pointer-like semantic. Yet, this is not true for all backends (e.g. MySQL). So all in all I believe that the Blob backends are just too different to support this with a consistent API and with consistent semantics. |
Krzmbrzl
force-pushed
the
fix-blob-get-as-string
branch
from
fb1fa42 to
f2e60f3
Compare
"Suitable" means that the
soci::is_contiguous_resizable_containeris specialized to have itsvaluemember yieldtrue. This specialization is provided forstd::stringandstd::vector<T>wheresizeof(T) == sizeof(char).Fixes #1173