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