Re: Proposed Patch for Consideration for Routing.cxx to handle 2 column SQL return better.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);
>


------------------------------------------------------------------------------
Nokia and AT&T present the 2010 Calling All Innovators-North America contest
Create new apps & games for the Nokia N8 for consumers in  U.S. and Canada
$10 million total in prizes - $4M cash, 500 devices, nearly $6M in marketing
Develop with Nokia Qt SDK, Web Runtime, or Java and Publish to Ovi Store 
http://p.sf.net/sfu/nokia-dev2dev
_______________________________________________________

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/


[Index of Archives]     [SIP]     [Open H.323]     [Gnu Gatekeeper]     [Asterisk PBX]     [ISDN Cause Codes]     [Yosemite News]

  Powered by Linux