Re: Patch: EMI UUCP DLR

Thanks,

1) I will make it a constant. I am trying not to unnecessarily inflate 
configuration. Besides user has no control over the MSISDNs passed. These 
are controlled by his clients and can be all over the place. Therefore I 
don't see that he could have more insight than the developer on this.

2) I imagine you mean:

if (len < 7)
   gwlist_append(binds, dst);

Done. Thinking about it twice I will probably remove the < 7 case in sql 
altogether, both to make code simpler, and I believe it is better to treat 
it with LIKE %... as in the general dst case. Performance wise is not a big 
savings, since very few MSISDNs are < 7.

3) Any thoughts on making use_dest configurable or driver (EMI) dependent?

BR,
Nikos

----- Original Message ----- 
From: "Alejandro Guerrieri" <[email protected]>
To: "Nikos Balkanas" <[email protected]>
Cc: <[email protected]>
Sent: Friday, June 25, 2010 1:21 PM
Subject: Re: Patch: EMI UUCP DLR


A couple of notes:

1. I wouldn't hardcode the "7" all over the place. At least I would use a 
constant, so it'd be easier to change by other people if needed. Better yet, 
I'd make it configurable.

2. I'm not a big fan of inlined conditions:

if (len < 7) gwlist_append(binds, dst);

or

else sql = octstr_format("DELETE FROM `%S` WHERE `%...

Regards,
--
Alejandro Guerrieri
[email protected]



On 25/06/2010, at 10:29, Nikos Balkanas wrote:

> Hi,
>
> Thanks all for your input. This patch improves on the previous one, by 
> accounting for inexact destinations send by the SMSc and deploying LIKE 
> %... constructs in the query for last 7 digits of destination. It also 
> will query on destination only when asked to (configuration or otherwise). 
> Since this is a lot of work, this time I have only changed dlr_mysql.c to 
> see if we all agree. If we are in agreement, I will expand to rest of 
> databases and rest of code.
>
> Notes:
>
> 1) If no destination is specified in the DLR, it assumes that relevant 
> argument is NULL. Verified from code in the case of EMI & SMPP.
> 2) Choise to implicate dst in query depends on variable use_dst. This 
> could be configured/smsc, or to avoid unnecessary configurations, it could 
> be set from the smsc driver. I.e. always use dst on queries for EMI smscs, 
> never for others. Is this safe, or best to let user configure this?
>
> BR,
> Nikos
> <mysql.diff>