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