Hi Wonsoek,
I hope to check this in on Monday.
Thanks for all your work on this issue,
Tom
Wonseok Kim wrote:
> Tom,
>
> I modified TableDefinition to follow coding convention and added javadocs.
> Could you check in please, unless there are other comments?
>
> I will file an issue regarding Updating TableCreators to use
> ForeignKeyConstraint object instead of deprecated foreignKeyFieldName
> in entity-persistence-tests.
>
> Thanks for review!
> -Wonseok
>
> On 11/4/06, *Tom Ware* <tom.ware_at_oracle.com
> <mailto:tom.ware_at_oracle.com>> wrote:
>
> 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>
> > <mailto: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>>
> > > <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,
> > >
> > > 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>>>
> > > > <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,
> > > >
> > > > 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>>>>
> > > > > <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 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>
> > >
> <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>>>>>>
> > > > > > > <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 <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>>>>> >>
> > > > > > > > <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 <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>
> <mailto: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 <mailto:tom.ware_at_oracle.com>
>
>
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com