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