Handling of ::sqlpp::tag::enforce_null_result_treatment does not seem to be implemented, NULL documentation is erroneous

[margaretselzer]

I am using sqlpp11 with sqlite3.

I wanted to enable the behavior of sqlpp11 throwing an sqlpp::exception when a null value is being assigned to a variable. E.g. I would expect the below test to pass:

    auto s2 = select(t.id, t.text, t.nulltext).from(t).where(t.nulltext.is_null());
    auto result = db(s2);

    ASSERT_FALSE(result.empty());
    ASSERT_TRUE(result.front().nulltext.is_null());
    std::string text;
    ASSERT_THROW(text = result.front().nulltext.value(), sqlpp::exception);

but in fact it fails with ASSERT_THROW.

From the documentation one could assume that the presence of the tag enforce_null_result_treatment with the connection and/or with the column influences this behavior. Now it appears that whether this tag is present or not at the column, it does not make the slightest difference, the null values are always converted to the trivial value of the column type.

Examining the code the tag does not appear anywhere else than the place it is defined in type-traits.h. and in connector_api/connection.h. I could not quite figure out in general how the _traits are used and have their effect. I would be grateful for a pointer on this to help me grasp the concept.

The documentation of NULL handling seems to be obsolete and is ambiguous/erroneous. For example the usage of tvin() mentioned there is surely dead based on this issue. And when it comes to the usage of enforce_null_result_treatment I could not really decide whether the tag being present makes(should make) the code throw an exception or the lack of that tag. Based on the tag name alone I guess that having the tag should cause conversion to trivial value and not having it should cause throwing exceptions, but this is just a guess.

I would gladly offer my assistance in form of a PR, but having spent only 2 days with sqlpp at this point I do not feel the confidence to add more than this humble notice.

Other than this, the library is great, thank you all who contributed and/or still do!

[rbock]

Thank you for the detailed report! And sorry for the mess in the documentation. As you already guessed, there is some history to it.

• tvin is removed. It was used in only one project, afaict. Hard to maintain and surprising edge cases.
• enforce_null_result_treatment is removed except for the places you found. It was never used, afaict. There were two options: per column and per connection.

If you really want an exception to the thrown then accessing the value of a result field that is NULL, the per-column configuration could be re-introduced with relative ease, I think. I would suggest a parameter for the code generator that adds the trait, and then checking for the trait in the in result_field_base.

However, in C++17 or later, I would definitely use std::optional to represent result fields. I am considering to have a simplified version as sqlpp::optional for older C++ versions. It would be a pretty radical change for anyone relying on the NULL is returned as 0 behavior, of course.

WDYT?

Thanks,
Roland

PS: All that said, I started updating the documentation to cover the current behavior.

[margaretselzer]

Hi Roland, thanks for the quick reply and action on the documentation, really appreciate it!

A couple of thoughts:

1. I think in 2024 you can expect/demand that anybody using sqlpp11 has at least C++17 support. At least if they want to use the latest and greatest version of sqlpp11.
2. Relying on NULL being represented with its trivial value without actually checking whether the field is NULL I would consider a lazy coder's programming error, so do not worry about that.
3. About using optional. I did not dig deep enough in the code to understand how _cpp_value_type gets its concrete type, but using here optional for nullable fields definitely feels a good way to go about things. You could then state as a generic rule that for non-nullable fields there is always a conversion and they never throw while nullable fields always return an std::optional when being assigned to an auto result. Using the conversion operator for nullables then would throw the same way as calling value() where for nullable fields the value() or value_or() method would be just a wrapper to std::optinal::value() or std::optional::value_or(). The value_or() field method could default to returning the type trivial value. This way one could choose whether one wants a trivial value, some other value, an exception thrown or wants to check whether there is a value or not by calling std::optional::has_value() on the assigned result or relying on result_field_base::is_null() before assignment.

I hope what I wrote is relatable to how sqlpp is thought to work. If not, then please forgive my ignorance.

As it feels from your message and from other threads that I read on this topic that you have pretty much made up your mind to go with optional - you just needed a final push? - I would appreciate if you could let me know when this will be available. If you need help with this, I am happy to assist, but then I need a crash course on the related parts of sqlpp architecture before I dare to make changes to it.

[rbock]

Thanks for your thoughts, this is much appreciated.

You are right. I am convinced that using std::optional is the right path forward for representing nullable columns and values. sqlpp17 serves as a proof of concept. But I am not sure if it will ever be ready for production and I am not super keen on maintaining two libraries.

Switching to sqlpp11 to C++17 and refactoring it step by step might therefore be an option. But I wonder how to deal with bug fixes for the C++11 code (there still are folks out there who use C++11)?

[rbock]

If you want to get your hands dirty and play with code, try out

https://github.com/rbock/sqlpp11/tree/optional

From the commit message:

Mini POC to get started with transition to using optional for result fields

Very hacky. All fields are considered optional
At this point, only tests/sqlite3/usage/Integral.cpp compiles and runs.

See also sqlpp17 for inspiration.

[rbock]

Continued a bit. All result types except blob should work now for sqlite3.

text parameters are currently not working.

I am thinking of backporting minimal versions of optional, string_view, and span into the library for versions the don't support them (type aliased if supported).

[margaretselzer]

I would suggest to drop the 11 from sqlpp11and just call it sqlpp. Add optional, string_view and span to the next release of the lib and just require minimum C++ version of C++17. That would give you a lot of room to maneuver with how you handle the accumulated fixes and patches of sqlpp11 and at what point in time you let go of it. Then if you believe that a rewrite from scratch is warranted on C++20/23 you can release it whenever you are done as the next - not backward compatible - release of sqlpp. If you want to differentiate it in name as well I would suggest sqlpp2 to signify that it is a new major iteration of the library. This approach would liberate you from being bound to a specific version of C++.
If you are keen on backward compatibility then yes, backporting a minimalistic version of those std classes with an alias sounds like a good way to approach it, but I would mark all these minimalistic versions with [[deprecated "Supporting an ancient C++ version. Use C++17 or later."]] and drop these in a subsequent release.

[MeanSquaredError]

@rbock

I am thinking of backporting minimal versions of optional, string_view, and span into the library for versions the don't support them (type aliased if supported).

+1 to that.
Right now the lack of support for std::optional or something compatible is one of the main shortcomings of the otherwise great library..

Btw Sy Brand has implemented tl::optional which is compatible with std::optional and works for C++11/14/17 on all major compilers.

So maybe it makes sense to use directly his implementation which is a single .hpp file, 72 KB in size.

I am not sure whether it is better to add his project as a dependency or just copy his source code into sqlpp. Or maybe just use his implementation and simplify it a bit (he has made a few extensions to std::optional, which we don't really have to copy).

[MeanSquaredError]

@margaretselzer

I would suggest to drop the 11 from sqlpp11and just call it sqlpp. Add optional, string_view and span to the next release of the lib and just require minimum C++ version of C++17. That would give you a lot of room to maneuver with how you handle the accumulated fixes and patches of sqlpp11 and at what point in time you let go of it.

If I remember correctly rbock wants to keep compatibility with sqlpp11 in the current codebase and then just make a full rewrite of sqlpp once C++ supports reflection, which will probably be in C++26.

We discussed it a while ago and IIRC decided to add sqlpp::compat::optional, which on C++17 and newer will be an alias for std::optional and on C++11/14 it will be a custom, maybe simplified, implementation of std::optional.

I think it makes sense to use the same approach with std::string_view and the other similar classes that are missing from C++11/14.

[margaretselzer]

@MeanSquaredError

If I remember correctly rbock wants to keep compatibility with sqlpp11 in the current codebase and then just make a full rewrite of sqlpp once C++ supports reflection, which will probably be in C++26.

Well, I'm obviously oblivious to the history. Thanks for the background. Aliasing these constructs is then a good way to solve the compatibility issue. Is that all that's missing is that somebody does it?

Regarding backward compatibility though, I am not sure if it is worth the effort. If the ultimate goal is a complete rewrite with reflection then the result will be surely something very similar to the EF in .Net. I would assume that it will not necessarily be a straightforward thing to do so a new stable library is quite a few years away after reflection becomes available. Sqlpp11 and the new version will then also need to live side by side for for a couple of more years. Imagining this timeline are we going to still have C++11 compatibility in the library in 2035?

Following from the above, I think dropping the burden of backward compatibility and just considering the current sqlpp11 as "version 1" and the new - yet to be written with reflection - "version 2" would make things lot less complicated going forward. It may also make that future rewrite somewhat easier.

Anyway, I rest my case. On the end I am happy with any working solution. Sqlpp is a great library aliases or not. :-)

[MeanSquaredError]

Well, I'm obviously oblivious to the history. Thanks for the background. Aliasing these constructs is then a good way to solve the compatibility issue. Is that all that's missing is that somebody does it?

In order to add support for std::optional, the core code of sqlpp11 will need some changes. IIRC rbock mentioned that it might be possible to backport some of the code from https://github.com/rbock/sqlpp17, so indeed in a general sense all that's missing is that someone has to actually write/backport the code to sqlpp11

IMHO implementing the compatibility layer (sqlpp::compat::optional for c++11/14) is not that much work per se as and the more complex part will be to backport the changes from sqlpp17. But I might be wrong since we don't really know until someone implements it.

BTW we already have sqlpp::compat::make_unique and it seems to work well, so I think it doesn't hurt to use the same approach with the other pieces missing from C++11/14.

Regarding the hypothetical sqlpp26 (or whatever its name will be), I would expect that from a user-point of view it will have the same or very similar interface. I think that sqlpp11 has found the perfect balance between unchecked raw SQL queries and the rigid and limited ORM frameworkis and I don't think it can be improved significantly by making major changes in its interface. That is why I wouldn't expect any major changes from a user perspective. But then again I might be wrong here too.

[rbock]

Hi, Thanks for the discussion! These are pretty much the thoughts I am going through myself.

To sum it up:\

• Yes, I would like sqlpp11 to be compatible with C++11 for the foreseeable future.
• Using optional would be a breaking change, but would also eliminate quite some complexity in the library and make the API more user friendly.
• Once there is decent reflection & generation support, I want to start working on a new version that would also work on other datasources like ranges. I am not familiar with EF in .Net, so I won't comment on potential similarities. Same with LINQ in C# (that was mentioned long, long ago). You can see a rather clumsy version here: https://github.com/rbock/sqlpp11-connector-stl

Re effort:\

• Backporting the bare necessities of optional, string_view, and span would be simple, I think, as we really don't need a standard-compliant version.
• Backporting support for those types in results: With the branch mentioned above, you can already try it with sqlite3. That was easy, just a few hours of work, mostly for getting the tests to compile. If you would prefer experimenting with another backend, that can be done quickly, too, of course.
• Feedback about the usability and ease/difficulty of transition from the existing result API would be appreciated.
• Backporting support for optional and friends in assignment and and comparison: I have not looked into that, but it is probably not that hard either.

Cheers,
Roland