Jan;
I've tested the new code with what I believe is all my test scenario's
and all is working perfectly. I have to say your patch is much more
elegant and to the point than mine was.
Thanks very much
Andrew
Jan Willamowius wrote:
> Andrew,
>
> your patch only handles the case where the database returns
> "<alias>, NOGATEWAY" properly, but fails when the first column in a 2
> column result is an IP.
>
> I have put a patch into the CVS that allows both "<alias>, IGNORE" and
> "<ip>, IGNORE" and will treat them as if the 2nd column wasn't there.
> My implementation also avoids the copy/past code duplication.
>
> I would have preferred a patch that handles these varying column
> numbers on a higher level in the GkSqlResult class for all SQL
> based modules, but I don't have the time right now to figure out an
> implementation that isn't too expensive performance wise.
>
> Regards,
> Jan
>
>
> Andrew Herdman wrote:
>
>> Jan;
>>
>> Been working on this in my "spare" time between other projects. I did
>> get what you mentioned working using mysql and a stored procedure that
>> returned what I wanted when I wanted it. So one column when it was just
>> an alias transform, and two columns when it was a gateway call (with
>> possibility of a transform at the same time). This is working well.
>>
>> Then someone came along and wanted me to look at using PostgreSQL
>> instead. So I re-wrote my stored procedures with some help from this
>> someone, but we ran into a significant problem. Unlike mysql,
>> postgresql will only return a deterministic number of columns, you then
>> can't provide the same translation and call routing option in the query
>> passed from GNU/GK, at least not without deploying multiple GNU/GK's,
>> one for translation, and one for routing, which is probably excessive in
>> any normal environment.
>>
>> So I re-visited the original patch I sent you last month, and I can say
>> that I do understand your hesitation on the NULL pieces now that I've
>> had to muck around in the database area myself. So, very similar to the
>> previous patch, but this time, matching a text much like you did for the
>> REJECT string, but in this case, if there is no gateway, instead of NULL
>> as I did last time, return "NOGATEWAY" which is interpreted in this patch;
>>
>> --- Routing.cxx.orig 2010-09-12 10:55:13.000000000 -0400
>> +++ Routing.cxx 2010-09-23 14:20:56.000000000 -0400
>> @@ -1818,7 +1818,21 @@
>> H323SetAliasAddress(newDestination, newAliases[0]);
>> destination.SetNewAliases(newAliases);
>> }
>> - } else if (result->GetNumFields() == 2) {
>> +// Added by Andrew Herdman to allow a return on the 2nd column always
>> but indicate not to use it.
>> + } else if ((result->GetNumFields() == 2) && (resultRow[1].first
>> == "NOGATEWAY")) {
>> + PString newDestinationAlias = resultRow[0].first;
>> + PString newDestinationIP = resultRow[1].first;
>> + PTRACE(5, m_name << "\tQuery result : " <<
>> newDestinationAlias << ", " << newDestinationIP);
>> + if (newDestinationAlias.ToUpper() == "REJECT") {
>> + destination.SetRejectCall(true);
>> + } else {
>> + H225_ArrayOf_AliasAddress newAliases;
>> + newAliases.SetSize(1);
>> + H323SetAliasAddress(newDestinationAlias,
>> newAliases[0]);
>> + destination.SetNewAliases(newAliases);
>> + }
>> + } else if ((result->GetNumFields() == 2) && (resultRow[1].first
>> != "NOGATEWAY")) {
>> +// End Code Added by Andrew Herdman (2010-09-23)
>> PString newDestinationAlias = resultRow[0].first;
>> PString newDestinationIP = resultRow[1].first;
>> PTRACE(5, m_name << "\tQuery result : " <<
>> newDestinationAlias << ", " << newDestinationIP);
>>
>> Now, I suspect this could be much more efficiently inserted in the code
>> that matches two columns in the first place, but I couldn't grasp in the
>> logic where to insert it.
>>
>> While for now I myself would have been happy to stick with mysql, I
>> recognize that postgresql offers much more options for stored functions,
>> including alternative languages that will greatly improve the
>> customization on call control that can be achieved beyond what mysql's
>> sql implementation allows for.
>>
>> Thanks and Best Regards
>> Andrew
>>
>>
>> Jan Willamowius wrote:
>>
>>> Andrew,
>>>
>>> sure, your patch makes your current query work, but so would rewriting
>>> the query. I am hesitant to add the patch to the CVS, because in many
>>> cases the NULL in one of the columns is the result of a bug in the SQL
>>> query, eg. a JOIN gone astray, missing data etc. To give that a legal
>>> meaning to a typical error condition will make debugging the SQL
>>> routing even more complicated than it is now.
>>>
>>> There are many ways to do what you want with the current implementation:
>>> Your SQL query gets complicated, because you try to produce both result
>>> formats (just the alias rewrite and the routing to a gateway) in a
>>> single query.
>>>
>>> It would be much easier if you would either provide a gateway IP for
>>> all calls or do the gateway routing by adding a tech-prefix from the
>>> database and let GnuGk do the gateway selection based on that.
>>>
>>> But even if you want to stick to your current model, you can just put
>>> the query in a stored procedure and use an IF or CASE statement to
>>> produce the format GnuGk currently expects.
>>>
>>> Regards,
>>> Jan
>>>
>>>
>>> Andrew Herdman wrote:
>>>
>>>> Hi Jan;
>>>>
>>>> I'm not sure I understand (to be clear, I'm historically a Network
>>>> Engineer, now Voice and Video as well, but I don't program per say, and
>>>> databases, well sometimes I don't get them at all). Let me explain my
>>>> use case and why I wrote (copied and edited really, again not a
>>>> programmer), the patch.
>>>>
>>>> I have an mysql Database called routing with columns;
>>>>
>>>> CREATE TABLE routing.lookup (
>>>> orig_alias varchar(255),
>>>> new_alias varchar(255),
>>>> gatewayip varchar(255),
>>>> active varchar(1),
>>>> comment varchar(255)
>>>> );
>>>>
>>>> I use the following query, to get my answers for GNU/GK;
>>>>
>>>> Query=SELECT new_alias, gatewayip FROM lookup WHERE orig_alias='%c' AND
>>>> active='Y'
>>>>
>>>>
>
>
>>>> I use this database to perform alias transforms, and also call routing
>>>> to gateways.
>>>>
>>>> Here's some sample data;
>>>>
>>>> +------------------+--------------------+---------------+--------+-----------+
>>>> | orig_alias | new_alias | gatewayip | active |
>>>> comment |
>>>> +------------------+--------------------+---------------+--------+-----------+
>>>> | 18665138599 | 18665138513 | NULL | Y |
>>>> NULL |
>>>> | 18667228512 | 18665128512 | NULL | Y |
>>>> NULL |
>>>> | 18665139999 | andrew@xxxxxxxxx | NULL | Y |
>>>> NULL |
>>>> | 18001234567 | 18665100110 | NULL | Y |
>>>> NULL |
>>>> | 18007654321 | 3334 | NULL | Y | MCU
>>>> Test |
>>>> | 18007654322 | andrew@@demo.com | NULL | Y |
>>>> NULL |
>>>> | bob@xxxxxxxx | bob@xxxxxxxx | 10.111.72.134 | Y |
>>>> NULL |
>>>> | _1.2.3.4 | _1.2.3.4 | 10.111.72.134 | Y |
>>>> Test |
>>>> | _ | _ | 10.111.72.134 | Y | IP
>>>> Dialing|
>>>> | *@subdom.cust.com| *@subdom.cust.com | 192.168.1.1 | Y |
>>>> NULL |
>>>> +------------------+--------------------+---------------+--------+-----------+
>>>>
>>>> So in the transform, if I call 18665138599 the gatekeeper will re-write
>>>> to 18665138513 and proceed.
>>>>
>>>> In the case where I dial _1.2.3.4, the alias remains the same, but now
>>>> the gatewayip is passed and the call proceeds to that new Gateway.
>>>>
>>>> Both these things now work with the "patch" I sent. Without the patch,
>>>> the transforms do not work anymore. Now you mention "NULL". Is there
>>>> something that can be put into the gatewayip that will satisfy the first
>>>> transform case without the code change?
>>>>
>>>> Thanks
>>>> Andrew
>>>>
>>>>
>>>>
>>>> Jan Willamowius wrote:
>>>>
>>>>> Andrew,
>>>>>
>>>>> I'm not sure the patch you propose is a good idea. Your code treats a
>>>>> NULL in the second result column as as if the column wasn't there.
>>>>>
>>>>> a.) This shadows bugs in the SQL code causing the NULL value, eg. a
>>>>> missing IFNULL(). If the SQL is written to return 2 columns, there is
>>>>> good chance that the rest of the configuration relies on a gateway IP
>>>>> being set. If your calls still connect, thats sheer luck.
>>>>>
>>>>> b.) We having special treatment for NULL in one case, but not in other
>>>>> columns and not in other SQL modules (SQLAuth etc.) makes the behavior
>>>>> somewhat inconsistent.
>>>>>
>>>>> I think I would prefer a patch that produce warning messages if NULL is
>>>>> encountered in any column across all SQL modules.
>>>>>
>>>>> Regards,
>>>>> Jan
>>>>>
>>>>>
>>>>> Andrew Herdman wrote:
>>>>>
>>>>>> Jan and Group;
>>>>>>
>>>>>> I've been playing heavily with the SQL routing policy pieces of recent
>>>>>> and stumbled upon an issue with a database structure that returns 2
>>>>>> columns, when the second one is NULL. If this case, the call fails.
>>>>>> If the query only returns one column, the function works properly, and
>>>>>> if you ensure that a gateway IP in a case where two columns are being
>>>>>> returned, those calls work (unless you try to use your own IP for the
>>>>>> gateway).
>>>>>>
>>>>>> Anyway, looking at the code, I copied, and edited this patch, it works
>>>>>> in all cases (translate just alias, or add gateway IP, or lastly,
>>>>>> translate alias and add gateway IP). I'd like to submit this patch for
>>>>>> consideration to be included in the CVS code. Please excuse the
>>>>>> comments, they were put into the code so I could track my changes.
>>>>>>
>>>>>> Thank you
>>>>>> Andrew
>>>>>>
>>>>>> --- Routing.cxx 2010-08-27 08:24:08.000000000 -0400
>>>>>> +++ /root/openh323gk/Routing.cxx 2010-08-26 14:24:49.000000000 -0400
>>>>>> @@ -1794,7 +1794,21 @@ void SqlPolicy::DatabaseLookup(
>>>>>> H323SetAliasAddress(newDestination, newAliases[0]);
>>>>>> destination.SetNewAliases(newAliases);
>>>>>> }
>>>>>> - } else if (result->GetNumFields() == 2) {
>>>>>> +// Added by Andrew Herdman to get around the NULL 2nd column (2010-08-26)
>>>>>> + } else if ((result->GetNumFields() == 2) && (resultRow[1].first
>>>>>> == NULL)) {
>>>>>> + PString newDestinationAlias = resultRow[0].first;
>>>>>> + PString newDestinationIP = resultRow[1].first;
>>>>>> + PTRACE(5, m_name << "\tQuery result : " <<
>>>>>> newDestinationAlias << ", " << newDestinationIP);
>>>>>> + if (newDestinationAlias.ToUpper() == "REJECT") {
>>>>>> + destination.SetRejectCall(true);
>>>>>> + } else {
>>>>>> + H225_ArrayOf_AliasAddress newAliases;
>>>>>> + newAliases.SetSize(1);
>>>>>> + H323SetAliasAddress(newDestinationAlias,
>>>>>> newAliases[0]);
>>>>>> + destination.SetNewAliases(newAliases);
>>>>>> + }
>>>>>> + } else if ((result->GetNumFields() == 2) && (resultRow[1].first
>>>>>> != NULL)) {
>>>>>> +// End Code Added by Andrew Herdman (2010-08-26)
>>>>>> PString newDestinationAlias = resultRow[0].first;
>>>>>> PString newDestinationIP = resultRow[1].first;
>>>>>> PTRACE(5, m_name << "\tQuery result : " <<
>>>>>> newDestinationAlias << ", " << newDestinationIP);
>>>>>>
>
>
------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1, ECMAScript5, and DOM L2 & L3.
Spend less time writing and rewriting code and more time creating great
experiences on the web. Be a part of the beta today.
http://p.sf.net/sfu/beautyoftheweb
_______________________________________________________
Posting: mailto:Openh323gk-users@xxxxxxxxxxxxxxxxxxxxx
Archive: http://sourceforge.net/mailarchive/forum.php?forum_name=openh323gk-users
Unsubscribe: http://lists.sourceforge.net/lists/listinfo/openh323gk-users
Homepage: http://www.gnugk.org/
