Hi Wonsoek,
Lets just javadoc the behavior of
TableDefinition.addForeignKeyConstraint() as you suggest below.
-Tom
Wonseok Kim wrote:
> Tom,
> Thanks for reviewing it quickly.
>
> Please see my comments inline.
>
> On 11/4/06, *Tom Ware* <tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>> wrote:
>
> Hi Wonsoek,
>
> Just a couple of comments:
>
> - TableDefinition, line 120 - our coding convention requires braces
> around all blocks even if they are only one line
>
>
> Right, I will fix it. (I've been full aware of it, but missed at that
> line)
>
> - TableDefinition.addForeignKeyConstraint(), do you think we
> should warn
> users or throw an exception if they are adding a constraint that
> already
> exists?
>
>
> For bidirectional relationships each of ForeignReferenceMappings is
> processed and two same foreign key constraints are made. So
> TableDefinition.addForeignKeyConstraint() is called twice with same
> thing. In this case, we should not throw an exception.
> Logging a warning can leave unnecessary warnings in this case.
>
> I think there are other two options:
> 1. Add a javadoc like "Adding a same name foreign key constraint will
> be ignored".
> 2. Revert it back to Vector and allow adding same name foreign key
> constraint. Then database will throw exceptions when adding constraints.
> In this case, DefaultTableGenerator should check if same foreign key
> constraint exist before adding it.
>
> What do you think?
>
> -Wonseok
>
> Other than that, 'looks good.
>
> Thanks,
> Tom
>
>
> Wonseok Kim wrote:
>
> > Tom,
> > Please review my second fix.
> >
> > As your comments I modified addForeignKeyConstraint() to consider
> > candidate keys and tested the case with newly-added models.
> >
> > Here is the summary which I modified after the previous fix.
> >
> > * When making ForeignKeyConstraint the keys are reordered using
> > primary keys or candidate keys of the target table. If target
> keys are
> > incorrect(neither primary keys nor unique keys), it doesn't reorder
> > keys and underlying DatabaseException will be thrown.
> > * Fixed issue 1419 - Unique constraints should be made before
> foreign
> > key constraints. In the test of relationships using
> candidate(unique)
> > keys, I found that adding foreign key constraints failed because
> > unique constraints are made late.
> >
> > I fixed 1419 also because it's required to fix this(1072).
> >
> > New tests are added.
> > * New models and tests are in
> ~.testing.models.cmp3.ddlgeneration and
> > ~.testing.tests.ddlgeneration
> > * New models have several kinds of composite foreign key
> relationships
> > including one using candidate keys.
> >
> > All unit tests have passed.
> >
> > Thanks
> > -Wonseok
> >
> > On 11/3/06, *Tom Ware* <tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>> wrote:
> >
> > Hi Wonsoek,
> >
> > I think your mapping from Source to Target should work
> (with the
> > assumption that ddl generation is successful)
> >
> > I agree with your plan for getForeignKeys()/setForeignKeys().
> >
> > -Tom
> >
> > Wonseok Kim wrote:
> >
> > > Tom,
> > >
> > > I will try an experiment of ForeignReferenceMapping which
> > references
> > > unique keys.
> > > Probably, in the following code if CODE1, CODE2 are unique
> keys but
> > > not primary keys, it could be such a case.
> > > Will it work well in current runtime code (supposing ddl
> generation
> > > were successful)?
> > >
> > > public class Source {
> > >
> > > @OneToOne
> > > @JoinColumns({
> > > @JoinColumn(name="TARGET_CODE1",
> referencedColumnName =
> > "CODE1"),
> > > @JoinColumn(name="TARGET_CODE2",
> referencedColumnName =
> > "CODE2")
> > > })
> > > Target target;
> > > ...
> > > }
> > >
> > > @Table(name="TARGET",
> > > uniqueConstraints={_at_UniqueConstraint(columnNames={"CODE1",
> > "CODE2"})})
> > > public class Target {
> > > @Id
> > > Long id;
> > >
> > > @Column(name="CODE1", nullable = false)
> > > String code1;
> > >
> > > @Column(name="CODE2", nullable = false)
> > > String code2;
> > > ...
> > > }
> > >
> > > As to getForeignKeys()/setForeignKeys(), I will keep the
> > signature and
> > > add a javadoc which says it's PUBLIC.
> > > But I won't make new getForeignKeyMap()/getForeignKeyMap()
> methods
> > > public, because it exposes the internal structure and make
> it harder
> > > to change the structure later.
> > >
> > > Thanks
> > > -Wonseok
> > >
> > > On 11/2/06, *Tom Ware* < tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>
> > <mailto: tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
> > > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
> <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>>> wrote:
> > >
> > > Hi Wonsoek,
> > >
> > > Answer inline:
> > >
> > > Wonseok Kim wrote:
> > >
> > > > Hi Tom,
> > > > Please see my comments inline.
> > > >
> > > > On 11/2/06, *Tom Ware* <tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com> >
> > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com> <mailto:tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>>>
> > > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com> <mailto:tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
> <mailto: tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>>>> wrote:
> > > >
> > > > Hi Wonsoek,
> > > >
> > > > Answers inline.
> > > >
> > > > <snip>
> > > >
> > > > >
> > > > > - Will the addForeignKeyConstraint()
> method work if
> > > the foreign
> > > > > keys do
> > > > > not map exactly to the primary keys of the
> target
> > > > table? Will it work
> > > > > if they, instead map to some candidate
> keys, or if
> > > they map to
> > > > > only part
> > > > > of the primary key table? Perhaps I am
> missing
> > > something
> > > > here and I
> > > > > just need some explantation.
> > > > >
> > > > >
> > > > > Foreign key and primary key list come from
> > > > ForeignReferenceMapping. So
> > > > > I assumed that they are correct - fkFields map
> > exactly to
> > > > pkFields and
> > > > > pkFields contain all composite primary keys of
> the
> > target
> > > table.
> > > > >
> > > > > Is is possible that they are not? Is there any
> case that
> > > composite
> > > > > foreign keys do not map to the primary keys of
> taget
> > > table? Or the
> > > > > part of composite primary keys are referenced? I
> > couldn't
> > > think of
> > > > > such a case. Could you tell me an example if it's
> > possible?
> > > > >
> > > > > If a user specify incorrect join columns, it might
> > lead to
> > > such a
> > > > > situation. Validation may be required, but I
> think
> > schema
> > > > generation
> > > > > is not good place to do it, because in that case
> > > > > ForeignReferenceMappings are already invalid
> and the
> > > problem will
> > > > > occur in runtime too if schema generation is not
> > used. Without
> > > > > validation an incorrect ForeignKeyConstraint
> may cause
> > > underlying
> > > > > DatabaseException.
> > > > >
> > > > > What do you mean by candidate keys? The method
> will be
> > > called in
> > > > > postInitTableSchema() phase after
> initTableSchema()
> > construct
> > > > primary
> > > > > keys. So it assume that targetTableDefinition
> > contain complete
> > > > primary
> > > > > keys.
> > > >
> > > > Here's an example on Oracle. I am wondering if
> we can
> > support
> > > > creating
> > > > constraints on tables that are defined as follows?
> > > >
> > > > CREATE TABLE A_TABLE (ID INTEGER NOT NULL, B_NAME
> > > VARCHAR(20), B_TYPE
> > > > INTEGER, PRIMARY KEY (ID))
> > > >
> > > > CREATE TABLE B_TABLE (ID INTEGER NOT NULL, NAME
> > VARCHAR(20),
> > > TYPE
> > > > INTEGER, PRIMARY KEY (ID), CONSTRAINT NAME_TYPE
> UNIQUE
> > > (NAME, TYPE))
> > > >
> > > > In this case, the combination of NAME and TYPE is a
> > candidate
> > > > key. The
> > > > constraint would look something like the following:
> > > >
> > > > ALTER TABLE A_TABLE ADD CONSTRAINT B_CONSTRAINT
> > FOREIGN KEY
> > > (B_NAME,
> > > > B_TYPE) REFERENCES B_TABLE (NAME, TYPE)
> > > >
> > > > I realize that JPA probably will not currently
> create
> > the Unique
> > > > Constraint on TABLE_B, but I think it should be
> > possible to
> > > create it
> > > > using base-TopLink
> > > API. ( TableDefinition.addUniqueKeyConstraint ())
> > > >
> > > > Again, Perhaps I am missing something
> fundmental, but if
> > > possible I
> > > > would like to avoid disallowing creating tables of
> > that kind.
> > > >
> > > >
> > > > In that case, TableDefinition.addForeignKeyConstraint ()
> > can be
> > > used.
> > > > The method we are discussing is default foreign key
> > constraints
> > > > generator based on ForeignReferenceMappings. Can
> > > > ForeignReferenceMapping reference candidate keys instead
> > of primary
> > > > keys of the target entity?
> > >
> > > Yes, ForeignReferenceMapping subclasses (for instance
> > OneToOneMapping)
> > > can use either candidate keys or primary keys. As long as
> > your code
> > > change works with a ForeignReferenceMapping that uses
> > either, it
> > > should
> > > be ok.
> > >
> > > -Tom
> > >
> > > >
> > > > >
> > > > >
> > > > > > * Deprecated
> FieldDefinition.foreginKeyFieldName .
> > > > > > * When making ForeignKeyConstraint the key
> > order is
> > > > reordered as the
> > > > > > primary key order of target table - if the
> > order is
> > > different,
> > > > > > databases don't accept it.
> > > > > > * Changed the data structure of foreign
> keys in
> > > > TableDefinition
> > > > > from
> > > > > > Vector to Map to avoid same foreign key
> > constraint being
> > > > added -
> > > > > this
> > > > > > occurs for bidirectional relationships.
> > > > >
> > > > > I am concerned about this change. It does not
> > appear
> > > to be
> > > > completely
> > > > > implemented.
> TableDefinition.getForeignKeys () and
> > > > > TableDefinition.setForeignKeys () still
> operate on
> > > > vectors. I think we
> > > > > need to choose. Either this data
> structure is dealt
> > > with as a
> > > > > Vector or
> > > > > as a HashMap.
> > > > >
> > > > >
> > > > > First of all I would like to know whether
> > getForeignKeys() and
> > > > > setForeignKeys() are PUBLIC or INTERNAL method. I
> > thought
> > > they are
> > > > > PUBLIC methods so I didn't modify their signature
> > because
> > > they
> > > > may be
> > > > > being used by existing code. If they are INTERNAL
> > methods,
> > > I will
> > > > > modify their signature to use Map. How about
> others
> > such as
> > > > > get/setFields() and get/setUniqueKeys()? Are
> they all
> > > INTERNAL
> > > > methods?
> > > >
> > > >
> > > > Good point about the public use of these
> methods. Go
> > ahead
> > > and change
> > > > the methods as you have suggested.
> > > >
> > > >
> > > > Do you mean I can treat them as INTERNAL methods
> or not?
> > > Actually I
> > > > don't see any reason they are PUBLIC methods,
> because there is
> > > already
> > > > addForeignKeyConstraint(). If required, we can add
> "Collection
> > > > getForeignKeyConstraints()" for public getter.
> > >
> > > I think you should leave your code the way it was when
> you sent
> > > it. You
> > > made a good argument about these methods being accessible
> > for the
> > > public. Although there are APIs available that will
> accomplish
> > > the same
> > > thing, for backwards compatibility, I think it is
> better not
> > to change
> > > the signatures. Perhaps a comment that indicates why
> the setter
> > > takes a
> > > Vector and converts it to a map would be a good idea.
> > >
> > > We could add setForeignKeyConstraint(Map) and Map
> > > getForeignKeyConstraint(), but I am not sure they would
> > really get
> > > used.
> > >
> > > -Tom
> > >
> > > >
> > > > -Wonseok
> > > >
> > > > -Tom
> > > >
> > > > >
> > > > > Thanks,
> > > > > -Wonseok
> > > > >
> > > > > >
> > > > > > This contains also a fix for issue 1392
> - Wrong
> > > primary key
> > > > > constraint
> > > > > > if one-to-one and many-to-many relationships
> > have same
> > > > join column
> > > > > > name. It is included because it is
> simple fix.
> > See
> > > > > >
> DefaultTableGenerator.resolveDatabaseField().
> > > > > >
> > > https://glassfish.dev.java.net/issues/show_bug.cgi?id=1392
> > <https://glassfish.dev.java.net/issues/show_bug.cgi?id=1392>
> > > > > >
> > > > > > For tests,
> > > > > > I would like to add separate models for ddl
> > generation
> > > > purpose.
> > > > > > * Added a new models in new
> > > > > >
> > > >
> > oracle.toplink.essentials.testing.models.cmp3.ddlgeneration
> > > package.
> > > > > > * Added a new test
> DDLGenerationJUnitTestSuite
> > which
> > > now
> > > > contains a
> > > > > > test for issue 1392.
> > > > > > * DDL generation doesn't throw
> exceptions, so I
> > > checked logs
> > > > > manually.
> > > > > > * Tested single field foreign key
> constraint by
> > > running ddl
> > > > > generation
> > > > > > for entity-persistence-tests models.
> There was no
> > > failure
> > > > during ddl
> > > > > > generation.
> > > > > >
> > > > > > Updating TableCreators to use
> ForeignKeyConstraint
> > > object
> > > > instead of
> > > > > > deprecated foreignKeyFieldName will be
> done after
> > > review,
> > > > how about
> > > > > > making this another enhancement issue?
> > > > > >
> > > > > > Regards,
> > > > > > -Wonseok
> > > > > >
> > > > > > On 10/31/06, *Tom Ware* <
> tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
> > > <mailto:tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com> <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>>>
> > > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
> <mailto: tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>>>
> > > > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
> > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com> <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>>>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
> <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
> > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com> <mailto:tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>>>>>
> > > > > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
> > > <mailto:tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com> <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>>>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
> <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
> > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com> <mailto:tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>>>>
> > > > <mailto: tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>
> > <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>
> <mailto: tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
> > <mailto: tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>>
> > > <mailto:tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com> <mailto:tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>>
> > <mailto: tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
> <mailto:tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>>>>>>> wrote:
> > > > > >
> > > > > > Hi Wonseok,
> > > > > >
> > > > > > Sorry for not replying sooner. I
> was out of
> > > the office
> > > > > for the last
> > > > > > few days.
> > > > > >
> > > > > > First, a brief explanation:
> > > > > >
> > > > > > There are some historical reasons in
> > TopLink
> > > why both
> > > > > > foreignKeyFieldName and the
> > ForeignKeyConstraint
> > > object
> > > > > exist. The
> > > > > > ForeignKeyConstraintObject is the newer
> > way of doing
> > > > things,
> > > > > but
> > > > > > due to
> > > > > > backwards compatibility requirements
> we have
> > > chosen to
> > > > keep both
> > > > > > ways of
> > > > > > doing things. In the past, some of the
> > tooling
> > > that
> > > > shipped
> > > > > with
> > > > > > TopLink relied on the
> > foreignKeyFieldName. I am
> > > in the
> > > > > process of
> > > > > > determining if that is still the
> case and
> > I will
> > > let you
> > > > > know the
> > > > > > result.
> > > > > >
> > > > > > I think the ideal approach to this
> > problem is as
> > > > follows:
> > > > > >
> > > > > > 1. deprecate setForeignKeyFieldName and
> > > > > getForeignKeyFieldName on
> > > > > > FieldDefinition and update
> > DefaultTableGenerator to
> > > > make use of
> > > > > > ForeignKeyConstraint instead.
> > > > > > 2. Make the changes you suggest
> below to
> > allow both
> > > > types of
> > > > > foreign
> > > > > > keys to be used
> > > TableDefinition.buildForeignFieldTypes ().
> > > > > > 3. Update the
> entity-persistence-tests to
> > use the
> > > > non-deprecated
> > > > > > API. I
> > > > > > realize this is a bit tedious, but I
> think we
> > > need to make
> > > > > that change
> > > > > > in order to completely fix the issue.
> > > > > >
> > > > > > I assume there are more changes
> required
> > to fix
> > > > issue 1072.
> > > > > >
> > > > > > As for the use of Generics in the
> schema
> > > generation
> > > > code,
> > > > > the simple
> > > > > > answer is that you may now use
> generics in
> > that
> > > code. For
> > > > > those that
> > > > > > have been working on the TopLink
> > Essentials code
> > > for some
> > > > > time, a
> > > > > > bit of
> > > > > > further explanation is
> required. Until fairly
> > > > recently, we
> > > > > only
> > > > > > allowed
> > > > > > Java 5-specific code in a few of our
> packages
> > > due to a
> > > > > requirement to
> > > > > > support core TopLink functionality
> on JDK
> > 1.4. That
> > > > > requirement no
> > > > > > longer exists and as a result, Java
> 5 code
> > can now
> > > > appear in
> > > > > our core
> > > > > > packages.
> > > > > >
> > > > > > -Tom
> > > > > >
> > > > > > Wonseok Kim wrote:
> > > > > >
> > > > > > > TopLink Team, please shed light on
> this so I
> > > can go
> > > > ahead.
> > > > > > > I added some comments in the below,
> > > > > > >
> > > > > > > On 10/26/06, *Wonseok Kim* <
> > > guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>
> > > > <mailto:guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com> <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>>
> > <mailto: guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>>
> > > > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> > > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>
> > <mailto: guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> > > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>>>
> > > > > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> > > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com> <mailto:guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>>>>
> > > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> > <mailto: guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>
> > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com> <mailto:guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>> >>
> > > > > > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> > > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>
> > > > <mailto:guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com> <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>>
> > <mailto: guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>>
> > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com> <mailto:guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto: guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>
> > > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>
> > <mailto: guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>>>
> > > > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com> <mailto:guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>>>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> > > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>>
> > > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> > <mailto: guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>
> > > <mailto: guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com> <mailto:guruwons_at_gmail.com
> <mailto:guruwons_at_gmail.com>>
> > <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>
> <mailto:guruwons_at_gmail.com <mailto:guruwons_at_gmail.com>>>>>>>> wrote:
> > > > > > >
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > I'm trying to fix issue 1072 - ddl
> > > generation of
> > > > > composite
> > > > > > foreign
> > > > > > > key constraints.
> > > > > > > I think the ForeignKeyConstraints
> > class is
> > > the right
> > > > > one to
> > > > > > use if
> > > > > > > composite foreign key, but
> there is
> > another
> > > > foreign key
> > > > > > setting in
> > > > > > > FieldDefinition.
> > > > > > >
> > > > > > > public class FieldDefinition
> implements
> > > > Serializable,
> > > > > > Cloneable {
> > > > > > > ...
> > > > > > > protected String
> > > > > foreignKeyFieldName;//fully-qualified
> > > > > > foreign
> > > > > > > key field name
> > > > > > >
> > > > > > > But current code shows that if
> above
> > > > > foreignKeyFieldName is set,
> > > > > > > it ignores the
> > TableDefinition.foreignKeys
> > > > > > > (ForeignKeyConstraints). See
> below.
> > > > > > >
> > > > > > > TableDefinition:
> > > > > > > private void
> > buildForeignFieldTypes(final
> > > > > AbstractSession
> > > > > > > session) {
> > > > > > > Hashtable fieldTypes =
> > > > > > session.getPlatform().getClassTypes();
> > > > > > > FieldDefinition field
> = null;
> > > > > > >
> > > > > > > Vector foreignKeysClone =
> > > > > > > (Vector)getForeignKeys().clone();
> > > > > > > setForeignKeys(new
> Vector());
> > > > > > > for (Enumeration enumtr =
> > > > getFields().elements();
> > > > > > > enumtr.hasMoreElements ();) {
> > > > > > > field =
> > > > (FieldDefinition)enumtr.nextElement();
> > > > > > > if
> > > (field.getForeignKeyFieldName ()
> > > > != null) {
> > > > > > >
> > > > > > >
> > > > >
> > >
> addForeignKeyConstraint(buildForeignKeyConstraint(field, this,
> > > > > > > session.getPlatform()));
> > > > > > >
> > > > > > > }
> > > > > > > if (
> field.getType() ==
> > null) {
> > > > > > > field.setType
> > > > ((Class)fieldTypes.get(
> > > > > > > field.getTypeName ()));
> > > > > > > if (
> field.getType() !=
> > > null) {
> > > > > > >
> > field.setTypeName (null);
> > > > > > > }
> > > > > > > }
> > > > > > > }
> > > > > > >
> > > > > > > /* bug #2997188
> constraints not
> > > getting
> > > > > generated when
> > > > > > > creating/replacing tables via
> > TableCreator
> > > > > > > had to add the next
> few lines
> > > instead of
> > > > > removing the
> > > > > > > above code for backwards
> > compatibility */
> > > > > > > if
> > (getForeignKeys().isEmpty()) {
> > > > > > > //if foreignKeys is
> > empty then we
> > > > know we are
> > > > > > not in 2.5
> > > > > > >
> > setForeignKeys(foreignKeysClone);
> > > > > > > }
> > > > > > > }
> > > > > > >
> > > > > > > I'm curious, should we use either
> > > > foreignKeyFieldName or
> > > > > > > foreignKeys
> (ForeignKeyConstraints)?
> > We could
> > > > not mix
> > > > > both of
> > > > > > > them? Is there any reason for not
> > allowing
> > > this?
> > > > > > > entity-persistence-tests table
> > creators are
> > > > using only
> > > > > > > foreignKeyFieldName now, but
> If I would
> > > like to add
> > > > > composite
> > > > > > > foreign key constraints, how can I
> > mix them?
> > > > > > >
> > > > > > >
> > > > > > > If there is no problem, I would
> like to
> > > support for both
> > > > > > > ForeignKeyConstraint object and
> > > foreignKeyFieldName by
> > > > > removing
> > > > > > > setForeignKeys(new Vector()) line and
> > below lines.
> > > > > > >
> > > > > > > /* bug #2997188
> constraints not
> > getting
> > > > generated
> > > > > when
> > > > > > > creating/replacing tables via
> TableCreator
> > > > > > > had to add the next few
> lines
> > > instead of
> > > > > removing the
> > > > > > above
> > > > > > > code for backwards compatibility */
> > > > > > > if
> (getForeignKeys().isEmpty()) {
> > > > > > > //if foreignKeys is empty
> > then we
> > > know
> > > > we are
> > > > > not in 2.5
> > > > > > >
> > setForeignKeys(foreignKeysClone);
> > > > > > > }
> > > > > > >
> > > > > > > Then we can use both
> > > > FieldDefinition.foreignKeyFieldName
> > > > > (for simple
> > > > > > > foreign key) and
> addForeignKeyConstraint
> > (for
> > > composite
> > > > > foreign
> > > > > > key).
> > > > > > > What do you think?
> > > > > > >
> > > > > > > Also, I don't know there is any
> > policy about
> > > > code, but
> > > > > could
> > > > > > I use
> > > > > > > Java SE 5 Generics in schema
> generation
> > > codes? This
> > > > > makes us
> > > > > > avoid
> > > > > > > type-mismatch mistakes and easy to
> > read code.
> > > > > > >
> > > > > > > Regards,
> > > > > > > - Wonseok
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> >
> >
>
> --
> Tom Ware
> Principal Software Engineer
> Oracle Canada Inc.
>
> Direct: (613) 783-4598
> Email: tom.ware_at_oracle.com <mailto:tom.ware_at_oracle.com>
>
>
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com