Re: [pmacct-discussion] Source port column name depends on database
Hi Chris, To say this work (as agreed in the shape of sql table version 8) has been just committed to the CVS. Please give it a try and let me know if it seems to work to your eyes. Cheers, Paolo On Mon, Sep 13, 2010 at 11:14:18AM +0200, Chris Wilson wrote: Hi all, We just had a bug report in pmGraph because it assumed that the source port database column was called src_port always, as it is in MySQL. The user is using a postgres database, and it appears that the column is called port_src there instead: if (!strcmp(config.type, mysql) || !strcmp(config.type, sqlite3)) { strncat(insert_clause, src_port, SPACELEFT(insert_clause)); strncat(where[primitive].string, src_port=%u, SPACELEFT(where[primitive].string)); } else { strncat(insert_clause, port_src, SPACELEFT(insert_clause)); strncat(where[primitive].string, port_src=%u, SPACELEFT(where[primitive].string)); } I would be much happier writing database-independent code around pmacct if it didn't do things like this. I understand that there is a backwards compatibility issue with changing it, but perhaps it could be done in a new version of the mysql or postgres schema? Cheers, Chris. -- Aptivate | http://www.aptivate.org | Phone: +44 1223 760887 The Humanitarian Centre, Fenner's, Gresham Road, Cambridge CB1 2ES Aptivate is a not-for-profit company registered in England and Wales with company number 04980791. ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
Re: [pmacct-discussion] Source port column name depends on database
Hi Paolo, On Wed, 6 Oct 2010, Paolo Lucente wrote: To say this work (as agreed in the shape of sql table version 8) has been just committed to the CVS. Please give it a try and let me know if it seems to work to your eyes. Thanks for this. I haven't compiled it yet, but I noticed this line: if ((!strcmp(config.type, mysql) || !strcmp(config.type, sqlite3)) config.sql_table_version != 8) { Doesn't this mean that it will revert to the old schema when we release a schema version 9? Is that what you wanted? It seems surprising to me. I would have expected config.sql_table_version 8 instead. By the way I've written this story up in a blog post, I hope that's OK, but please let me know if you want me to edit it: http://blog.aptivate.org/2010/10/06/consistency-portability-and-backwards-compatibility/ Cheers, Chris. ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
Re: [pmacct-discussion] Source port column name depends on database
Hi Paolo, On Wed, 6 Oct 2010, Paolo Lucente wrote: Yes, that's intended for a couple of reasons: 1) don't expect to release any more table versions: you see that already happening with recently introduced primitives; idea is to stick to a table version (or style nowadays) and then customize it from there, adding (or removing) fields to the base schema. 2) combinations of table type/version are internally mapped to a number greater than 8, ie. table type BGP, table version 1. OK, I didn't know that, thanks. No problem with the blog entry. I believe you can change the Luckily he agreed to simply He agreed - i'm not such of an un-cooperative beast, am i? Of course not, far from it :) I've changed it. Cheers, Chris. ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
Re: [pmacct-discussion] Source port column name depends on database
Hi Paolo, On Wed, 15 Sep 2010, Paolo Lucente wrote: On Tue, Sep 14, 2010 at 09:16:37AM +0200, Chris Wilson wrote: I'm not sure about adding a new config switch, do we actually need it? Funnily enough, and that was my perspective, in this case a configuration switch only adds two if-then-else in the common SQL plugins code. Whereas impact of a new schema version you can verify it yourself by grepping the source code for 'sql_table_version'. I think the code that uses sql_table_version has been well written, and none of these places should need to be changed at all. The only place that should need changing, I hope, is the one line of sql_common.c that currently says: if (!strcmp(config.type, mysql) || !strcmp(config.type, sqlite3)) { and would now check for sql_table_version = 7 (or similar) instead. So this change does not actually increase the code complexity, or the number of config options, at all. I'd target release 0.12.5 for this as 0.12.4 is planned to be out soon (by end of the month). Will give a shout as soon as i get something workable in the CVS. That would be great, please do! Cheers, Chris. -- Aptivate | http://www.aptivate.org | Phone: +44 1223 760887 The Humanitarian Centre, Fenner's, Gresham Road, Cambridge CB1 2ES Aptivate is a not-for-profit company registered in England and Wales with company number 04980791. ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
Re: [pmacct-discussion] Source port column name depends on database
On 09/15/2010 05:48:51 AM, Paolo Lucente wrote: Hi Chris, On Tue, Sep 14, 2010 at 09:16:37AM +0200, Chris Wilson wrote: I'm not sure about adding a new config switch, do we actually need it? I seem to recall some wiser counsel to not add configuration options where possible, as it exponentially multiplies the complexity of the software code and also linearly increases the complexity of using it. If our intention is to rename the MySQL fields going forward, why not just use a new schema version to grandfather the old column names? Funnily enough, and that was my perspective, in this case a configuration switch only adds two if-then-else in the common SQL plugins code. Whereas impact of a new schema version you can verify it yourself by grepping the source code for 'sql_table_version'. But saving a config switch, i agree, it's always nice. I'd target release 0.12.5 for this as 0.12.4 is planned to be out soon (by end of the month). Will give a shout as soon as i get something workable in the CVS. I too don't like having a config switch. But note that changing the schema in this fashion breaks backwards compatibility in anything that's querying the data. So, I have thoughts. Mostly on version numbering schemes. Feel free to ignore this. I've not given it a whole lot of thought regards pmacct. Breaking backwards compatibility is ok in software under development, pre version 1.0. After that I like seeing the major version number bumped when there's a backwards incompatible change (e.g. 1.0 - 2.0). The friendly way to proceed is to announce the upcoming change and introduce a flag to enable the new functionality, with the default to be not-enabled. (--new-feature=off). Eventually the default changes to --new-feature=on and the major version number increments. The next time the major version number increments the flag, --new-feature, is dropped. This is annoying to maintain but should give the end-user enough time to make any changes that there should be no cause for complaint. So, I like the major.minor.bugfix version numbering system, where major is incremented when the user really needs to pay attention or things just won't work. Perhaps the schema version numbering system could also use a policy, so as to be able to distinguish changes that break backwards compatibility? Karl k...@meme.com Free Software: You don't pay back, you pay forward. -- Robert A. Heinlein ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
Re: [pmacct-discussion] Source port column name depends on database
Hi Chris, Agree. I seem to reckon this legacy issue is limited to the TCP/UDP ports only and i'm thinking perhaps the best way to approach it is to issue a true/false config switch, ie. sql_table_compat, for the purpose. But for consistency with the rest, these fields should be aligned to port_src and port_dst. Agree? Cheers, Paolo On Mon, Sep 13, 2010 at 11:14:18AM +0200, Chris Wilson wrote: Hi all, We just had a bug report in pmGraph because it assumed that the source port database column was called src_port always, as it is in MySQL. The user is using a postgres database, and it appears that the column is called port_src there instead: if (!strcmp(config.type, mysql) || !strcmp(config.type, sqlite3)) { strncat(insert_clause, src_port, SPACELEFT(insert_clause)); strncat(where[primitive].string, src_port=%u, SPACELEFT(where[primitive].string)); } else { strncat(insert_clause, port_src, SPACELEFT(insert_clause)); strncat(where[primitive].string, port_src=%u, SPACELEFT(where[primitive].string)); } I would be much happier writing database-independent code around pmacct if it didn't do things like this. I understand that there is a backwards compatibility issue with changing it, but perhaps it could be done in a new version of the mysql or postgres schema? Cheers, Chris. -- Aptivate | http://www.aptivate.org | Phone: +44 1223 760887 The Humanitarian Centre, Fenner's, Gresham Road, Cambridge CB1 2ES Aptivate is a not-for-profit company registered in England and Wales with company number 04980791. ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
Re: [pmacct-discussion] Source port column name depends on database
Hi Paolo, On Tue, 14 Sep 2010, Paolo Lucente wrote: Agree. I seem to reckon this legacy issue is limited to the TCP/UDP ports only and i'm thinking perhaps the best way to approach it is to issue a true/false config switch, ie. sql_table_compat, for the purpose. But for consistency with the rest, these fields should be aligned to port_src and port_dst. Agree? Agree definitely on consistency, and don't really mind which way the name goes. I'm not sure about adding a new config switch, do we actually need it? I seem to recall some wiser counsel to not add configuration options where possible, as it exponentially multiplies the complexity of the software code and also linearly increases the complexity of using it. If our intention is to rename the MySQL fields going forward, why not just use a new schema version to grandfather the old column names? Cheers, Chris. -- Aptivate | http://www.aptivate.org | Phone: +44 1223 760887 The Humanitarian Centre, Fenner's, Gresham Road, Cambridge CB1 2ES Aptivate is a not-for-profit company registered in England and Wales with company number 04980791. ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists
[pmacct-discussion] Source port column name depends on database
Hi all, We just had a bug report in pmGraph because it assumed that the source port database column was called src_port always, as it is in MySQL. The user is using a postgres database, and it appears that the column is called port_src there instead: if (!strcmp(config.type, mysql) || !strcmp(config.type, sqlite3)) { strncat(insert_clause, src_port, SPACELEFT(insert_clause)); strncat(where[primitive].string, src_port=%u, SPACELEFT(where[primitive].string)); } else { strncat(insert_clause, port_src, SPACELEFT(insert_clause)); strncat(where[primitive].string, port_src=%u, SPACELEFT(where[primitive].string)); } I would be much happier writing database-independent code around pmacct if it didn't do things like this. I understand that there is a backwards compatibility issue with changing it, but perhaps it could be done in a new version of the mysql or postgres schema? Cheers, Chris. -- Aptivate | http://www.aptivate.org | Phone: +44 1223 760887 The Humanitarian Centre, Fenner's, Gresham Road, Cambridge CB1 2ES Aptivate is a not-for-profit company registered in England and Wales with company number 04980791. ___ pmacct-discussion mailing list http://www.pmacct.net/#mailinglists