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);
--
Jan Willamowius, jan@xxxxxxxxxxxxxx, http://www.gnugk.org/
------------------------------------------------------------------------------
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/
