unexpected behavior in soci::row::get for unsigned and long types

[ricardofandrade]

Hey guys,

Is there any special reason to force all unsigned types to be long long while getting a value from a row?

I expected unsigned char, unsigned short to be an int and unsigned int to be long long.

Besides that, there's no type traits support for neither long nor unsigned long.

I saw how you select types for exchange and long is there, including the support for both 4-byte and 8-byte long variations.

Do you see any problem in changing/improving that?

Thanks, Ricardo

[mloskot]

Ricardo,

This is generally a known and recurring issue. You can see my thread What is status of integer types support? where I try to summarise current status linked to older threads that may give some background on decisions made in the past.

I agree with you that the current situation is inconvenient and may be confusing, and I'm going to improve it. I have started writing RFC1 on Complete support for C++ integer types where I intent to decide and document the improved implementation.

So, I don't see any problem with improving, on the contrary, I think it's important to get it right. I'm just not eager in accepting random patches without properly documenting it and covering all cases.

At the moment I'm busy preparing for 3.2.0 release, but I've planned to work on integers soon.

If you feel like you could help with discussing it and writing the RFC1, please feel invited.

p.s. Perhaps this issue could be renamed and become a task directly linked to the work on RFC1

[pfedor]

Ricardo: Which backend are you using? The MySQL works like you said: "I expected unsigned char, unsigned short to be an int and unsigned int to be long long." -- this is exactly how it behaves, I just added a test to confirm it in https://github.com/SOCI/soci/commit/7751cf4008bf83e53809cfef7a47c5bbec548da1

Thanks,

Aleksander

[ricardofandrade]

Sorry for the long delay, we decided to remove the unsigned types from our db and this become a minor issue. But I'm still interested in getting soci even better, so let me explain with more details.

Actually, it doesn't matter the backend used @pfedor. In the tests you are using int to retrieve both signed and unsigned versions of 'val'. However, there's a file named unsigned-types.h in soci which allows me to get the unsigned one as unsigned char. Unfortunately, it's mapping unsigned char to long long (through the typedef base_type) what causes a bad_cast exception for this case. The same applies to unsigned short. I think the only adjust needed here is changing the base_type to int.

[pfedor]

Sorry, I'm confused, I'm not sure I understand what you mean--could you show me a code snippet, demonstrating what you are trying to do? Like a little test which currently would fail but might be fixed by the code change you are proposing?

Thanks,

Aleksander

[ricardofandrade]

Consider the following - MySQL - table:

CREATE TABLE test (val TINYINT UNSIGNED);
INSERT INTO test VALUES(1);

And the following soci code:

soci::session sql(...);
soci::rowset<> rows(sql.prepare << "SELECT val FROM test");
for (auto it = rows.begin(); it != rows.end(); ++it)
{
    it->get<unsigned char>(0);
}

This code causes std::bad_cast. Modifying unsigned-types.h, lines 22 and 53 to int solves the issue.

[pfedor]

Got it, thanks. Sorry I didn't get your meaning previuosly, I was confused.

Yes we're all in a violent agreement that the code you posted should work. It doesn't work currently and you are right that after changing long long to int in that file it starts working (I had no idea.)

I don't know the purpose of core/unsigned-types.h and why it uses long long and not int, I also don't understand why it makes a difference. From a cursory look at the thread on soci-users around the time core/unsigned-types.h was added, it seems to have been added precisely to make code like yours work. I even started suspecting that maybe it used to work and something changed between now and then, but no, I just checked out code as of that commit https://github.com/SOCI/soci/commit/7472d9bc75738d633d7eaabd5b07496967fe595b and get<unsigned char>(...) didn't work then either.

Note also that it would also be nice to be able to get for a TINYINT column which currently also doesn't work.

Since I don't understand this code in core/ I'll leave it to Mateusz who is working on fixing the integral types thing for soci once and for all.

Thanks,

Aleksander

[mloskot]

Guys, the src/core/unsigned-types.h has been added by Maciej long time ago and I'm not aware of actual motivation. Either it was a simple improvement or it was part of bigger plan Maciej might have at that time. Thanks Aleksander for testing that state, I was wondering if it used to work, so your analysis is very helpful.

As I have mentioned, I'm going to try to solve Complete support for C++ integer types in SOCI 4.0.0, but I don't reserve the work for myself. I certainly I'd welcome any suggestions and help.

At the moment, we are releasing SOCI 3.2.0 (it should have been released already, but we're applying final touches, so it will be released this week). So, I think, we have to accept the current state with all its present deficiencies.

[mloskot]

Here is new report from Ricardo Muñoz with use case of boost::tuple and unsigned int that throws a bad::cast exception using soci::into. This is related to this bug report. Below, I copied Ricardo's test program (removed boost::optional uses, unnecessary for this problem):

// Test case for problem reported on soci-users
// http://thread.gmane.org/gmane.comp.db.soci.user/2546
#include <boost/tuple/tuple.hpp>
#include <iostream>
#include <exception>
#define SOCI_USE_BOOST
#include "soci.h"
#include "boost-tuple.h"
#include "soci-sqlite3.h"
typedef boost::tuple<unsigned int, std::string> row_type;

row_type get_last_row(soci::session& sql)
{
    unsigned int id = 0;
    std::string str1;
    sql << "SELECT id, str1 from example ORDER BY id DESC LIMIT 1",
        soci::into(id), soci::into(str1);
    return row_type(id, str1);
}

row_type get_last_row2(soci::session& sql)
{
    row_type myrow;
    try
    {
        sql << "SELECT id, str1 from example ORDER BY id DESC LIMIT 1",
            soci::into(myrow);
    }
    catch(const std::exception& e)
    {
        std::cout << "Exception: " << e.what() << std::endl;
    }
    return myrow;
}

int main()
{
    soci::session sql(*soci::factory_sqlite3(), ":memory:");

    sql << "CREATE TEMP TABLE example("
        " id INTEGER PRIMARY KEY AUTOINCREMENT,"
        " str1 TEXT)";

    sql << "INSERT INTO example(id, str1, str2)"
        " VALUES(NULL, datetime('now'), NULL)";

    // works
    row_type myrow = get_last_row(sql);
    std::cout << myrow.get<0>() << std::endl;
    std::cout << myrow.get<1>() << std::endl;

    //std::bad_cast
    row_type myrow2 = get_last_row2(sql);
    std::cout << myrow2.get<0>() << std::endl;
    std::cout << myrow2.get<1>() << std::endl;

    return 0;
}  

Unfortunately, I didn't have time to move the integer support forward. I'm afraid I won't have time to work on it until mid/end of June.

[mloskot]

Similar report has just been posted in bad_cast error (oracle backend)

[LazyBui]

I've hit this issue a few times since the switch to SOCI - it seems to me that since it's unlikely that all backends share the same size mappings, a real fix would be to include both the source type and destination type in the conversion structs, not just the destination type. In this way you could have your long long -> unsigned char just fine and we could simply add int -> unsigned char if needed (or it could be provided, either is fine).

The real problem is that you can only define one mapping to a specific type, not necessarily that the defaults are bunk. I believe I recall reading something to this effect a couple years ago in researching the issue originally - that there was a plan to eventually look at doing this.

I know from a limited experience that this is a painful addition and I'm not sure how it compares to the "integer support." However, it would also allow for some useful corner cases, such as defining a class that could be obtained from either a text or integer column (say, maybe, a CRC).

[ArnaudD-FR]

Hi, I also encountered this issue of bad cast when using row::get for example. At this time I didn't understood why we got this issue using soci::row::get<> and not when using into() or use().

In fact, row::get<> where not using the same mechanism than the other functions in soci.

You can check my commit in my branch develop2. Instead of using typedef typename type_conversion<T>::base_type base_type;

which totally bypass supported base types by the library, row::get<> now use: typedef typename details::exchange_traits<T>::base_type base_type;

https://github.com/ArnaudD-FR/soci/commit/1fb1d9cd5017d7317b924889de6adee897ba6202

[Cleroth]

Similarly, I'm not able to use unsigned long to insert data (I have to manually cast it to a unsigned long long).

[Cleroth]

Got excited when I saw 4.0 is released. Except... there is still no change in something as basic as this.