Hi Wonseok,
PR 381 is an issue that was fixed some time in the mid '90s when
TopLink was still owned by The Object People (2 companies ago), so I am
a little short on detail about the actual bug. Here is what I
understand about the isAggregateMapping check. We do not set the type
for AggregateMappings in ClassDescriptor because that job is meant to be
done in the Descriptor representing the actual aggregate object. Note:
AggregateDescriptors are treated differently by TopLink. The original
descriptor stored in the project is cloned for each time the Aggregate
is used in a mapping and that clone is the actual descriptor that is
used (instead of the initial instance)
My initial thoughts on your suggested fix is that although it would
likely work, it is more of a fix of the symptoms then a fix of root of
the problem.
I believe the real bug is that we do not get the correct value for the
type at initialization time and that the DDL generation simply shows
that bug.
If I were investigating this bug, I would see if it was possible to
get the type right during the initialization process. I would take a
look at how Aggregates are initialized and see if I could plug into that
process somehow.
-Tom
Wonseok Kim wrote:
> Hi Tom,
>
> I'm investigating the issue 1391 - Java2DB generates Wrong primary key
> type in JOINED strategy
> https://glassfish.dev.java.net/issues/show_bug.cgi?id=1391
> <https://glassfish.dev.java.net/issues/show_bug.cgi?id=1391>
>
> The root cause of this is that the 'type' value of DatabaseFields for
> subclass primary key fields is null.
> We don't call setType() for DatabaseFields which come from
> AggregateObjectMapping like below.
>
> ClassDescriptor:2155
> // Index and classify fields and primary key.
> // This is in post because it needs field classification defined
> in initializeMapping
> // this can come through a 1:1 so requires all descriptors to be
> initialized (mappings).
> // May 02, 2000 - Jon D.
> // Added mapping.isAggregateMapping() check for fix to pr 381
> for (int index = 0; index < getFields().size(); index++) {
> DatabaseField field =
> (DatabaseField)getFields().elementAt(index);
> DatabaseMapping mapping =
> getObjectBuilder().getMappingForField(field);
> if ((mapping != null) && (!(mapping.isAggregateMapping()))) {
> field.setType(mapping.getFieldClassification (field));
> }
> field.setIndex(index);
> }
>
> Of course, even if I comment "!(mapping.isAggregateMapping()" out, the
> type is still null for subclass primary key fields because
> AggregateObjectMapping.getFieldClassification (field) returns null for
> fields of subclass table. So I should take another approach.
>
> But I'm curious what "pr 381" is and why the type of
> AggregateObjectMapping fields should be remained null. Tom, could you
> check this?
>
> As you see below logs, if the type of DatabaseField is null it is
> defaulted to String in DDL generation and it produces this issue.
>
> [TopLink Finest]: 2006.12.06
> 09:50:57.746--Thread(Thread[main,5,main])--The default table generator
> could not locate or convert a java type (null) into a database type
> for database field (DDL_TARGET.ID2). The generator uses
> java.lang.String as default java type for the field.
> [TopLink Finest]: 2006.12.06
> 09:50:57.747--Thread(Thread[main,5,main])--The default table generator
> could not locate or convert a java type (null) into a database type
> for database field (DDL_TARGET.ID1). The generator uses
> java.lang.String as default java type for the field.
>
> My solution for this is setting the type of subclass primary key
> FieldDefinition same to the type of superclass pk FieldDefinition in
> DefaultTableGenerator.addJoinColumnsFkConstraint (). Similar thing is
> being done in addForeignKeyFieldToFieldDefinition() for relationship
> fields like below.
>
> FieldDefinition srcFldDef = (FieldDefinition)fieldMap.get(dbSrcField);
> FieldDefinition trgFldDef =
> (FieldDefinition)fieldMap.get(dbTrgField);
>
> if(srcFldDef != null && trgFldDef != null) {
> // Also ensure that the type, size and subsize of the foreign
> key field is
> // same as that of the original field.
> srcFldDef.setType (trgFldDef.getType());
> srcFldDef.setSize(trgFldDef.getSize());
> srcFldDef.setSubSize(trgFldDef.getSubSize());
> }
>
> It seems to work correctly. Do you see any problem in this approach?
>
> Regards,
> -Wonseok
>
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com