Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

row::get<int64_t>() overflows 32bit integer in internal representation (SQLite) #1190

Open
Krzmbrzl opened this issue Jan 7, 2025 · 3 comments

Comments

@Krzmbrzl
Copy link
Contributor

Krzmbrzl commented Jan 7, 2025

This is a continuation of #1185. The MWE remains the same:

#include <soci/soci.h>
#include <soci/sqlite3/soci-sqlite3.h>
#include <iostream>

int main() {
    try {
        // Create an in-memory SQLite database session
        soci::session sql(soci::sqlite3, ":memory:");
        sql << "CREATE TABLE test_table (id INTEGER PRIMARY KEY, myInt INTEGER);";
        sql << "INSERT INTO test_table (myInt) VALUES (9223372036854775807);";

        // This works
        int64_t retrievedValue1;
        sql << "SELECT myInt FROM test_table WHERE id = 1", soci::into(retrievedValue1);
        std::cout << "Implicit retrieval: " << retrievedValue1 << std::endl;

        // This throws std::bad_cast (windows) or overflows int32 for a -1 (linux)
        soci::row r;
        sql << "SELECT myInt FROM test_table WHERE id = 1", soci::into(r);
        int64_t retrievedValue2 = r.get<int64_t>("myInt");
        std::cout << "Explict 64-bit retrieval: " << retrievedValue2 << std::endl;

    } catch (const std::exception& e) {
        std::cerr << "Error: " << e.what() << std::endl;
    }

    return 0;
}

On current master this compiles fine, but the get function of the row object will return -1 (on Linux), which seems to suggest that internally SOCI is using a 32bit integer representation. However, this seems wrong as clearly SQLite can support 64bit integer values for INTEGER columns.
This is likely related to SQLite not really supporting data types. The docs describe the INTEGER type as

INTEGER. The value is a signed integer, stored in 0, 1, 2, 3, 4, 6, or 8 bytes depending on the magnitude of the value.

@Sildra
Copy link
Contributor

Sildra commented Jan 7, 2025

Duplicate of #1185.
This can be fixed by replacing the type INTEGER to bigint in the CREATE statement line 9 as specified in the doc : https://soci.sourceforge.net/doc/master/backends/sqlite3/

@Krzmbrzl
Copy link
Contributor Author

Krzmbrzl commented Jan 7, 2025

Duplicate of #1185.

I quote from the beginning of my post:

This is a continuation of #1185

This can be fixed by replacing the type INTEGER to bigint in the CREATE statement

I know, but that's a workaround for this specific bug in SOCI. The correct thing for SOCI to do would be to always use int64_t for SQLite

@Krzmbrzl
Copy link
Contributor Author

Krzmbrzl commented Jan 7, 2025

The fix would be more or less trivial: change the respective mappings in

// integer types
m["tinyint"] = db_int8;
m["unsignedtinyint"] = db_uint8;
m["smallint"] = db_int16;
m["int2"] = db_int16;
m["unsignedsmallint"] = db_uint16;
m["boolean"] = db_int32;
m["int"] = db_int32;
m["integer"] = db_int32;
m["mediumint"] = db_int32;
m["int4"] = db_int32;
m["unsignedint"] = db_uint32;
m["bigint"] = db_int64;
m["int8"] = db_int64;
m["unsignedbigint"] = db_uint64;

However, the issue will be that this would break backwards compatibility (in the worst possible way as old code that used to work would now get a bad_cast exception unless they switch to querying SQLite integer types as int64_t everywhere).

It even gets wilder - again from the docs:

When text data is inserted into a NUMERIC column, the storage class of the text is converted to INTEGER or REAL (in order of preference) if the text is a well-formed integer or real literal, respectively. If the TEXT value is a well-formed integer literal that is too large to fit in a 64-bit signed integer, it is converted to REAL.

So I guess the SQLite type-issue is just fundamentally unsolvable.

However, in order to not have silent overflows in the backend wreck havoc on expected results, I would propose always fetching into a int64_t variable internally and then do a runtime-check whether the given value will still fit in whatever integer type SOCI has chosen in its representation. If it does, perform the conversion and restore current behavior. If not, throw an exception (instead of silently overflowing).

@vadz what's your opinion on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants