[sr-dev] RFC: database unsigned number types

[Henning Westerholt]

Am 05.07.2018 um 14:43 schrieb Daniel-Constantin Mierla:
> Hello,

Hello Daniel,
> [..]
> So far, the database API supported only signed types for numbers,
> respectively DB1_INT (int in c) and DB1_BIGINT (long long in c).
> However, many database table columns are defined as UNSIGNED INT or
> UNSIGNED BIGINT. The database connector modules are mapping UNSIGED
> values retrieved from database over signed fields in DB API structure.
> The other way around is done as well: signed values are the inserted in
> the unsigned columns.
>
> There is no issue if the value in C is a positive number, however, if
> the value goes over MAX_UINT/2 (over 2147483647), it becomes negative
> and inserting the value in database results in an exception and 0 being
> stored instead.
>
> So far, I guess the issue was rarely exposed, if at all, because no
> report on it, even these data types for DB1 are since the SER project
> was started in 2001. As I looked at database definition schema, most of
> unsigned columns are for internal flags or ids (e.g., lcr_id), where I
> guess no large values were used or needed so far.
Good summary. I wondered also about it a long time ago, I think for the 
same reasons it was not yet exposed.
> However, it can bite at any time and needs to be addressed. So far, two
> solutions come in mind:
>
> 1) drop using UNSIGNED INT for those db columns, use only INT and do
> unsigned cast in the C code when reading and cast to int when writing.
>
> 2) update the db connector modules to support unsigned types -- I added
> support for them in DB API, but each db_* module has to be updated. The
> also each module that uses UNSIGNED DB columns must be updated
>
> 1) should be simpler, 2) more work but better in long term
>
> Any other ideas? Which of the options you prefer to go for
I also think that option 2) is better, in the end our internal (Kamailio 
"v1") DB API should provide a good coverage for the actual types that 
the respective databases offer. Its also a legacy (from SER times) that 
we don't need to keep. This change would be also much less intrusive for 
the upgrade path for our users, as no database schema changes are necessary.

  As you already mentioned, many modules use this columns for flags or 
IDs, so a conversion should be straight-forward in most cases.
> Not to forget: somehow related, probably we have to switch from int to
> long for PV number values, otherwise timestamp variables can go negative
> once unix timestamp approaches MAX_UINT/2 (still plenty of time, but
> should not be delayed for long ...).
Good idea as well. We should introduce this change at the start of the 
next development cycle, if this introduces a lot of changes in our 
module code base.

Best regards,

Henning