Hi all )
using Oracle, Liquibase 2.0 RC2 (trunk svn)
On this moment, data type for default value is a result of such statements like these:
public DataType getDataType(Object object) {
if (object instanceof BigInteger) {
return getBigIntType();
} else if (object instanceof Boolean) {
return getBooleanType();
} else if (object instanceof String) {
...
}
}
So we setting up value’s type without relations to column’s type and it follows us to wrong way of converting:
dataType.convertObjectToString(defaultValue, database)
For example:
SQL script generation process for updating db by changelog.
I have TIMESTAMP column. Oracle let me set default value SYSTIMESTAMP for this column, because it is an oracle getting-time function.
But liquibase gets it as a string-typed value and wrap it by quotes. After that it tries to set up this as default value for TIMESTAMP column and then throws Exception.
I think that it’s a wrong way of data type recognizing. So we should consider with column’s data type in this process.
To fix it I did (locally) these changes:
- I changed calling of getDataType(defaultValue) in CreateTableGenerator (line 67) to:
buffer.append(TypeConverterFactory.getInstance().findTypeConverter(database).getDataType(statement.getColumnTypes().get(column), isAutoIncrement).convertObjectToString(defaultValue, database));
So first i tries to get column's data type, and then i'm trying to use it's converting method.
2. I added "databaseFunctions" field and getter/setter methods in AbstractDatabase class.
// List of Database native functions.
protected List databaseFunctions;
After that I did initialization of this List of database functions in OracleDatabase constructor. For Oracle it looks like:
databaseFunctions = new ArrayList();
databaseFunctions.add(new DatabaseFunction("SYSDATE"));
databaseFunctions.add(new DatabaseFunction("SYSTIMESTAMP"));
3. And then I added one more "else if" statement to method convertObjectToString(Object value, Database database) in DateTimeType class:
else if (database.getDatabaseFunctions().contains(new DatabaseFunction(value.toString()))) {
return value.toString();
}
Am I right in my opinion and are my fixations correct?
I know that it may break another functional, that’s why I’m asking you for advice )
Fixing bug by this way shows me another bug (or it’s not a bug?)
There is no full implementation of VARCHAR2 oracle data-type.
If we using construction like this:
then liquibase (after my fixation) throws NumberFormatException, because it knows nothing about string-typed precision.
Any ideas about fixing it? Is it real or it’s only bug in my plagued brain? 
Should I correct it?
It is something that would need to be fixed. The trouble is that for 2.0 I added/enhanced validation on type parameters, but did not add code to handle units on those parameters. If you wanted to try finding a solution, that would be fine. Otherwise I can take a look at it too.
Nathan
I added to DataType class new field and getter/setter methods:
// Unit of data-type precision (i.e. BYTE, CHAR for Oracle)
private String unit;
Also I edited toString() method, so if getUnit() returns not-null value then this value will be append to result.
And finally, I overrides getDataType(String, Boolean, String, String) method in OracleTypeConverter class.
So, if we found nothing compatible among standard data-types then it tries to find correct data-type from native database types.
I tested it and in my case it works fine.
Thanks, that sounds good. I’ll take a look at the code to make sure it’s not causing any other troubles.
Nathan
I have been searching of data-type creation in code and found something strange.
There are two places where datatype’s precision may be set.
First, it is a DataType class. It has first/second precision’s parameter. And I think it’s right.
But also, precisions may be generate through column.getDataTypeString() method. Because it use something like that:
getDataType() + getColumnSize()
where ColumnSize is maximum that may be set in current database. It is a result of such query: resultSet.getInt(“COLUMN_SIZE”);
that’s why if column have no precisions - for example: NUMBER type - in changeLog it will converted to NUMBER(22,0);
This is minor priority issue. And I mess with it only in case of “build diff between original and clon schemas” trouble (for this I use another tools, not LB).
These diffs like NUMBER vs NUMBER(22,0) makes a lot of noise )) And it makes task “create diff” too difficult.
May I fix it? And try to replace uses of getColumnSize() by getting precision from DataType? Or it is a bad idea?
Why we are not using DataType class for definition of dataType field in Column class? Are there some reasons?
I have searching and debugging code to fix ‘NUMBER vs NUMBER(22,0)’ bug.
I tried to find some diffs between them.
And found that in both cases liquibase recognize data type as ‘22’ precision and ‘0’ scale counts.
The cause of it in rs.getInt() method.
Even value is null getInt(value) returns ‘0’.
My solution in using getString(“DECIMAL_DIGITS”) to check value - is it null or not. And if it really equals null then generate default number type without precisions. Only ‘NUMBER’.
if(rs.getString("DECIMAL_DIGITS") != null) {
//initialize precision and scale by rs.getInt() values.
}
I can’t explain how it acceptable for other kinds of databases - but it may be wrapped by database instanceof checking.
Tomorrow i’ll test this fix and if it’s work fine - i’m ready to commit it.
P.S.
But I still think that it’s better way to create DataType object on this stage instead of using int-value.
It is probably better to be creating and using DataType objects. The int type you get from the database jdbc metadata is probably too low-level for the Column class.
If your use of getString works better, I like that option. If you commit it, I can test it against some other databases I have installed.
Nathan
Unfortunately, i have fixed only NUMBER situation, when:
precision - default value
scale - default value
But also it can be set like:
precision - default value
scale - 0
And database will show it as NUMBER too… without (22,0)
X(
It’s seems it cant be recognized by using only databaseMetaData.getColumns()…
Do you have some idea or solution?
P.S. now I think that it is major bag…
I have found only one cure of it.
For example, by using such code in generator.readColumns() method:
if (database instanceof OracleDatabase) {
ResultSet integerListRS = selectStatement.executeQuery("select * from user_tab_columns where data_precision is null and data_scale = 0 and data_type = 'NUMBER'");
while (integerListRS.next()) {
integerList.add(integerListRS.getString("TABLE_NAME") + "." + integerListRS.getString("COLUMN_NAME"));
}
}
We just generate a list of all columns with INTEGER type. And then if we find that this list contains current column - sets it’s type as Types.Integer and TypeName as INTEGER.
And finally, it requires to edit parsing of type=“INTEGER” in Type Converter.
On this moment it runs to creation of “new IntType()”, but in Oracle it means only NUMBER(10,0)… So, It pushes me to add new type: IntegerType, with INTEGER signature…
But, it’s very awful solution… :-X
It will be more simple to find solutions by localizing all type-generation process in one place. I’ll think about it.
P.S.
Is it correct for other databases to add IntegerType?
P.P.S.
Still thinking of fix of it… ???
It seems I fixed it
P.S.
I didn’t add new IntegerType - it’s not compulsory. I just corrected getInt() method for Oracle implementation to return new Number(“INTEGER”);
Looks like a good fix. I’ve tried to abstract out a lot of that logic so it can be database specific. Your new configureColumnType method helps as well.
Nathan
