[Home]

Summary:ASTERISK-04118: RealTime mysql does not parse commas
Reporter:j (j)Labels:
Date Opened:2005-05-09 08:43:49Date Closed:2005-05-16 09:27:40
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Resources/res_features
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:In the latest cvs head, RealTime isn't parsing commas, resulting in things such as

   -- Executing Dial("SIP/205-414b", "SIP/207,20")
May  9 09:49:40 WARNING[14190]: chan_sip.c:1556 create_addr: No such host: 207,20

 If I use a pipe character instead of a comma in the database things work correctly.
Comments:By: Michael Jerris (mikej) 2005-05-09 08:50:28

You clearly have a workaround, using |.  This is not major.  Do you have a patch for this?
By: j (j) 2005-05-09 08:58:28

I am not all that confident in my C skills. I am looking into the code to see if I can track it down though.

 Actually. This is Major. I work for a company who has a very large dial plan, nearly all of which is in a database. We're gearing toward rolling out RealTime so fixing every single comma is ... well... outrageous :(

 I don't know enough about the innards of asterisk code to suggest the correct approach to patching this. Obviously the dial plan parses commas into pipe characters, so replacing them in the db results set doesn't seem correct.

 I'll look some more.
 Help would be appreciated of course.

 Thanks
j
By: Michael Jerris (mikej) 2005-05-09 09:38:31

use unixodbc realtime with mysql, I am told that it works with the commas.
By: j (j) 2005-05-09 10:16:35

I will do so.

Is realtime_mysql going bye bye then?
By: j (j) 2005-05-09 10:27:24

Nope.
Just got res_config_odbc running. Does the same thing.
By: Brian West (bkw918) 2005-05-09 10:36:04

is this with pbx_realtime? If so you must use |'s becuase mark didn't want the routine in there to turn ,'s into |'s in fear that it might actually use the CPU. :P  Er I mean cause a performance hit... of like what two nano seconds.

/b
By: j (j) 2005-05-09 10:48:50

Well, it's with "RealTime".
I <blush> don't know the difference. pbx_realtime is the wrapper that includes odbc or mysql I take it?

 Yes it's with realtime. DB config stuff.
 He wanted to make realtime not maintain the syntax we've been using in the files all this time because of two nano-seconds?

 It seems more important in my opinion to maintain a convention. I've already spent two days on figuring out what's going on simply because realtime doesn't support ","'s. Seems silly to me.

 So even if I submit a patch there are/will be no plans to include conversions between |'s and ,'s? Which means I have to add it myself every single upgrade? That's rough :(

 Thanks for your help guys. Don't wanna sound unappreciative.
 Perhaps you could bring this up with Mark again? Most people I know using this uses it in an enterprise environment, and would be more than willing to sacrifice 2 nano-seconds in order to have the files and DB work with the same syntax.
By: Brian West (bkw918) 2005-05-11 13:07:48

Ya I did the patch already to get smacked saying "NO NO NO"

/b
By: Gunnar Harms (speedy) 2005-05-12 05:06:14

I would like to have this patch. Why does ODBC use the comma and in mysql it's broken? The convention is more important than 2 nano-seconds. Can anybody tell me where to change the behaviour in the sources? I change it myself for my installations ... don't care about nano-seconds.
By: j (j) 2005-05-12 08:01:36

Hey speedy.

Yes. It's a simple patch. I'm sure the guys @ digium will be saying "NO NO NO" to me too, cause I'm not a C coder by trade, but I've tested and am rolling out this patch so I know, while maybe notthe most elegant approach, it at least works:

in res_config_mysql.c: around line 140 here's a block of code.
 Find the difference between this and yours and replace:
       char *comma;
while((row = mysql_fetch_row(result))) {
for(i = 0; i < numFields; i++) {
stringp = row[i];
while(stringp) {
chunk = strsep(&stringp, ";");
if(chunk && !ast_strlen_zero(ast_strip(chunk))) {
                       if(strcasecmp(fields[i].name, "appdata") == 0){
                           while((comma = strchr(chunk, ',')) != NULL){ *comma = '|'; }
                       }
if(prev) {
prev->next = ast_variable_new(fields[i].name, chunk);
if (prev->next) {
prev = prev->next;
}
} else {
prev = var = ast_variable_new(fields[i].name, chunk);
}
}
}
}
}

and in res_config_odbc.c: around line 153:
           char *comma;
while(stringp) {
chunk = strsep(&stringp, ";");
if (chunk && !ast_strlen_zero(ast_strip(chunk))) {
                   if(strcasecmp(coltitle, "appdata") == 0){
                       while((comma = strchr(chunk, ',')) != NULL){ *comma = '|'; }
                   }
if (prev) {
prev->next = ast_variable_new(coltitle, chunk);
if (prev->next)
prev = prev->next;
} else
prev = var = ast_variable_new(coltitle, chunk);

}
}


That will simply take only the "appdata" field from the extensions table and change every instance of a "," to a "|" .

 Hopefully I didn't do anything wrong there. Seems to work great for me.
By: Brian West (bkw918) 2005-05-13 11:29:49

The correct way would be to move this into app.c or the core:

static char *process_quotes_and_slashes(char *start, char find, char replace_with)

Its currently located in pbx_config.c  Thats how I did it.

/b
By: j (j) 2005-05-13 13:18:32

This is why I submitted it to the Mantis :)

 I knew I wouldn't do it exactly right.

 Could we, perhaps, come to a compromise and include a patch in the "contrib" directory? ... Or at the very least, supply a working patch to this post?

 I would be very grateful for a "proper" working patch to this problem.

 Thanks
By: Kevin P. Fleming (kpfleming) 2005-05-14 20:56:57

what is wrong with just using '|' characters, since that is what Asterisk applications actually receive anyway? I've never used ',' in my dialplan, ever since I learned they just got converted to '|' behind the scenes...

I consider ',' as an application argument separator to be a legacy artifact of the _text file_ dialplan reader, and it shouldn't be carried over into new dialplan parsers, of which Realtime is the first example. Maybe we should just change the sample extensions.conf file to use '|' and start the process to deprecate usage of ','?
By: j (j) 2005-05-16 08:01:46

Well, part of it is preference, this is true.

 Consider this:

 You are a programmer. Using "strange" symbols in "code" makes sense to you. I can deal with it too, as I am a coder. But; many people aren't coders. Most of the people who work on our servers are admin type people, and for a conf file, especially one that uses "function calls" such as Dial(arg,arg), commas make more visual sense, and are more common throughout conf files for all software than are pipe characters.

 Another point is that there is a convention in place. If the conf file had been using only pipe characters the whole time, sure it'd be weird, but we would have gotten used to it. Well, it wasn't set up that way. So everyone is used to seeing commas and/or pipe characters, so forcing us to change once we've got a 12,000 line dialplan is, well that's just "right out".


 On a side note; I've noticed a trend on this mantis from *some* of the people who have responded to my posts, which I find, to say the least, incredibly distrurbing and disheartening.
  I post to this place, and open bugs because I work with this software all day long, every day, think it's great, and want to help and see it improve. I do not, in my opinion, abuse this forum, and everything I've asked has been serious, and I try to remain professional.

  When I post a bug and 80% of the responses I get are "well if that's broken, just do it a different way", it shows a laziness, and lack of understanding of the industry and of how people use the software you are building, that is beyond frustrating.

  Read the posts in the beginning of this bug note. Most of the answers are "just use pipe characters". It took several posts before I even got a straight answer as to WHY it didn't work. And now I'm STILL getting "just use pipe characters" responses.
  This is not the first time I've gotten reponses like this on bug notes and it's really getting old.

  Anyway. Thanks  bkw918  for being helpful.

  I'm sure my account will be closed for trying to be honest, so I'll see you somewhere else ;(
By: Kevin P. Fleming (kpfleming) 2005-05-16 09:27:08

With that sort of attitude it's no wonder you feel like you aren't being helped.

Realtime entries do _not_ have to be formatted the same way as text file entries; if we've learned that there is a better way to do things, then we have the opportunity to take advantage of that when a new feature is added (just like all the recent deprecation of simple applications and turning them into dialplan functions). I don't buy the argument that 'admins' are used to seeing this in configuration files... do you have 'admins' directly editing database entries in your Realtime database? I suspect you have some sort of front-end interface for them to use, which could easily handle generating fields separated by | instead of , characters.

I don't want to see the performance hit of searching/replacing , characters in Realtime lookups just so people can not have to deal with a relatively minor change... What's going to happen when those people upgrade to current CVS HEAD and suddenly their logs are flooded with deprecation warnings from all the changes that have been made in the last two weeks? Should we not be allowed to require people to occasionally upgrade their configurations so that we can take advantage of new features and get rid of old stuff?

I am going to close this bug now, but not because I'm trying to ignore you or anyone else; it's just been decided that the most efficient way to handle dialplans from Realtime is to hold the data in its 'native' format, which is using '|' characters to separate application arguments.