Hi Tom, Gordon
I fixed the columnDefinition problem as the attached fix. Please review
the fix.
The summary of fix:
FieldDefinition.java
* Add typeDefinition property which contains complete type definition
string like "VARCHAR2(50) UNIQUE NOT NULL".
* Clarify the meaning of typeName - generic database type name which is
translated based on platform.
* Modify appendDBString() to handle typeDefinition, typeName properly.
I removed FieldDefinition.appendTypeString
() method which was mostly duplicate code with appendDBString() and was
not being used anywhere. Let me know if this method is required.
TableDefinition.java
* Remove typeName -> type conversion code, which is now done in
FieldDefinition.appendDBString() while generating type definition.
Dynamic conversion is better way because it does not modify field
values of user-given FieldDefinition internally.
DefaultTableGenerator.java
* The column definition value is now set to typeDefinition property
instead of typeName of FieldDefinition.
I ran a local test which contains columnDefinition with MySQL and
entity-persistence-tests. The entity-persistence unit testsuite is
hitting 2 failures on Derby, but this is not the result of this fix as
I said in a separate e-mail.
Thanks,
-Wonseok
On 3/10/07, Gordon Yorke <gordon.yorke@oracle.com>
wrote:
Hello
Wonseok,
I'm not
sure if Tom responded to you or not I haven't received the email
through the persistence mailing list yet.
typeName
in TopLink is used as a place for a user to define definitions using
generic database types (well mostly generic). Those types are then
translated to a particular database type based on platform as you
mentioned. This code is from the original implementation of the
TopLink schema framework before ddl generation was written. I think
the issue here is that columnDefinition is being stored in the typeName
attribute. ColumnDefinition should really have its own property that
overrides both type and typeName.
--Gordon
Hi Tom,
I'm looking into issue 1454 columnDefinition="TEXT" does not apply in
table generation.
I'm fixing it, but I'm having problem in understanding the exact
meaning of typeName field in
FieldDefinition class.
DatabaseField has 'columnDefinition' field corresponding to
user-defined column definition, and which is set to FieldDefinition's
typeName(in this case FieldDefinition's type is null).
But as you see below code, the typeName is used to set its 'type' field
and it is set to null!
TableDefinition.java
/**
* INTERNAL:
* Build the field types based on the given name read in from the
file.
* Build the foriegn key constraints as well.
*/
protected void buildFieldTypes(AbstractSession session) {
buildForeignFieldTypes(session);
}
private void buildForeignFieldTypes(final AbstractSession session)
{
Hashtable fieldTypes = session.getPlatform().getClassTypes();
FieldDefinition field = null;
// The ForeignKeyConstraint object is the newer way of doing
things.
// We support FieldDefinition.getForeignKeyFieldName() due to
backwards compatibility
// by converting it. To allow mixing both ways, we just add
converted one to foreignKeys list.
for (Enumeration enumtr = getFields().elements();
enumtr.hasMoreElements();) {
field = (FieldDefinition)enumtr.nextElement();
if (field.getForeignKeyFieldName() != null) {
addForeignKeyConstraint(buildForeignKeyConstraint(field,
session.getPlatform()));
}
//FROM HERE - It's removing
typeName(columnDefinition)!
if ( field.getType()
== null) {
field.setType((Class)fieldTypes.get(field.getTypeName()));
if
(field.getType() != null) {
field.setTypeName(null);
}
}
}
}
This does not keep user-defined column definition. I thought the code
is wrong and removed it. With some other changes, I could see the
user-specified column definition is used properly.
But, I found that the code is used for another purpose in
entity-persistence-tests. EPT is directly using FieldDefinition to
generate tables like below.
FieldDefinition fieldSTREET = new FieldDefinition();
fieldSTREET.setName("STREET");
fieldSTREET.setTypeName("VARCHAR2");
fieldSTREET.setSize(60);
fieldSTREET.setSubSize(0);
fieldSTREET.setIsPrimaryKey(false);
fieldSTREET.setIsIdentity(false);
fieldSTREET.setUnique(false);
fieldSTREET.setShouldAllowNull(true);
table.addField(fieldSTREET);
As you know typeName "VARCHAR2" is only for Oracle, so it could not be
used for other databases. The above type-related code is converting
this 'typeName' to corresponding Java type which is converted to real
database type later according to DatabasePlatform.
What is the exact meaning of 'typeName'?
If it is type definition analogous to @Column.columnDefinition, then
tableCreators of EPT is abusing it - instead setType() should be used
to be portable.
If it should be translated according to DatabasePlatform, it should not
be used for @ Column.columnDefinition.
Could you explain?
Thanks
-Wonseok
Index: entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java
===================================================================
RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java,v
retrieving revision 1.14
diff -c -w -r1.14 DefaultTableGenerator.java
*** entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java 7 Mar 2007 02:26:57 -0000 1.14
--- entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/DefaultTableGenerator.java 12 Mar 2007 05:10:39 -0000
***************
*** 496,502 ****
if (dbField.getColumnDefinition().length() > 0) {
// This column definition would include the complete definition of the
// column like type, size, "NULL/NOT NULL" clause, unique key clause
! fieldDef.setTypeName(dbField.getColumnDefinition());
} else {
Class fieldType = dbField.getType();
--- 496,502 ----
if (dbField.getColumnDefinition().length() > 0) {
// This column definition would include the complete definition of the
// column like type, size, "NULL/NOT NULL" clause, unique key clause
! fieldDef.setTypeDefinition(dbField.getColumnDefinition());
} else {
Class fieldType = dbField.getType();
Index: entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/FieldDefinition.java
===================================================================
RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/FieldDefinition.java,v
retrieving revision 1.5
diff -c -w -r1.5 FieldDefinition.java
*** entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/FieldDefinition.java 4 Jan 2007 14:32:10 -0000 1.5
--- entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/FieldDefinition.java 12 Mar 2007 05:10:39 -0000
***************
*** 22,27 ****
--- 22,29 ----
package oracle.toplink.essentials.tools.schemaframework;
import java.io.*;
+ import java.util.Hashtable;
+
import oracle.toplink.essentials.internal.helper.*;
import oracle.toplink.essentials.internal.databaseaccess.*;
import oracle.toplink.essentials.exceptions.*;
***************
*** 40,47 ****
--- 42,62 ----
*/
public class FieldDefinition implements Serializable, Cloneable {
protected String name;
+ /**
+ * Java type class for the field.
+ * Particular database type is generated based on platform from this.
+ */
protected Class type;
+ /**
+ * Generic database type name for the field, which can be used instead of the Java class 'type'.
+ * This is translated to a particular database type based on platform.
+ */
protected String typeName;
+ /**
+ * Database-specific complete type definition like "VARCHAR2(50) UNIQUE NOT NULL".
+ * If this is given, other additional type constraint fields(size, unique, null) are meaningless.
+ */
+ protected String typeDefinition;
protected int size;
protected int subSize;
protected boolean shouldAllowNull;
***************
*** 100,117 ****
* Append the database field definition string to the table creation statement.
*/
public void appendDBString(Writer writer, AbstractSession session, TableDefinition table) throws ValidationException {
FieldTypeDefinition fieldType;
! if (getType() != null) {
fieldType = session.getPlatform().getFieldTypeDefinition(getType());
if (fieldType == null) {
throw ValidationException.javaTypeIsNotAValidDatabaseType(getType());
}
! } else {
fieldType = new FieldTypeDefinition(getTypeName());
}
! try {
! writer.write(getName());
! writer.write(" ");
String qualifiedName = table.getFullName() + '.' + getName();
session.getPlatform().printFieldTypeSize(writer, this, fieldType, session, qualifiedName);
session.getPlatform().printFieldUnique(writer, isUnique(), session, qualifiedName);
--- 115,152 ----
* Append the database field definition string to the table creation statement.
*/
public void appendDBString(Writer writer, AbstractSession session, TableDefinition table) throws ValidationException {
+ try {
+ writer.write(getName());
+ writer.write(" ");
+
+ if (getTypeDefinition() != null) { //apply user-defined complete type definition
+ writer.write(getTypeDefinition());
+
+ } else {
+ // compose type definition - type name, size, unique, identity, constraints...
FieldTypeDefinition fieldType;
!
! if (getType() != null) { //translate Java 'type'
fieldType = session.getPlatform().getFieldTypeDefinition(getType());
if (fieldType == null) {
throw ValidationException.javaTypeIsNotAValidDatabaseType(getType());
}
! } else if (getTypeName() != null) { //translate generic type name
! Hashtable fieldTypes = session.getPlatform().getClassTypes();
! Class type = (Class)fieldTypes.get(getTypeName());
! if (type == null) { // if unknown type name, use as it is
fieldType = new FieldTypeDefinition(getTypeName());
+ } else {
+ fieldType = session.getPlatform().getFieldTypeDefinition(type);
+ if (fieldType == null) {
+ throw ValidationException.javaTypeIsNotAValidDatabaseType(type);
}
! }
! } else {
! // both type and typeName is null
! throw ValidationException.javaTypeIsNotAValidDatabaseType(null);
! }
!
String qualifiedName = table.getFullName() + '.' + getName();
session.getPlatform().printFieldTypeSize(writer, this, fieldType, session, qualifiedName);
session.getPlatform().printFieldUnique(writer, isUnique(), session, qualifiedName);
***************
*** 130,177 ****
if (getAdditional() != null) {
writer.write(" " + getAdditional());
}
- } catch (IOException ioException) {
- throw ValidationException.fileError(ioException);
- }
- }
-
- /**
- * INTERNAL:
- * Append the database field definition string to the type creation statement.
- * Types do not support constraints.
- */
- public void appendTypeString(Writer writer, AbstractSession session) throws ValidationException {
- FieldTypeDefinition fieldType;
- if (getType() != null) {
- fieldType = session.getPlatform().getFieldTypeDefinition(getType());
- if (fieldType == null) {
- throw ValidationException.javaTypeIsNotAValidDatabaseType(getType());
- }
- } else {
- fieldType = new FieldTypeDefinition(getTypeName());
- }
- try {
- writer.write(getName());
- writer.write(" ");
- writer.write(fieldType.getName());
- if ((fieldType.isSizeAllowed()) && ((getSize() != 0) || (fieldType.isSizeRequired()))) {
- writer.write("(");
- if (getSize() == 0) {
- writer.write(new Integer(fieldType.getDefaultSize()).toString());
- } else {
- writer.write(new Integer(getSize()).toString());
- }
- if (getSubSize() != 0) {
- writer.write(",");
- writer.write(new Integer(getSubSize()).toString());
- } else if (fieldType.getDefaultSubSize() != 0) {
- writer.write(",");
- writer.write(new Integer(fieldType.getDefaultSubSize()).toString());
- }
- writer.write(")");
- }
- if (getAdditional() != null) {
- writer.write(" " + getAdditional());
}
} catch (IOException ioException) {
throw ValidationException.fileError(ioException);
--- 165,170 ----
***************
*** 251,258 ****
/**
* PUBLIC:
! * Return the type of the field.
! * This is the exact DB type name, which can be used instead of the Java class.
*/
public String getTypeName() {
return typeName;
--- 244,252 ----
/**
* PUBLIC:
! * Return the type name of the field.
! * This is the generic database type name, which can be used instead of the Java class 'type'.
! * This is translated to a particular database type based on platform.
*/
public String getTypeName() {
return typeName;
***************
*** 260,265 ****
--- 254,269 ----
/**
* PUBLIC:
+ * Return the type definition of the field.
+ * This is database-specific complete type definition like "VARCHAR2(50) UNIQUE NOT NULL".
+ * If this is given, other additional type constraint fields(size, unique, null) are meaningless.
+ */
+ public String getTypeDefinition() {
+ return typeDefinition;
+ }
+
+ /**
+ * PUBLIC:
* Answer whether the receiver is an identity field.
* Identity fields are Sybase specific,
* they insure that on insert a unique sequencial value is store in the row.
***************
*** 382,392 ****
/**
* PUBLIC:
! * Set the type of the field.
! * This is the exact DB type name, which can be used instead of the Java class.
*/
public void setTypeName(String typeName) {
this.typeName = typeName;
}
/**
--- 386,407 ----
/**
* PUBLIC:
! * Set the type name of the field.
! * This is the generic database type name, which can be used instead of the Java class 'type'.
! * This is translated to a particular database type based on platform.
*/
public void setTypeName(String typeName) {
this.typeName = typeName;
+ }
+
+ /**
+ * PUBLIC:
+ * Set the type definition of the field.
+ * This is database-specific complete type definition like "VARCHAR2(50) UNIQUE NOT NULL".
+ * If this is given, other additional type constraint fields(size, unique, null) are meaningless.
+ */
+ public void setTypeDefinition(String typeDefinition) {
+ this.typeDefinition = typeDefinition;
}
/**
Index: entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/TableDefinition.java
===================================================================
RCS file: /cvs/glassfish/entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/TableDefinition.java,v
retrieving revision 1.8
diff -c -w -r1.8 TableDefinition.java
*** entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/TableDefinition.java 4 Jan 2007 14:32:11 -0000 1.8
--- entity-persistence/src/java/oracle/toplink/essentials/tools/schemaframework/TableDefinition.java 12 Mar 2007 05:10:39 -0000
***************
*** 357,371 ****
/**
* INTERNAL:
! * Build the field types based on the given name read in from the file.
! * Build the foriegn key constraintsas well.
*/
protected void buildFieldTypes(AbstractSession session) {
- buildForeignFieldTypes(session);
- }
-
- private void buildForeignFieldTypes(final AbstractSession session) {
- Hashtable fieldTypes = session.getPlatform().getClassTypes();
FieldDefinition field = null;
// The ForeignKeyConstraint object is the newer way of doing things.
--- 357,365 ----
/**
* INTERNAL:
! * Build the foriegn key constraints.
*/
protected void buildFieldTypes(AbstractSession session) {
FieldDefinition field = null;
// The ForeignKeyConstraint object is the newer way of doing things.
***************
*** 376,388 ****
field = (FieldDefinition)enumtr.nextElement();
if (field.getForeignKeyFieldName() != null) {
addForeignKeyConstraint(buildForeignKeyConstraint(field, session.getPlatform()));
-
- }
- if (field.getType() == null) {
- field.setType((Class)fieldTypes.get(field.getTypeName()));
- if (field.getType() != null) {
- field.setTypeName(null);
- }
}
}
--- 370,375 ----