row::to should support optional<string_view>

[alexolog]

template<typename Tuple> void row::to(Tuple &t) const supports std::optional<std::string> as a tuple type, but not std::optional<std::string_view>. Please fix.

[jtv]

Thanks for finding that. I think I'll have to look into some ways to reduce the redundancy between those places in the code.

[jtv]

@alexolog I'm slowly coming around to the view that perhaps libpqxx should just allow you to convert a field to, say, a std::string_view and leave it as your problem to make sure the buffer holding the data is still valid when you access it.

That would simplify the code inside libpqxx, and allow applications a bit more freedom to optimise their memory management. The cost: more responsibility to be aware of the data's lifetimes.

Thoughts?

[alexolog]

As long as pqxx::row holds the result internally

[jtv]

As long as pqxx::row holds the result internally

It's pqxx::result that holds the data, and pqxx::row acts as a reference to that, but yes. In fact pqxx::result is basically a std::shared_ptr so you can copy it around and it will still refer to the same buffer. I'm fairly sure I documented it somewhere, but I'd have to make that clearer.

[alexolog]

I assume that pqxx::row holds a shared_ptr copy to the corresponding pqxx::result, so I can safely do something like (please ignore the syntax):

auto f() {
  return query.exec()[0];
}

[jtv]

Yes and no, respectively.

Yes, the pqxx::row is a reference to the pqxx::result which in turn is basically a std::shared_ptr to the data.

But in this return example, the pqxx::result itself goes out of scope when you leave the function! And there's no other pqxx::result pointing to the same buffer, so that's the end of the buffer's lifetime.

[alexolog]

So pqxx::row does not actually hold an instance of the shared pointer itself (otherwise the memory would stick around) and is just a simple reference to an inter data structure.

I haven't checked the documentation very closely, but it would be a good idea to warn about such usage because it looks like the row is returned by value.

[github-actions]

There has been no activity on this ticket. Consider closing it.

[github-actions]

There has been no activity on this ticket. Consider closing it.

[github-actions]

There has been no activity on this ticket. Consider closing it.

[jtv]

I do still intend to fix this one... But it's a pretty radical change to allow general conversion to string_view, so probably best for 8.0.

[github-actions]

There has been no activity on this ticket. Consider closing it.

[github-actions]

There has been no activity on this ticket. Consider closing it.

[github-actions]

There has been no activity on this ticket. Consider closing it.

[jlittlenz]

I just tripped on something related to this. pqxx::transaction_base::query<> allows std::string_view, but not std::optionalstd::string_view.

[jtv]

@jlittlenz converting to string_view is a bit awkward in the general case, for lifetime reasons: it's relatively easy to get yourself into a situation where the underlying string data gets deallocated before you're done reading it. That's why so far it's only been supported in a few select cases.

However in libpqxx 8.0 I'm thinking to make this the caller's problem. You would be able to "convert" any string to a string_view, but you would be responsible for ensuring that you don't access the string beyond its lifetime. That does mean that I'll have to document lifetime information in a lot of places.

[jlittlenz]

Thank you. My "trip" was partly due to my expectations of the field::view method; null values were tripping everything, but not field::view. It just gives an empty string, and one can use field::is_null if you care about it.

[jtv]

Right, @jlittlenz. Generally you should check for nullness before trying to read a value.

Although I did just notice that nowadays you are guaranteed to get an empty string when the value is null. That was not the case when I first wrote libpqxx, but it is the case in all supported versions now. So it may be time to review the design for that. Converting a null to any type is unsupported, but for some types it may be reasonable to accept an empty string as a null. Think std::optional<int> for instance.

[jtv]

For future reference: I just checked and the documentation for field::as() does say that it will throw an exception if the field is null. And I think conversion of a string_view to string_view already works.

[github-actions]

There has been no activity on this ticket. Consider closing it.

[jtv]

Yes, this has been on the shelf for a long time... Life has distracted me to some degree.