Apache JDO Project - TCK Schema : M-N relations

Author

TCK Schema : M-N relations

Andy Jefferson

2005-09-03, 5:45 pm

Schema for M-N unit tests has an odd foreign-key defined currently (certainly
for datastore identity) that causes issues. We have the following tables in
one of the M-N's

CREATE TABLE persons (
DATASTORE_IDENTITY INTEGER NOT NULL GENERATED ALWAYS AS IDENTITY,
PERSONID INTEGER UNIQUE NOT NULL,
...
CONSTRAINT EMPS_PK PRIMARY KEY (DATASTORE_IDENTITY)
)
CREATE TABLE projects (
DATASTORE_IDENTITY INTEGER NOT NULL GENERATED ALWAYS AS IDENTITY,
PROJID INTEGER UNIQUE NOT NULL,
...
CONSTRAINT PROJS_PK PRIMARY KEY (DATASTORE_IDENTITY)
);
with join table
CREATE TABLE project_reviewer (
PROJID INTEGER NOT NULL,
REVIEWER INTEGER NOT NULL
);

However there's an FK defined as
ALTER TABLE project_reviewer
ADD CONSTRAINT PR_PROJ_FK FOREIGN KEY
(PROJID) REFERENCES projects(PROJID);

so the FK goes from join table project id column *not* to the datastore
identity column in the "project" table but instead to a different column. That
won't work - the "DATASTORE_IDENTITY" values are set by the implementation,
whereas the PROJID is set by the TCK so likely won't concur! The definition
of <join> in the spec is that it goes between the *primary key columns* of
the primary table and the join table column(s). It should be mapped to the PK
of the "projects" table, so should go to "DATASTORE_IDENTITY" not "PROJID".
The same applies to the FK "PR_REV_FK" in the schema.

The join table "project_member" has no FK's defined on it in the schema
currently - so maybe that should have 2 FK's added.

The schema for the TCK also doesn't currently impose PK's on
any of its join tables. While the TCK tests will flush out any implementation
behaviour that is non-compliant, this could be aided by addition of PK's. For
example on the tables "project_reviewer", "project_member".

--
Andy

Craig Russell

2005-09-03, 5:45 pm

Andy Jefferson

2005-09-04, 2:45 am

Hi Craig,

> I'd be happy if you could propose a patch fixing the FK's.

patch is attached. Not raised a JIRA because in the time taken to raise the
JIRA somebody could just have applied the patch

--
Andy

Michael Bouschen

2005-09-04, 5:45 pm

Hi Andy,

thanks for the patch!

I think the changes also apply to the other datastoreidentity schema
files: schema[1-4].sql, since they have exactly the same problem. Am I
right? I have patched all 5 schema files in my workspace. I just want to
double check before I check in the changes.

Regards Michael

>Hi Craig,
>
>
>
>
>patch is attached. Not raised a JIRA because in the time taken to raise the
>JIRA somebody could just have applied the patch
>
>
>
>
>------------------------------------------------------------------------
>
>Index: test/sql/derby/datastoreidentity/schema.sql
> ========================================
===========================
>--- test/sql/derby/datastoreidentity/schema.sql (revision 267234)
>+++ test/sql/derby/datastoreidentity/schema.sql (working copy)
>@@ -167,12 +167,20 @@
>
> ALTER TABLE project_reviewer
> ADD CONSTRAINT PR_PROJ_FK FOREIGN KEY
>- (PROJID) REFERENCES projects(PROJID);
>+ (PROJID) REFERENCES projects(DATASTORE_IDENTITY);
>
> ALTER TABLE project_reviewer
> ADD CONSTRAINT PR_REV_FK FOREIGN KEY
>- (REVIEWER) REFERENCES persons(PERSONID);
>+ (REVIEWER) REFERENCES persons(DATASTORE_IDENTITY);
>
>+ALTER TABLE project_member
>+ ADD CONSTRAINT PM_PROJ_FK FOREIGN KEY
>+ (PROJID) REFERENCES projects(DATASTORE_IDENTITY);
>+
>+ALTER TABLE project_member
>+ ADD CONSTRAINT PM_MEMB_FK FOREIGN KEY
>+ (MEMBER) REFERENCES persons(DATASTORE_IDENTITY);
>+
> ALTER TABLE departments
> ADD CONSTRAINT EMP_MO_FK FOREIGN KEY
> (EMP_OF_THE_MONTH) REFERENCES persons(DATASTORE_IDENTITY);
>
>

--
Michael Bouschen Tech@Spree Engineering GmbH
mailto:mbo.tech@spree.de http://www.tech.spree.de/
Tel.:++49/30/235 520-33 Buelowstr. 66
Fax.:++49/30/2175 2012 D-10783 Berlin

Craig Russell

2005-09-04, 5:45 pm

Michael Bouschen

2005-09-04, 5:45 pm

Hi Craig,

> Hi Michael,
>
> I believe that the patch applies to the other
> datastoreidentityschemafiles as well.

Yes, this is what I think, too. I will check in the change for all
datastoreidentity schema files unless I hear any objections.

>
> Any idea why one join table uses this pattern:
>
> CREATE TABLE project_member (
> PROJID INTEGER REFERENCES projects NOT NULL,
> MEMBER INTEGER REFERENCES persons NOT NULL);
>
> and others use this pattern?
>
> CREATE TABLE project_reviewer (
> PROJID INTEGER NOT NULL,
> REVIEWER INTEGER NOT NULL
> );
> ALTER TABLE project_reviewer
> ADD CONSTRAINT PR_PROJ_FK FOREIGN KEY
> (PROJID) REFERENCES projects(PROJID);
>
> ALTER TABLE project_reviewer
> ADD CONSTRAINT PR_REV_FK FOREIGN KEY
> (REVIEWER) REFERENCES persons(PERSONID);
>
> I guess the latter allows us to name the foreign key so it's easier to
> delete them by name.

No I have no idea.

This is a good catch. I guess the first pattern creates a FK to the
column having the same name in the referenced table. But we want the FK
to reference a different column called DATASTORE_IDENTITY. Andys patch
adds ALTER TABLE definitions for table project_member, so we should skip
the REFERENCES clause from its definition:
CREATE TABLE project_member (
PROJID INTEGER NOT NULL,
MEMBER INTEGER NOT NULL
);

What do you think?

Regards Michael

>
> Craig
>
> On Sep 4, 2005, at 2:55 PM, Michael Bouschen wrote:
>
>
> Craig Russell
>
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
>
> 408 276-5638 mailto:Craig.Russell@sun.com
>
> P.S. A good JDO? O, Gasp!
>
>

--
Michael Bouschen Tech@Spree Engineering GmbH
mailto:mbo.tech@spree.de http://www.tech.spree.de/
Tel.:++49/30/235 520-33 Buelowstr. 66
Fax.:++49/30/2175 2012 D-10783 Berlin

Craig Russell

2005-09-04, 5:45 pm

Michael Bouschen

2005-09-04, 5:45 pm

Hi Craig,

you are right. I will change the project_member table as we discussed
and check in the changes tomorrow.

Regards Michael

> Hi Michael,
>
> You won't hear any objections from the U.S. Everyone is on vacation
> until Tuesday!
>
> So let's go ahead. We can always make minor changes later.
>
> On Sep 4, 2005, at 3:18 PM, Michael Bouschen wrote:
>
>
>
> I don't know for sure, but I don't think so. I think the first pattern
> creates an FK to the primary key column.
>
> Anyone have a SQL reference handy?
>
>
>
> Yes. Let's make the FK definitions explicit.
>
> Regards,
>
> Craig
>
>
> Craig Russell
>
> Architect, Sun Java Enterprise System http://java.sun.com/products/jdo
>
> 408 276-5638 mailto:Craig.Russell@sun.com
>
> P.S. A good JDO? O, Gasp!
>
>

--
Michael Bouschen Tech@Spree Engineering GmbH
mailto:mbo.tech@spree.de http://www.tech.spree.de/
Tel.:++49/30/235 520-33 Buelowstr. 66
Fax.:++49/30/2175 2012 D-10783 Berlin

Michelle Caisse

2005-09-05, 2:45 am

Hi Michael,

Michael Bouschen wrote:

> Hi Craig,
>
>
>
> Yes, this is what I think, too. I will check in the change for all
> datastoreidentity schema files unless I hear any objections.
>
>
I changed the table definitions and added the constraints in an ALTER
TABLE statement so that we would have named constraints for a test. I
don't remember which one now. That one form specifies the column list
and the other defaults it is whimsy.

>
> No I have no idea.
>
> This is a good catch. I guess the first pattern creates a FK to the
> column having the same name in the referenced table.

When the column list is omitted the referenced column(s) is(are) the
primary key of the referenced table.

> But we want the FK to reference a different column called
> DATASTORE_IDENTITY. Andys patch adds ALTER TABLE definitions for table
> project_member, so we should skip the REFERENCES clause from its
> definition:
> CREATE TABLE project_member (
> PROJID INTEGER NOT NULL,
> MEMBER INTEGER NOT NULL
> );
>
> What do you think?

Yup.

-- Michelle

>
> Regards Michael
>
>
>

Michael Bouschen

2005-09-05, 5:45 pm

Hi,

I checked in the changes (revision 278730). I included three changes wrt
Andys patch:
- I applied all the changes to schema[1-4].sql, too.
- I removed the foreign key definition from the CREATE TABLE statement
for project_member, since we have a separate ALTER TABLE defining the
foreign key.
- I changed all the REFERENCES clauses to not include an explicit column
specification such that it always refers the primary key, e.g. I changed
(EMP_OF_THE_MONTH) REFERENCES persons(DATASTORE_IDENTITY);
to
(EMP_OF_THE_MONTH) REFERENCES persons;
Most of the FK definitions used the latter form already, but not all of
them.

Regards Michael
[vbcol=seagreen]
> Hi Michael,
>
> Michael Bouschen wrote:
>
> I changed the table definitions and added the constraints in an ALTER
> TABLE statement so that we would have named constraints for a test. I
> don't remember which one now. That one form specifies the column list
> and the other defaults it is whimsy.
>
>
>
> When the column list is omitted the referenced column(s) is(are) the
> primary key of the referenced table.
>
>
>
> Yup.
>
> -- Michelle
>

--
Michael Bouschen Tech@Spree Engineering GmbH
mailto:mbo.tech@spree.de http://www.tech.spree.de/
Tel.:++49/30/235 520-33 Buelowstr. 66
Fax.:++49/30/2175 2012 D-10783 Berlin

Michael Watzek

2005-09-05, 5:45 pm

Hi Andy,

I agree with your bug description.

I wonder why JPOX did not throw an FK constraint violation after
makePersistentAll for "companyM-MRelationships". Actually, the test
fails comparing test data with inserted data because the
"reviewedProjects" collection is empty. However, in the JPOX log file
the FK constraint violation is logged. The following line appears 5 times:

11:11:31,046 (main) ERROR [JPOX.RDBMS.SQL] - Exception thrown while
executing SQL (INSERT INTO datastoreidentity0.PROJECT_REVIEWER
(REVIEWER,PROJID) VALUES (?,?) ) : INSERT on table 'PROJECT_REVIEWER'
caused a violation of foreign key constraint 'PR_PROJ_FK' for key (82).
The statement has been rolled back.

I would have expected that the test "companyAllRelationships" had the
same outcome as "companyM-MRelationships" wrt this issue. Confusingly,
the test threw an FK constraint violation for datastore identity.

So, what is the explanation for one test throwing an FK constraint
violation but not the other?

Regards,
Michael

> Schema for M-N unit tests has an odd foreign-key defined currently (certainly
> for datastore identity) that causes issues. We have the following tables in
> one of the M-N's
>
> CREATE TABLE persons (
> DATASTORE_IDENTITY INTEGER NOT NULL GENERATED ALWAYS AS IDENTITY,
> PERSONID INTEGER UNIQUE NOT NULL,
> ...
> CONSTRAINT EMPS_PK PRIMARY KEY (DATASTORE_IDENTITY)
> )
> CREATE TABLE projects (
> DATASTORE_IDENTITY INTEGER NOT NULL GENERATED ALWAYS AS IDENTITY,
> PROJID INTEGER UNIQUE NOT NULL,
> ...
> CONSTRAINT PROJS_PK PRIMARY KEY (DATASTORE_IDENTITY)
> );
> with join table
> CREATE TABLE project_reviewer (
> PROJID INTEGER NOT NULL,
> REVIEWER INTEGER NOT NULL
> );
>
> However there's an FK defined as
> ALTER TABLE project_reviewer
> ADD CONSTRAINT PR_PROJ_FK FOREIGN KEY
> (PROJID) REFERENCES projects(PROJID);
>
> so the FK goes from join table project id column *not* to the datastore
> identity column in the "project" table but instead to a different column. That
> won't work - the "DATASTORE_IDENTITY" values are set by the implementation,
> whereas the PROJID is set by the TCK so likely won't concur! The definition
> of <join> in the spec is that it goes between the *primary key columns* of
> the primary table and the join table column(s). It should be mapped to the PK
> of the "projects" table, so should go to "DATASTORE_IDENTITY" not "PROJID".
> The same applies to the FK "PR_REV_FK" in the schema.
>
> The join table "project_member" has no FK's defined on it in the schema
> currently - so maybe that should have 2 FK's added.
>
>
>
> The schema for the TCK also doesn't currently impose PK's on
> any of its join tables. While the TCK tests will flush out any implementation
> behaviour that is non-compliant, this could be aided by addition of PK's. For
> example on the tables "project_reviewer", "project_member".
>

--
-------------------------------------------------------------------
Michael Watzek Tech@Spree Engineering GmbH
mailto:mwa.tech@spree.de Buelowstr. 66
Tel.: ++49/30/235 520 36 10783 Berlin - Germany
Fax.: ++49/30/217 520 12 http://www.spree.de/
-------------------------------------------------------------------