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>> 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>>> 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
> > > >
> > > > 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>>>>> 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>>>>>> 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
> > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> >
> >
>