Destination is of type int(11)

[tombii]

Destination is of type int(11) while dialprefix is of type char(30). This breaks where dialprefix is larger than 2147483647 and the simulator shows the wrong destination.

[areski]

Why not using a bigint ?

[tombii]

Good point. Then change prefix to bigint as well so that it aligns with cc_prefix

[areski]

Your point is valid, but this hasn't been a problem until now, I believe those are edge cases where someone have prefix longer than 11chars. I would rather do small changes than big ones which may have side-effects if not well tested.

[tombii]

Ok, but one of my providers gives me prefix 5216621707 and that breaks. The simulator says South Africa since that's what I have for 2147483647 in cc_prefix.

[areski]

Yes but honestly 10 numbers for a prefix, we almost dont have room for a phonenumber anymore, we will soon charges individual depending to whom you are calling, but your point is valid the system should be more flexible and most of all consistent.

[tombii]

I agree but I import what my provider sends me since I don't want to have incorrect rates. It could lead to me over- or undercharging my clients and potential loss. I made the changes in my installation to test it out. That bigint could also be unsigned, I'll change that as well.

[tombii]

For some reason, after changing dialprefix to BIGINT, my simulator queries run much slower. CHAR seems to perform better although I cannot find any evidence to support that CHAR should be faster than BIGINT. Changing back cc_ratecard.dialprefixes to CHAR but keeping BIGINT for cc_ratecard.destination and cc_prefix.

[areski]

We need to validate the change or roll it back. Best way would be to explain the same query with CHAR and Bigint and compare time and indexes used: http://dev.mysql.com/doc/refman/5.7/en/explain.html

[areski]

Thanks for the patch update but this was already merged yesterday -> https://github.com/Star2Billing/a2billing/commit/271cc42a872c8ab1b41b1adb117474d53786d312

[tombii]

Thanks, that's good. I've been trying out the patch that was merged without any performance hits so that's good. Let's close this for now then.