Summary:	ASTERISK-14180: [patch] MySQL ENUM Type Not Detected
Reporter:	Leo Brown (netfuse)	Labels:
Date Opened:	2009-05-21 07:45:05	Date Closed:	2010-10-06 17:46:53
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Addons/cdr_mysql
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) cdr_addon_mysql_enum.patch ( 1) cdr_addon_mysql-intenum.patch
Description:	The raw values for the AMAFLAGS column are detected by a switch case that checks all numeric types. Unfortunately ENUM is not in the list. ENUM is technically a numeric type. So if you have the column definition:  amaflags ENUM('DOCUMENTATION','BILLING','OMIT') This will work. However, if you have the definition:  amaflags ENUM('info','outbound','inbound') This will *not* work and the column will be set to NULL due to an unparsable value. By adding the ENUM column type instead, both syntaxes will work! ****** ADDITIONAL INFORMATION ****** Patch attached.
Comments:	By: Leo Brown (netfuse) 2009-07-24 08:50:00 Any thoughts on this patch? By: Tilghman Lesher (tilghman) 2009-08-20 15:59:00 While enum may be implemented internally as a numeric type, I am uncomfortable with this change.  Enumerated columns are supposed to enforce specific values, and this change subverts that.  For example, if the enum specifies the possible values out of order, then this change could cause the incorrect enumerated type to be used, when a number is inserted, and the corresponding column is picked. My vote on this is no. By: Leo Brown (netfuse) 2009-08-20 16:09:16 Hi, interesting point. I do disagree (hence the ticket!) that ENUM is only 'technically' an ENUM type. It's very common for people to implement ENUM and SET numerically. It's right up at the start of the relevant man page  http://dev.mysql.com/doc/refman/5.0/en/enum.html  "Each enumeration value has an index. Values from the list of allowable elements in the column specification are numbered beginning with 1." And from an implementation viewpoint, it makes a lot of sense. CDRs can be stored on remote systems but the limitation of the ENUM field as text means that the remote server HAS to call the values INFO, OMIT, and BILLING, which really only relate to very specific scenarios. In many implementations people use these as a kind of VERBOSE, INBOUND, OUTBOUND setup. Finally, from your point that "if the enum specifies the possible values out of order" there will be an issue, I really don't think this should be considered an issue. The CREATE TABLE syntax is given with the module and this is what is used. Why, if your point is valid, is there a test in the module for numeric types in the AMAFLAGS field? However, you do have the last word - but I can say that from an implementation PoV, this change makes a lot of sense, because the remote system can use this column more dynamically. Perhaps a global setting for MySQL CDR to provide this functionality would be sufficient. If you feel that is also uncalled for for some reason, then I happily rescind the patch... We can continue to use it internally only, but I thought others would benefit. By: Tilghman Lesher (tilghman) 2009-08-20 16:26:41 <i>Finally, from your point that "if the enum specifies the possible values out of order" there will be an issue, I really don't think this should be considered an issue. The CREATE TABLE syntax is given with the module and this is what is used.</i> It could be used, but it is not necessarily the only option. <i>Why, if your point is valid, is there a test in the module for numeric types in the AMAFLAGS field?</i> For legacy reasons; the amaflags was once specified by this backend as an integer field, and we want to remain compatible with that implementation. By: Leo Brown (netfuse) 2010-09-30 06:01:36 Hi I have patched this as a config option instead. Will you consider it for inclusion? Thanks LEo By: Tilghman Lesher (tilghman) 2010-10-06 13:10:41 Sorry, no.  It needs to detect the right mode of operation automatically.  Using an enum other than the default is equally invalid. By: Leo Brown (netfuse) 2010-10-06 13:21:26 Tilghman, I'm just trying to provide something that solves all cases. Since my patch includes a configuration option, I hoped this would meet both my case and the standard usage that you preferred. There is no way to automatically detect what column preference most users have. Many people who don't know SQL very well will have a CHAR(n) field to store the BILLING, OMIT etc, but they are wasting a lot of bytes in the CDR. Other users will apply the 0,1,2 mode of AMAFLAGS because they want to save space. The problem with this is that it is not very human-readable. The best solution is to use ENUM, and ENUM(BILLING, OMIT, DOCUMENTATION) would be fine, but with most people's systems they find that those three terms to not accurately describe three distinct CDR types. Hence, in my case, the use of Inbound, Outbound, and Info. I.e. toll-free billing, outbound billing, and unbilled inbound calls. The problem is that I can not have this elegant solution, because app_addon_cdr.so does not let me use MySQL's built-in "ENUM as INT" feature which was designed exactly to meet this need. Could you please provide me more information guiding me to produce a patch that reflects your interests and wishes, and then I can ensure that this is useful for someone. Thanks in advance, Leo By: Tilghman Lesher (tilghman) 2010-10-06 17:06:45 Retitling the fields might have worked, but it worked as a happy accident, not intentionally.  If the enum fields are reordered from the default, it might not work at all. The new way of autodetecting the right field types is much better for all involved, since it means less chance of screwing up the records. And an ENUM is NOT designed to retitle fields; it is designed to constrain the set of values.  That means your retitled field names are simply invalid. By: Leo Brown (netfuse) 2010-10-06 17:28:11 It's not a Happy Accident, this is core MySQL functionality provided as soon as Enum type was released! Check the manpage- most of the content is about Enum()'s Index functionality... http://www.mirrorservice.org/sites/ftp.mysql.com/doc/refman/5.5/en/enum.html By: Leo Brown (netfuse) 2010-10-06 17:29:33 Tilghman If you close this again, of course I'll leave it dormant, but I felt I must correct your "happy accident" theory! Thanks Leo