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/
------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users
worldwide. Take advantage of special opportunities to increase revenue and
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d
_______________________________________________________
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/
