SOCI / soci

Official repository of the SOCI - The C++ Database Access Library
http://soci.sourceforge.net/
Boost Software License 1.0
1.37k stars 472 forks source link

SQLite incorrect DDL handling #1085

Closed Sildra closed 2 months ago

Sildra commented 9 months ago

Mapping all of the types to integer will result in type narrowing while retrieving all of the data. Two solutions here: either integer is mapped to int64 or we distinguish types in the create_column.

In the second case, integer primary key also needs to be handled separately.

_Originally posted by @Sildra in https://github.com/SOCI/soci/pull/954#discussion_r1355037313_

This issue also contradicts the documentation : https://github.com/SOCI/soci/blob/master/docs/backends/sqlite3.md#dynamic-binding

Sildra commented 8 months ago

Proposal for fixing the DDL creation of SQLite : https://github.com/Sildra/soci/commit/087f3821a06d795efdecad9b5b4104fd3ff3ad40

The following commit has a perfect matching between DDL creation and SQLite.

vadz commented 8 months ago

I admit I don't understand what does the linked commit change: according to SQLite docs all these types map to INTEGER affinity, so why does it change anything?

Sildra commented 8 months ago

Because SOCI is performing affinity check based on the description of the column and for SQLite it is based on the name of the type. In every test, columns are not created using the DDL sql.create_table("TEST").column("VAL", soci::dt_long_long) but using a raw query. The equivalent query is "CREATE TABLE TEST (VAL INTEGER)". The INTEGER part is wrong, as my expected type is long long (or "bigint" as defined in https://github.com/SOCI/soci/blob/2e4b44b9fabc65a2362d6421c78fab35b1552717/src/backends/sqlite3/statement.cpp#L412-L465).

Then when you get the data with the get, as a developer, I am expected to do v.get<long long>("VAL") but the following throws a std::bad_cast().

The current correct way to retrieve the data is v.get<int>("VAL") but it only supports 32bit values.

With the change, it is possible to do v.get<long long>("VAL") but it will break v.get<int>("VAL"). So for both to work it will require https://github.com/Sildra/soci/commit/10d2f776b2e7ab6a23a3e68d9884848e9fcef550.

Sildra commented 2 months ago

Resolved by #1120