Of course you can submit your patch, we will have a look at it and it sounds like everything has been taken care of. Let us know when the patch was uploaded to a new JIRA issue.
Cheers, Francis On Tue, Jan 19, 2010 at 4:49 PM, McKinley <[email protected]> wrote: > I need to create a patch for multiple data type support for auto > increment for my projects this week and I would like it to be > acceptable if I submit it to Empire-db. I have now studied the code > and I think this will be very easy. I recommend the following. > > 1. AUTOINC is mostly used in the database drivers to set the datatype > to INT or LONG depending on the DBMS. It should stay there for now but > AUTOINC should be deprecated. Anyone who leaves their columns set to > AUTOINC will get the existing behavior. > > 2. An autoIncrement boolean member should be added to DBTableColumn > class along side the "protected DataType type" member. > > 3. isAutoIncrement() and setAutoIncrement() members will be added to > DBTableColumn. > > 4. The checks for AUTOINC in org.apache.empire.db and above are very > few. They will be moved to check isAutoIncrement() or autoIncrement > directly as approriate. > > 5. The drivers that call methods by pass DataType will need to be > overloaded to accept DBTableColumn. Anyone making use of the DataType > arguments on the methods directly will get the same results. I don't > know who might be using those methods outside the driver, but the > functionality will not change. The method overloads that accept > DBColumn will use the access to all column info to check the > getDataType() and isAutoIncrement() methods. The drivers internally > will use DBTableColumn arguments on these methods. Anyone who has > DECIMAL or UNKNOWN (for example, GUID on SQL Server) for the DataType > will get auto increment support if isAutoIncrement() resolves to true. > The check on DataType and isAutoIncrement can intersect properly for > SQL Server to provide the 'INDENTITY' DDL in the case of int and > decimal and not specify 'IDENTITY' in the case of uniqueidentifier. > That way, a column can always state its explicit data type and gain > all the type checks for that type, but also have the ability to be > left out of an INSERT statment and gain its new value upon update. > > 6. The DBTableColumn() will be overloaded to accept a autoincrement boolean. > > Thoughts? > > Thanks, > > McKinley > > > > On Mon, Jan 18, 2010 at 3:31 PM, McKinley <[email protected]> wrote: >> I am using AUTOINC columns pretty heavily. There is an assumptions >> made in Empire-db that does not match DBMS implementations. Auto >> Increment columns in a database are not necessarily of type int. >> >> In Empire-db the type check for AUTOINC usually looks like this: >> >> case AUTOINC: >> case INT: ... >> >> where it is considered to be INT or AUTOINC is not mentioned at all in >> which case the column is checked, set and get against that default. >> Some of the problems that I have had are that may columns in DBRecord >> are getting set to Java Integers when they should be Java Long. Also, >> I had a bug in some code that was applying a Java String to an AUTOINC >> column which did not parsed and checked to be numeric because there is >> no AUTOINC check in DBTableColumn.checkValue(). >> >> Because AUTOINC is not one actual data type, but really a different >> column attribute, can we separate it from DataType? The DataType can >> be set to INT, LONG or even binary and get all the correct behavior >> already. The checks for DataType.AUTOINC for determing read only >> fields can just check a different column attribute and not the >> DataType. The column definition in DBTable.addColumn() can have an >> overload for the new auto increment definition. >> >> Any solution that keeps AUTOINC in DataType will mean complicating >> DataType which is an enum now or it will mean adding duplicate AUTOINC >> types for every other type and duplicate case statements that will >> usually just fall through to the real type. >> >> What do you think? >> >> Thanks, >> >> McKinley >> > -- http://www.somatik.be Microsoft gives you windows, Linux gives you the whole house.
