Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.Sign up
[dev.icinga.com #3408] Unsafe handling of dbi_conn_sequence_last #1153
This issue has been migrated from Redmine: https://dev.icinga.com/issues/3408
Created by abraxas on 2012-10-29 02:22:11 +00:00
In various functions dbi_conn_sequence_last is called to retrieve SQL ids generated by the Database backend. This is safe if called after an INSERT statement. However in the following functions dbi_conn_sequence_last is called after an insert_or_update function call.
The object id returned will be 0 for mysql UPDATE and the highest sequence of the connection and table for postgresql UPDATE. In both cases the reported id will be wrong and not safe for further use potentially resulting in an inconsistend database.
Due to the nature of definitions and configfilevariables (both usually cleared during icinga startup) this error is of a potential nature, however I observed this issue in notification_data.
A fix would be to set a flag in _insert_or_update functions whether UPDATE or INSERT has been called, that could be checked before calling dbi_conn_sequence_last. In case of INSERT the call is safe, in case of UPDATE the row-id of the entry needs to be retrieved by a separate SELECT statement.
2012-10-31 23:33:14 +00:00 by mfriedrich 90c7677
2012-10-31 23:57:28 +00:00 by mfriedrich 9b4b848
2012-11-01 00:32:48 +00:00 by mfriedrich e323e4e
2012-11-01 10:32:44 +00:00 by mfriedrich 9097281
2012-11-01 10:56:01 +00:00 by mfriedrich 2376fc2
2012-11-01 11:12:05 +00:00 by mfriedrich 6dc1239
2012-11-01 11:27:43 +00:00 by mfriedrich 90ef902
2012-11-01 11:55:38 +00:00 by mfriedrich 3dae1a8
2012-11-01 12:10:34 +00:00 by mfriedrich d8b7009
2012-11-01 12:18:58 +00:00 by mfriedrich 06a8ae3
2012-11-01 12:33:46 +00:00 by mfriedrich 186347f
2012-11-01 12:45:02 +00:00 by mfriedrich 3673c1c
2012-11-01 12:55:13 +00:00 by mfriedrich c6031ae
2012-11-01 13:07:33 +00:00 by mfriedrich 5344f68
2012-11-01 13:33:01 +00:00 by mfriedrich f3df924
2012-11-01 13:43:13 +00:00 by mfriedrich 7a23cdc
Updated by mfriedrich on 2012-10-29 18:06:12 +00:00
hmmmm that would explain a lot, though i am not sure if #3326 is the same origin/cause. thanks for the thoughts though, i wasn't aware of the fact that libdbi behaves badly on such a change. (bad enough that i had to change all queries thanks for mysql itsself).
Updated by mfriedrich on 2012-10-30 21:22:26 +00:00
another proposal would be to let the insert_or_update query figure the "last insert id" to be determined by itsself, passing as reference parameters (int or unsignedlong). since the select is already there, but as safety select for the insert, and not after the update itsself for getting the last id.
though, all functions need to be analyzed which ids are to be fetched in dbhandlers for these query calls.
and, to note, if moved inside the insert_or_update functions, the final free call on the dbi_result must be put inside the functions, after the last query, removing it from dbhandlers, to avoid confusion.
Updated by mfriedrich on 2012-10-31 22:45:42 +00:00
Updated by mfriedrich on 2012-10-31 23:32:00 +00:00
ok, the general approach is now
within the function, do all the last insert id / sequence stuff yourself.
all the insert id get foo is deleted from within dbhandlers.c function calls, also the dbi_free stuff is entirely moved into the insert_or_update functions.
example from a pgsql insert query on notifications, rewritten
Updated by mfriedrich on 2012-11-01 00:32:14 +00:00
for debug reasons, always hitting the update statements when testing, set clean_config_tables_on_core_startup=0 in ido2db.cfg
tested with ido2db_query_insert_or_update_configfilevariables_add still pgsql
Updated by mfriedrich on 2012-11-01 00:35:51 +00:00
still missing for the next days (too tired now). example is implemented above.
Updated by mfriedrich on 2012-11-01 11:55:06 +00:00
Updated by mfriedrich on 2012-11-01 12:34:23 +00:00
Updated by mfriedrich on 2012-11-01 13:06:28 +00:00
Updated by mfriedrich on 2012-11-01 14:00:25 +00:00
so, executive summary
from my tests on pgsql with existing config in it, this works.
Updated by mfriedrich on 2012-11-01 14:22:29 +00:00
Updated by mfriedrich on 2012-11-10 15:05:02 +00:00
in my dev environment, this runs like a charm, though, it slightly decreases performance due to the added select query after running an update. this only happens on startup, as those "last insert ids" are just for relating among config data, so it does not really touch the overall performance during runtime of ido2db.