30 Oct
2013
30 Oct
'13
7:37 p.m.
Module: sip-router
Branch: master
Commit: 6fc84c2cf610791939ba73e38b8b5b3c0b5cd047
URL:
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6fc84c2…
Author: Juha Heinanen <jh(a)tutpro.com>
Committer: Juha Heinanen <jh(a)tutpro.com>
Date: Wed Oct 30 17:31:04 2013 +0200
modules/mtree: when loading data from db, load each tree separately
---
modules/mtree/mtree_mod.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/modules/mtree/mtree_mod.c b/modules/mtree/mtree_mod.c
index 0f4348c..eeec89c 100644
--- a/modules/mtree/mtree_mod.c
+++ b/modules/mtree/mtree_mod.c
@@ -291,6 +291,9 @@ static int mod_init(void)
while(pt!=NULL)
{
+ LM_DBG("loading from tree <%.*s>\n",
+ pt->tname.len, pt->tname.s);
+
/* loading all information from database */
if(mt_load_db(&pt->tname)!=0)
{
@@ -491,6 +494,9 @@ error:
static int mt_load_db(str *tname)
{
db_key_t db_cols[3] = {&tprefix_column, &tvalue_column};
+ db_key_t key_cols[1];
+ db_op_t op[1] = {OP_EQ};
+ db_val_t vals[1];
str tprefix, tvalue;
db1_res_t* db_res = NULL;
int i, ret;
@@ -498,6 +504,11 @@ static int mt_load_db(str *tname)
m_tree_t *old_tree = NULL;
mt_node_t *bk_head = NULL;
+ key_cols[0] = &tname_column;
+ VAL_TYPE(vals) = DB1_STRING;
+ VAL_NULL(vals) = 0;
+ VAL_STRING(vals) = tname->s;
+
if(db_con==NULL)
{
LM_ERR("no db connection\n");
@@ -521,7 +532,7 @@ static int mt_load_db(str *tname)
}
if (DB_CAPABILITY(mt_dbf, DB_CAP_FETCH)) {
- if(mt_dbf.query(db_con, 0, 0, 0, db_cols, 0, 2, 0, 0) < 0)
+ if(mt_dbf.query(db_con, key_cols, op, vals, db_cols, 1, 2, 0, 0) < 0)
{
LM_ERR("Error while querying db\n");
return -1;
30 Oct
30 Oct
7:44 p.m.
Hello,
this doesn't look correct.
mtree module works with two types of database tables:
- one having only (tprefix, tvalue)
- one having also tree name (tname, tprefix, tvalue)
Your change seems to affect the first case, which is supposed not to
have tname column, thus not working.
If you keep all the trees in the same database table, you should set db
table parameter for the module:
- http://kamailio.org/docs/modules/stable/modules/mtree.html#idp2546944
Check also the utils/kamctl/mysql/mtree-create.sql to see the structure
of those database tables.
Cheers,
Daniel
On 10/30/13 6:37 PM, Juha Heinanen wrote:
Module: sip-router
Branch: master
Commit: 6fc84c2cf610791939ba73e38b8b5b3c0b5cd047
URL:
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6fc84c2…
Author: Juha Heinanen <jh(a)tutpro.com>
Committer: Juha Heinanen <jh(a)tutpro.com>
Date: Wed Oct 30 17:31:04 2013 +0200
modules/mtree: when loading data from db, load each tree separately
---
modules/mtree/mtree_mod.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/modules/mtree/mtree_mod.c b/modules/mtree/mtree_mod.c
index 0f4348c..eeec89c 100644
--- a/modules/mtree/mtree_mod.c
+++ b/modules/mtree/mtree_mod.c
@@ -291,6 +291,9 @@ static int mod_init(void)
while(pt!=NULL)
{
+ LM_DBG("loading from tree <%.*s>\n",
+ pt->tname.len, pt->tname.s);
+
/* loading all information from database */
if(mt_load_db(&pt->tname)!=0)
{
@@ -491,6 +494,9 @@ error:
static int mt_load_db(str *tname)
{
db_key_t db_cols[3] = {&tprefix_column, &tvalue_column};
+ db_key_t key_cols[1];
+ db_op_t op[1] = {OP_EQ};
+ db_val_t vals[1];
str tprefix, tvalue;
db1_res_t* db_res = NULL;
int i, ret;
@@ -498,6 +504,11 @@ static int mt_load_db(str *tname)
m_tree_t *old_tree = NULL;
mt_node_t *bk_head = NULL;
+ key_cols[0] = &tname_column;
+ VAL_TYPE(vals) = DB1_STRING;
+ VAL_NULL(vals) = 0;
+ VAL_STRING(vals) = tname->s;
+
if(db_con==NULL)
{
LM_ERR("no db connection\n");
@@ -521,7 +532,7 @@ static int mt_load_db(str *tname)
}
if (DB_CAPABILITY(mt_dbf, DB_CAP_FETCH)) {
- if(mt_dbf.query(db_con, 0, 0, 0, db_cols, 0, 2, 0, 0) < 0)
+ if(mt_dbf.query(db_con, key_cols, op, vals, db_cols, 1, 2, 0, 0) < 0)
{
LM_ERR("Error while querying db\n");
return -1;
_______________________________________________
sr-dev mailing list
sr-dev(a)lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
5:45 p.m.
Daniel-Constantin Mierla writes:
mtree module works with two types of database tables:
- one having only (tprefix, tvalue)
- one having also tree name (tname, tprefix, tvalue)
Your change seems to affect the first case, which is supposed not to
have tname column, thus not working.
If you keep all the trees in the same database table, you should set db
table parameter for the module:
- http://kamailio.org/docs/modules/stable/modules/mtree.html#idp2546944
it reads:
3.2. db_table (string)
Name of database table where data for trees is stored. It is ignored if
a 'mtree' parameter is defined.
since i have defined two 'mtree' parameters, db_table param has no
meaning.
in my tests, the change that i made has worked fine. in which case
would it fail?
-- juha
9:52 p.m.
On 10/30/13 4:45 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
if you define the trees with module parameters, then you have to use
different tables in database, one for each. Those tables don't need
tname column.
If you just store all the trees in one table (which has tname column),
then don't define them in the config, just set the table where they are
stored. A reload command will reload all the trees in that table
Cheers,
Daniel
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
mtree module works with two types of database
tables:
- one having only (tprefix, tvalue)
- one having also tree name (tname, tprefix, tvalue)
Your change seems to affect the first case, which is supposed not to
have tname column, thus not working.
If you keep all the trees in the same database table, you should set db
table parameter for the module:
- http://kamailio.org/docs/modules/stable/modules/mtree.html#idp2546944
it reads:
3.2. db_table (string)
Name of database table where data for trees is stored. It is ignored if
a 'mtree' parameter is defined.
since i have defined two 'mtree' parameters, db_table param has no
meaning.
in my tests, the change that i made has worked fine. in which case
would it fail?
7:47 p.m.
Daniel-Constantin Mierla writes:
i have not found such a constraint in the readme. is my patch somehow
conflicting the current readme?
in my tests,
the change that i made has worked fine. in which case
would it fail?
if you define the trees with module parameters, then you have to
use
different tables in database, one for each. Those tables don't need
tname column. If you just store all the trees in one table (which
has tname column),
then don't define them in the config, just set the table where they are
stored. A reload command will reload all the trees in that table
the trees may have different types. how can you tell that without mtree
parameter?
after the patch, one db table can have many trees each defined
separately by mtree parameter.
does that break something in the readme?
-- juha
10:16 p.m.
On 10/30/13 6:47 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
i have not found such a constraint in the readme. is my patch
somehow
conflicting the current readme?
I haven't checked for now, but it
doesn't really matters - readme is
written after the code, not the other way around. If there is a missing
description in the readme, it will be fixed.
Your patch breaks existing functionality, so practically you cannot have
mtree table, only mtrees. That's not good because for large number of
records (e.g., millions), I don't want to store tree name for each
record when I know is only one (and do WHERE comparison for each load).
Again, see utils/kamctl/mysql/mtree-create.sql - both tables are there
from the beginning of the module.
It is no a problem to add a patch that will enhance for defining trees
that load from a table that has tname, But should not break the other one.
Cheers,
Daniel
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
in my
tests, the change that i made has worked fine. in which case
would it fail?
if you define the trees with module parameters, then you have to
use
different tables in database, one for each. Those tables don't need
tname column. If you just store all the trees in one table
(which has tname column),
then don't define them in the config, just set the table where they are
stored. A reload command will reload all the trees in that table
the trees may
have different types. how can you tell that without mtree
parameter?
after the patch, one db table can have many trees each defined
separately by mtree parameter.
does that break something in the readme?
8:08 p.m.
Daniel-Constantin Mierla writes:
It is no a problem to add a patch that will enhance
for defining trees
that load from a table that has tname, But should not break the other one.
which case it is that the patch breaks? perhaps it is possible to
enhance the patch avoid breaking existing behavior.
-- juha
10:36 p.m.
On 10/30/13 7:08 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
# cat utils/kamctl/mysql/mtree-create.sql
INSERT INTO version (table_name, table_version) values ('mtree','1');
CREATE TABLE mtree (
id INT(10) UNSIGNED AUTO_INCREMENT PRIMARY KEY NOT NULL,
tprefix VARCHAR(32) DEFAULT '' NOT NULL,
tvalue VARCHAR(128) DEFAULT '' NOT NULL,
CONSTRAINT tprefix_idx UNIQUE (tprefix)
);
INSERT INTO version (table_name, table_version) values ('mtrees','2');
CREATE TABLE mtrees (
id INT(10) UNSIGNED AUTO_INCREMENT PRIMARY KEY NOT NULL,
tname VARCHAR(128) DEFAULT '' NOT NULL,
tprefix VARCHAR(32) DEFAULT '' NOT NULL,
tvalue VARCHAR(128) DEFAULT '' NOT NULL,
CONSTRAINT tname_tprefix_tvalue_idx UNIQUE (tname, tprefix, tvalue)
);
As you can see from above sal table definitions, 'mtree' doesn't have
tname column and you introduced a constraint that a table is loaded only
when 'tname' column is present. So practically, the module doesn't work
anymore with such table structure.
I could suggest adding a new attribute to table definition, such as
'multiple=1' (derived from the fact that a db table has records for
multiple trees), which when present, the load operation will do the
query with WHERE condition on tname. Otherwise, will do it like it was
so far.
Daniel
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
It is no a problem to add a patch that will
enhance for defining trees
that load from a table that has tname, But should not break the other one.
which
case it is that the patch breaks? perhaps it is possible to
enhance the patch avoid breaking existing behavior.
5:49 p.m.
Daniel-Constantin Mierla writes:
mtree module works with two types of database tables:
- one having only (tprefix, tvalue)
- one having also tree name (tname, tprefix, tvalue)
Your change seems to affect the first case, which is supposed not to
have tname column, thus not working.
there is another function mt_load_db_trees() that load all the trees at
one shot. my change didn't affect that branch of the code.
-- juha
if(mt_defined_trees())
{
LM_DBG("static trees defined\n");
pt = mt_get_first_tree();
while(pt!=NULL)
{
LM_DBG("loading from tree <%.*s>\n",
pt->tname.len, pt->tname.s);
/* loading all information from database */
if(mt_load_db(&pt->tname)!=0)
{
LM_ERR("cannot load info from database\n");
goto error1;
}
pt = pt->next;
}
} else {
if(db_table.len<=0)
{
LM_ERR("no trees table defined\n");
goto error1;
}
if(mt_init_list_head()<0)
{
LM_ERR("unable to init trees list head\n");
goto error1;
}
/* loading all information from database */
if(mt_load_db_trees()!=0)
{
LM_ERR("cannot load trees from database\n");
goto error1;
}
}
10 p.m.
On 10/30/13 4:49 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
This is the one you should use when you store many trees in one database
table.
The code you changed is for dedicated table per tree, where tname column
is not present (and not needed as there are only the records for one
tree, thus the value will be the same).
Therefore, you should not define the trees in config file, just set the
db table name in config.
Cheers,
Daniel
mtree module works with two types of database
tables:
- one having only (tprefix, tvalue)
- one having also tree name (tname, tprefix, tvalue)
Your change seems to affect the first case, which is supposed not to
have tname column, thus not working.
there is another function mt_load_db_trees()
that load all the trees at
one shot. my change didn't affect that branch of the code.
-- juha
if(mt_defined_trees())
{
LM_DBG("static trees defined\n");
pt = mt_get_first_tree();
while(pt!=NULL)
{
LM_DBG("loading from tree <%.*s>\n",
pt->tname.len, pt->tname.s);
/* loading all information from database */
if(mt_load_db(&pt->tname)!=0)
{
LM_ERR("cannot load info from database\n");
goto error1;
}
pt = pt->next;
}
} else {
if(db_table.len<=0)
{
LM_ERR("no trees table defined\n");
goto error1;
}
if(mt_init_list_head()<0)
{
LM_ERR("unable to init trees list head\n");
goto error1;
}
/* loading all information from database */
if(mt_load_db_trees()!=0)
{
LM_ERR("cannot load trees from database\n");
goto error1;
}
}
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
10:10 p.m.
On 10/30/13 6:48 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
Then use different tables (or views)
per table.
Cheers,
Daniel
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
Therefore, you should not define the trees in
config file, just set the
db table name in config.
as i already replied, i don't see how that can be
done when some trees
hold int values and some hold string values.
8:02 p.m.
Daniel-Constantin Mierla writes:
i don't want so complicated solution. mtrees table is designed to hold
many trees because it has tname column. with mtree parameter it is
possible to define the type of each table. what is wrong with that?
-- juha
as i already
replied, i don't see how that can be done when some trees
hold int values and some hold string values.
Then use different tables (or views)
per table.
10:26 p.m.
On 10/30/13 7:02 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
i don't want so complicated solution. mtrees table is designed to
hold
many trees because it has tname column. with mtree parameter it is
possible to define the type of each table. what is wrong with that?
The wrong part
here is that your patch breaks loading mtree-like tables.
If you want to combine mtrees-like tables with definition in config,
that's fine, do proper patch for it. Don't break what was designed from
the start of the module.
Daniel
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
as i
already replied, i don't see how that can be done when some trees
hold int values and some hold string values.
Then use different tables (or views)
per table.
8:12 p.m.
Daniel-Constantin Mierla writes:
The wrong part here is that your patch breaks loading
mtree-like tables.
If you want to combine mtrees-like tables with definition in config,
that's fine, do proper patch for it. Don't break what was designed from
the start of the module.
i still don't understand the case that breaks. can you give an example
of mtree module parameter config involding mtree table that has stopped
working?
-- juha
10:37 p.m.
On 10/30/13 7:12 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
create a table that has the same structure as 'tree' table from the
utils/kamctl/mysql/mtree-create.sql and provide it to mtree definition.
Daniel
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
The wrong part here is that your patch breaks
loading mtree-like tables.
If you want to combine mtrees-like tables with definition in config,
that's fine, do proper patch for it. Don't break what was designed from
the start of the module.
i still don't understand the case that breaks. can
you give an example
of mtree module parameter config involding mtree table that has stopped
working?
8:33 p.m.
Daniel-Constantin Mierla writes:
create a table that has the same structure as
'tree' table from the
utils/kamctl/mysql/mtree-create.sql and provide it to mtree
definition.
ok, now i got it. does k db api allow querying table description to
find out if table has tname_column?
did you mean something like this:
modparam("mtree", "mtree",
"name=mytable;dbtable=routes;multiple=1;type=0;")
-- juha
10:56 p.m.
On 10/30/13 7:33 PM, Juha Heinanen wrote:
Daniel-Constantin Mierla writes:
Not a dedicated member for that and I would
avoid adding one for now, it
requires changes in all db modules.
create a table that has the same structure as
'tree' table from the
utils/kamctl/mysql/mtree-create.sql and provide it to mtree
definition.
ok, now i got it. does k db api allow querying table description to
find out if table has tname_column?
did you mean something like this:
modparam("mtree", "mtree",
"name=mytable;dbtable=routes;multiple=1;type=0;")
Yes. That seemed the
simplest approach from my point of view, not to
touch db structure, so it can be pushed as a fix, of course. Other ideas
are welcome, of course.
Daniel
--
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Trainings - Berlin, Nov 25-28
- more details about Kamailio trainings at http://www.asipto.com -
8:51 p.m.
Daniel-Constantin Mierla writes:
i reverted the change, since i don't have time right now to work more on
it. i'll see what i can do during the next couple of days.
-- juha
did you mean
something like this:
modparam("mtree", "mtree",
"name=mytable;dbtable=routes;multiple=1;type=0;")
Yes. That seemed the
simplest approach from my point of view, not to
touch db structure, so it can be pushed as a fix, of course. Other ideas
are welcome, of course.
1 Nov
1 Nov
9:33 p.m.
Daniel-Constantin Mierla writes:
i just made a commit that adds 'multi' param to mtree definition and
allows one db table to contain trees of different node types.
-- juha
modparam("mtree", "mtree",
"name=mytable;dbtable=routes;multiple=1;type=0;")
Yes. That seemed the
simplest approach from my point of view, not to
touch db structure, so it can be pushed as a fix, of course. Other ideas
are welcome, of course.
4039
days inactive
4041
days old
19 comments
2 participants
participants (2)
-
Daniel-Constantin Mierla -
Juha Heinanen