Hi,
IMO we don't need to handle no destination case in DLR lookup but maybe it's
not a wrong idea to be able to ignore destination for some
reasons when SMSC send some junk to us.
ok here is the review for your patch:
+ if (dst){
+ int len = octstr_len(dst);
+ char *p = octstr_get_cstr(dst);
+ if (len > MIN_DST_LEN)
+ p += len - MIN_DST_LEN; /* get last MIN_DST_LEN digits */
+ like = octstr_create(strcat("%", p));
+ gwlist_append(binds, like);
+ }
why is this in every dlr implementation? we have abstraction for this, see dlr.c
- sql = octstr_format("SELECT `%S`, `%S`, `%S`, `%S`, `%S`, `%S` FROM `%S`
WHERE `%S`=? AND `%S`=? LIMIT 1",
+ if (dst)
+ sql = octstr_format("SELECT `%S`, `%S`, `%S`, `%S`, `%S`, `%S` FROM
`%S` WHERE `%S`=? AND `%S`=? AND `%S` LIKE ? LIMIT 1",
fields->field_mask, fields->field_serv,
fields->field_url, fields->field_src,
fields->field_dst, fields->field_boxc,
fields->table, fields->field_smsc,
+ fields->field_ts, fields->field_dst);
...
First of all: like ? doesn't work as you expect... it should be something like:
LIKE CONCAT('%%', ?) and
this is too much maintenance for SQL that defined two times, how about like
this:
if (dst)
like = octstr_format('LIKE CONCAT('%%', ?)' ...);
else
like = octstr_create("");
sql = ...(".... %S", like)
The same is for DELETE, UPDATE...
+ if (like) octstr_destroy(like);
octstr_destroy will check for NULL for you...
+ if (octstr_compare(dlr->smsc, smsc) == 0 && octstr_compare(dlr->
+ timestamp, ts) == 0 && memcmp(p1 + len1 - size, p2 + len2 - size,
+ size) == 0)
+ return 0;
memcmp??? why not just use truncated destination and do: octstr_search???
+Msg *dlr_find(const Octstr *smsc, const Octstr *ts, const Octstr *dst, int
typ, int use_dst)
you don't need to change function. Just use dst = NULL and check it.
Thanks,
Alexander Malysh
Am 26.06.2010 um 10:23 schrieb Nikos Balkanas:
> Hi,
>
> I believe I have accounted for almost all your comments to produce the final
> working patch. I have no way of testing, other than compilation.
>
> This patch will povide:
>
> 1) Align dlr_oracle.c with the rest. Currently, Oracle does a full dst
> find/remove/update on each DLR.
> 2) Dst use on find/remove/update is controlled in dlr_find by a single
> variable, use_dst. This is currently set only at driver level and only in the
> emi driver, while everyone else has it false. However, very easily, if need
> arises, it can be set in smsc configuration.
> 3) All DLR handling for all smscs will remain as it used to be till now. Only
> emi handling changes, hopefully for the better (that is the purpose of the
> patch :-)).
> 4) It will try to match the last 7 digits of the destination or the length of
> the destination if it smaller. This is defined in gw/dlr_p.h as MIN_DST_LEN.
> Didn't want to make it larger, wanted to avoid prefix territory at all costs,
> since I have seen a lot of mangling there by the SMScs. Besides, I believe
> that 7 digits give enough resolution
> 5) People using emi, should rebuild their indeces, especially if they are
> running large batch jobs through EMI. The LIKE % construction is not very
> efficient.
>
> Enjoy,
> Nikos
> <kannel.diff>
