| Summary: | ASTERISK-09798: [Realtime] Wrong Matching within extensions table | ||
| Reporter: | Michael E. Kromer (medozas) | Labels: | |
| Date Opened: | 2007-07-03 09:40:26 | Date Closed: | 2007-09-26 12:49:41 |
| Priority: | Major | Regression? | No |
| Status: | Closed/Complete | Components: | Addons/res_config_mysql |
| Versions: | Frequency of Occurrence | ||
| Related Issues: | |||
| Environment: | Attachments: | ( 0) sort-order-exten.diff | |
| Description: | First of all I'm sorry I can't supply any patches for this (as rather newbie to patch-management), but I will describe it so hopefully everybody understands what i mean. We have a setup asterisk here with realtime backend for both flat (cat_metric, var_metric, etc.) and dynamic (sippeers, iaxpeers, etc.) running successfully and stable. But: with in extconfig.conf setup "extensions => mysql,<DATABASE>,<EXTENSIONS-TABLE>" everything works ok actually, but the matching is incorrect. Example: Extension defined in extensions table: -> "_0." -> "_0174." The following happens: at line 255 of res_config_mysql.c (asterisk-addons package) and at line 250 of res/res_config_odbc.c (asterisk core) snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), " ORDER BY %s", initfield); all extensions are being sorted ASCENDING. This behaviour is wrong, as to limit Queries sent to database ARA hits the first match, and executes app/appdata with first found match. So even now calling 0174123 will first be matched with the _0. rule, even if _0174. is more precise. Please see query examples at "Additional Information" The easiest fix for this problem is to sort DESCENDING, as the order of matching now is the same as with flat file configuration (->best match). So changing above stated line to snprintf(sql + strlen(sql), sizeof(sql) - strlen(sql), " ORDER BY %s DESC", initfield); returns expected results. Now when 0174. is being called, it matches 0174. first, and only when different from 0174. pattern it goes to next _0. and fires that one successfully. This problem is only with pattern matching, as before making the pattern lookup realtime always tries to find the exact match of dialed number: [output already with patched version (DESC)] Jul 3 15:03:59 DEBUG[19101] res_config_mysql.c: MySQL RealTime: Retrieve SQL: SELECT * FROM config_extensions WHERE exten = '0174XXXXXXX' AND context = 'TERMINATION' AND priority = '1' Jul 3 15:03:59 DEBUG[19101] res_config_mysql.c: MySQL RealTime: Everything is fine. Jul 3 15:03:59 DEBUG[19101] res_config_mysql.c: MySQL RealTime: Retrieve SQL: SELECT * FROM config_extensions WHERE exten LIKE '\\_%' AND context = 'TERMINATION' AND priority = '1' ORDER BY exten DESC Jul 3 15:03:59 DEBUG[19101] res_config_mysql.c: MySQL RealTime: Everything is fine. As ARA always tries to lookup priority directly by number, it doesn't matter how the priority gets sorted anyways, and so this isn't a problem (AND priority = '1' / AND priority = '2', etc.)' Changing this line has no sideeffects as we have seen, as the flat config has own ordering (cat_metric/var_metric, etc.) and all other tables like sippeers or iaxpeers make no difference even if they would sort DESC. As said above we have this change running now on our LIVE installation and there is no difference in 100% functionality except now using correct matchind for dial patterns. Sorry if there might be something unclear, as this is my first bug-report with asterisk ;) ****** ADDITIONAL INFORMATION ****** Query with wrong sorting order: mysql> SELECT * FROM config_extensions WHERE context="TERMINATION" order by exten; +------+-------------+--------+----------+------------+--------------------------------+ | id | context | exten | priority | app | appdata | +------+-------------+--------+----------+------------+--------------------------------+ | 7502 | TERMINATION | _0. | 3 | Congestion | | | 7501 | TERMINATION | _0. | 2 | Busy | | | 7500 | TERMINATION | _0. | 1 | Dial | Zap/g1/01013${EXTEN}|60|tTr | | 7505 | TERMINATION | _0174. | 3 | Congestion | | | 7504 | TERMINATION | _0174. | 2 | Busy | | | 7503 | TERMINATION | _0174. | 1 | Dial | SIP/${EXTEN}@provider17|60|tTr | +------+-------------+--------+----------+------------+--------------------------------+ 6 rows in set (0.00 sec) --- Query with correct sorting order: mysql> SELECT * FROM config_extensions WHERE context="TERMINATION" order by exten DESC; +------+-------------+--------+----------+------------+--------------------------------+ | id | context | exten | priority | app | appdata | +------+-------------+--------+----------+------------+--------------------------------+ | 7505 | TERMINATION | _0174. | 3 | Congestion | | | 7504 | TERMINATION | _0174. | 2 | Busy | | | 7503 | TERMINATION | _0174. | 1 | Dial | SIP/${EXTEN}@provider17|60|tTr | | 7502 | TERMINATION | _0. | 3 | Congestion | | | 7501 | TERMINATION | _0. | 2 | Busy | | | 7500 | TERMINATION | _0. | 1 | Dial | Zap/g1/01013${EXTEN}|60|tTr | +------+-------------+--------+----------+------------+--------------------------------+ | ||
| Comments: | By: Jason Parker (jparker) 2007-07-03 09:47:05 _0. _0174. _017[1-5]. _017[6-9]. If the above is sorted descending, you would match _017[1-5]. and not _0174. - this is not an appropriate fix. By: Michael E. Kromer (medozas) 2007-07-03 09:58:33 right, i missed that one, sorry for that. although it hits more precise than at the moment. By: Michael E. Kromer (medozas) 2007-07-03 16:27:47 The diff uploaded represents a permanent and tested fix of the issue stated above. It does NOT match in descending way, as qwell (thanks for the note) correctly stated this doesn't make a "best match". It's using kind of quicksort-elimination to get the best match. The patch is against 1.2.19, and isn't hard to adapt to 1.2.20 or cvs (although not tested). Credits for this patch go to Jan Engelhardt <jengelh[at]computergmbh.de>. This code has been tested now in various ways and declared stable from our sides. Would be great to get response about this one. Would be nice to see this one in mainstream once. By: Jason Parker (jparker) 2007-07-03 16:33:25 Jan Engelhardt would also need a license agreement to be submitted. It would also be very nice if it were Jan that uploaded the patch. Our new license agreement system doesn't really like it when somebody uploads a patch somebody else wrote. By: Michael E. Kromer (medozas) 2007-07-03 16:44:03 Good to know ... as I have sent you the agreement as company and Jan Engelhardt as an employee also of our company, I thought this would be enough to do. Does he really need to create an extra account, or isn't it enough to agree with this as company? Should he resubmit the patch himself with an account created by himself or is it OK this way now? By: Michael E. Kromer (medozas) 2007-07-03 17:01:50 I think with this http://lists.digium.com/pipermail/asterisk-dev/2007-July/028391.html everything should be ok for now. By: Kaloyan Kovachev (knk) 2007-07-04 02:02:07 The best results will be provided if the sql query is sorted descending by length of the exten, not just exten i.e. SELECT * FROM config_extensions WHERE context="TERMINATION" order by LENGTH(exten) Desc; By: Michael E. Kromer (medozas) 2007-07-04 02:17:39 Sorting by length is not the solution 1. See Patch Provided 2. See Example Provided by qwell. If you would really sort by length of exten, the same would happen as with sorting descending. With the words of qwell: "you would match _017[1-5]. and not _0174. - this is not an appropriate fix." length: _0. 3 _0174. 6 _017[1-5]. 10 _017[6-9]. 10 (but doesnt match, just an example) By: Michael E. Kromer (medozas) 2007-08-23 10:43:57 Will this be included in asterisk's base sometime? Just wondering, as this is already some time ago, and we run this patch now successfully with our customers. By: Tilghman Lesher (tilghman) 2007-09-26 12:49:41 Sorry, it's nice that you got the behavior you were looking for, but it's not the behavior that Asterisk intentionally has and was documented to have. So I'm closing this bug as a "Won't fix". | ||