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_at_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
>
> -----Original Message-----
> *From:* Wonseok Kim [mailto:guruwons_at_gmail.com]
> *Sent:* Friday, March 09, 2007 8:05 AM
> *To:* persistence_at_glassfish.dev.java.net
> *Subject:* Issue 1454 and FieldDefinition.typeName?
>
> 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
>
>