19 Mar
2017
19 Mar
'17
12:50 a.m.
Why is insert_update doing nothing in Kamailio db insert_update ?
As explained in the following article the design of "UPSERT" in
PostgreSQL requires to be explicit about the constraint on which
we accept to do update modification to Kamailio database framework/API
would be required to expose which specific key constraint should be
handled as an update
http://pgeoghegan.blogspot.com/2015/10/avoid-naming-constraint-directly-whe…
In this first step, DO NOTHING is providing the simple feature to ignore
error on insert when facing a constraint violation
You can view, comment on, or merge this pull request online at:
https://github.com/kamailio/kamailio/pull/1039
-- Commit Summary --
* db_postgress: insert_update() with DO NOTHING
-- File Changes --
M src/modules/db_postgres/db_postgres.c (1)
M src/modules/db_postgres/db_postgres.h (2)
M src/modules/db_postgres/km_dbase.c (83)
M src/modules/db_postgres/km_dbase.h (5)
M src/modules/db_postgres/km_pg_con.c (4)
M src/modules/db_postgres/pg_mod.c (4)
-- Patch Links --
https://github.com/kamailio/kamailio/pull/1039.patch
https://github.com/kamailio/kamailio/pull/1039.diff
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039
19 Mar
19 Mar
4:56 p.m.
Is this patch useful in existing code base? If the function doesn't exist in the db
connector module, it should be not used by the modules, but do an alternative solution.
Can you give some example where this is useful now?
Anyhow, as I could understand from the patch, it doesn't do an update on an insert
conflict. This can mislead, by seeing the function exported by postgres module, but not
fully implemented.
Regarding the commit message, it must be strictly related to what the patch adds/changes
to Kamailio sources, not the story of doing the patch. Such details can be in the pull
requests description or comments of PR.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039#issuecomment-287618378
20 Mar
20 Mar
10:10 p.m.
The immediate value is the have a way to distinguish and ignore insert failures when they
are caused by by a constraint violation.
The first use case would be in usrloc when we write to db and use dmq replication.
I would add a param to usrloc module "db_insert_update" defaulting to 0 but that
could be set to 1, shit would already work completely with mysql
However with postgres, it would remove any unwanted errors and retries when syncing from
dmq for example, I saw other posts about such errors.
This way usrloc would only retry it there is a real failure not a constraint violation.
Then we could plan for a second step to implement the "DO UPDATE" instead of
just "DO NOTHING"
- add an optional param to insert_update (modify the db framework slightly) to specify the
name of the constraint where we want to update on failure
- add a db_postgres module variables that could contain tablename_constraint and check for
them when doing insert update
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039#issuecomment-287866358
21 Mar
21 Mar
2:37 a.m.
The most strati forward solution would probably be changing the function prototype :
```/**
* \brief Insert a row into specified table, update on duplicate key.
*
* The function implements the INSERT ON DUPLICATE KEY UPDATE SQL directive.
* It is possible to insert a row and update if one already exists.
* The old row will not deleted before the insertion of the new data.
* \param _h structure representing database connection
* \param _k key names
* \param _v values of the keys
* \param _n number of key=value pairs
* \return returns 0 if everything is OK, otherwise returns value < 0
*/
typedef int (*db_insert_update_f) (const db1_con_t* _h, const db_key_t* _k,
const db_val_t* _v, const int _n);
```
To
```/**
* \brief Insert a row into specified table, update on duplicate key.
*
* The function implements the INSERT ON DUPLICATE KEY UPDATE SQL directive.
* It is possible to insert a row and update if one already exists.
* The old row will not deleted before the insertion of the new data.
* \param _h structure representing database connection
* \param _k key names
* \param _v values of the keys
* \param _n number of key=value pairs
* \param _v values of the keys_constrains
* \return returns 0 if everything is OK, otherwise returns value < 0
*/
typedef int (*db_insert_update_f) (const db1_con_t* _h, const db_key_t* _k,
const db_val_t* _v, const int _n, const db_key_t* _kc);
```
Then a db driver that does not need them would simply ignore them, I was not sure bout
this option because postgres may be the only one requiring explicit constraints
specification ?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039#issuecomment-287931666
3:09 a.m.
This would require modifications to
```
~/git/kamailio/src$ grep -R --include="*.c" insert_update *
the DB base framework and 4 implementations : db_mysql, db_cassandra, db_mysql,
db_cluster
lib/srdb1/db.c: if (dbf->insert_update) {
lib/srdb1/db.c: dbf.insert_update = (db_insert_update_f)find_mod_export(tmp,
lib/srdb1/db.c: "db_insert_update", 2, 0);
modules/db_postgres/km_dbase.c: * Why is insert_update doing nothing in Kamailio db
insert_update ?
modules/db_postgres/km_dbase.c:int db_postgres_insert_update(const db1_con_t* _h, const
db_key_t* _k, const db_val_t* _v,
modules/db_postgres/km_dbase.c: LM_ERR("error while preparing insert_update
operation\n");
modules/db_postgres/db_postgres.c: dbb->insert_update = db_postgres_insert_update;
modules/db_postgres/km_pg_con.c: LM_WARN("server version < 9.5 does not support
insert_update\n");
modules/db_cassandra/db_cassandra.c: dbb->insert_update = db_cassa_insert;
modules/db_mysql/km_dbase.c: int db_mysql_insert_update(const db1_con_t* _h, const
db_key_t* _k, const db_val_t* _v,
modules/db_mysql/km_dbase.c: LM_ERR("error while preparing insert_update
operation\n");
modules/db_mysql/km_db_mysql.c: dbb->insert_update = db_mysql_insert_update;
modules/db_cluster/db_cluster_mod.c: dbb->insert_update = db_cluster_insert_update;
modules/db_cluster/dbcl_api.c:int db_cluster_insert_update(const db1_con_t* _h, const
db_key_t* _k, const db_val_t* _v,
modules/db_cluster/dbcl_api.c: DBCL_WRITE(insert_update, insert_update(dbh, _k, _v, _n));
as well as the clients :
modules/p_usrloc/ul_db_ins_upd.c:int db_insert_update(ul_db_handle_t * handle,
modules/p_usrloc/ul_db.c:int ul_db_insert_update(str * table, str * first, str * second,
modules/p_usrloc/ul_db.c: return db_insert_update(handle, table, _k, _v, _n);
modules/p_usrloc/ul_db_form_query.c: if(dbf->insert_update(dbh, _k, _v, _n ) < 0)
{
modules/ims_usrloc_scscf/usrloc_db.c: if (ul_dbf.insert_update(ul_dbh, key, val, i) != 0)
{
modules/ims_usrloc_scscf/usrloc_db.c: if (ul_dbf.insert_update(ul_dbh, key, val, 7) != 0)
{
modules/ims_usrloc_scscf/usrloc_db.c: if (ul_dbf.insert_update(ul_dbh, key, val, col_num)
!= 0) {
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039#issuecomment-287936993
6:32 a.m.
There is another option to do it directly in db_postgres constraints "auto
detection"
https://www.postgresql.org/docs/9.1/static/catalog-pg-constraint.html
We would search the catalog :
```
select conntype from pg_constraint where conrelid = (select oid from pg_class where
relname like 'location_test') and contype = u;
select conntype from pg_constraint where conrelid = (select oid from pg_class where
relname like 'location_test') and contype = p;
```
wich would return : `location_ruid_idx_test` or `location_test_pkey`
Then we can save this in the module memory :
```struct constraint {
char *db_name;
char *db_table;
char *unique_contraint;
char *primary_key_constraint;
};
```
And use it automatically in the next queries ...
```insert into location_test (ruid, username, domain) values ('1234','jo',
'my dom') on conflict on constraint location_ruid_idx_test do update set
ruid='1234', username='jo', domain='my dom';
```
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039#issuecomment-287967115
22 Mar
22 Mar
9:24 p.m.
I am working on OPTION2, where not modification is required outside of db_postgres, if you
think this is not an option or not valuable enough let me know ?
If you do not want to merge DO NOTHING because it can be confusing / misleading then we
can clause this PR.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039#issuecomment-288493936
23 Mar
23 Mar
4:23 p.m.
Traveling during the past week, didn't have time to follow up on this. You can close
it if you plan another patch.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039#issuecomment-288717300
5:23 p.m.
Closed #1039.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/pull/1039#event-1012500837
2802
days inactive
2807
days old
8 comments
2 participants
participants (2)
-
Daniel-Constantin Mierla -
Julien Chavanton