Refactor SequenceKeyGenerator (similar to IdentityKeyGenerator)

• • • Bug 1652 has been marked as a duplicate of this bug. ***

Now I'm in doubt, hope someone can answer my question:

Isn't it wrong to use rs.getInt(1) if the type is to be converted to BigDecimal
or Long?

In other words, correct usage is "new Long(rs.getLong(1))" and not "new
Long(rs.getInt(1))"?

> Isn't it wrong to use rs.getInt(1) if the type is to be converted to
BigDecimal > or Long?

Yes. When fetching a BigDecimal use rs.getBigDecimal() and when fetching a long
use rs.getLong().

> In other words, correct usage is "new Long(rs.getLong(1))" and not "new
> Long(rs.getInt(1))"?

Yes, correct.

Created an attachment (id=547)
Refactored SequenceGenerator

This is a refactored version of the SequenceKeyGenerator and it fixes the
getInt-bug.

Many things can be done with the keygenerators, but that task must be for
another bug.

They are for one thing not very effecient, the factory-classes are not doing
very much and the key-generators can be moved to a key-generator package.

I just had a look at all this ... great stuff. I have made some really minor
changes, extracting constant definitions from all the RDBMS names and moving
them to KeyGenerator, etc. Just small change, imho.

I think once we have verified that tests still work on a system that supports
sequences, we should go and commit this a.s.a.p.

I'm working on MaxGenerator too, but I'll wait to get this bug applied first...

Bruce, does postgreSQL have support for sequences ?

Why, yes it does. Please see the following:

http://www.postgresql.org/docs/current/static/sql-createsequence.html

Since you have PostgreSQL would you mind run the jdo-tests with the refactored
SequenceGenerator?

I havent changed the PostgreSQL-specific code tough...

Anybody out there who is running on an RDBMS with support for sequences, willing
to test this bug ?

Created an attachment (id=675)
Proposed patch (rest)

Ignore my last attachment, as it should have gone to bug 1655.

Bruce, any chances of runnnig this against postgreSQL ?

Sure. I just ran it against PostgreSQL version 7.4.2 using the PostgreSQL JDBC
driver pg74.214.jdbc3.jar. (I don't use the postgresql.jar in the Castor lib dir.)

Below are the results:

$ ./test.sh castor.postgresql

testsuite: castor
testsuite: postgresql
Test: TC01 Duplicate key detection .
Test: TC02 Concurrent access .
Test: TC05 Read only tests .
Test: TC06 Deadlock detection .
Test: TC07 Race .
Test: TC08 Cache leakage .
Test: TC09 Query garbage collected .
Test: TC10 Cache expiry measure .
Test: TC20 Type handling .E
Test: TC20b Type Conversion .
Test: TC22 Multiple Columns Primary Keys .
Test: TC23 ManyToMany .
Test: TC24 Dependent objects tests .
Test: TC25 Dependent update objects tests .
Test: TC24a Dependent objects tests .
Test: TC25a Dependent update objects tests .
Test: TC26 Lazy Loading .
Test: TC27 Nested fields .
Test: TC28 Collections .
Test: TC30 OQL queries for extends .
Test: TC31 Dependent object order using key-gen .
Test: TC31 ManyToManyKeyGen .
Test: TC32 Circular References using key-gen .
Test: TC41 Key generators: MAX, HIGH-LOW .
Test: TC45 Key generator: UUID .
Test: TC42 Key generator: SEQUENCE .
Test: TC47 Persistence interface tests .
Test: TC48 InstanceFactory interface tests .
Test: TC66 Size .F
Test: TC67 Absolute .
Test: TC60 TimeStampMismatched .CHANGING TIMESTAMP FROM: 0 TO: 1095655591504
CHANGING TIMESTAMP FROM: 0 TO: 1095655591518
GRABBING TIMESTAMP 1095655591518
CHANGING TIMESTAMP FROM: 0 TO: 1095655591518
GRABBING TIMESTAMP 1095655591518
CHANGING TIMESTAMP FROM: 1095655591518 TO: 1095655591771
GRABBING TIMESTAMP 1095655591771

Test: TC71 Expire Many-To-Many .
Test: TC72 Expire Lazy Employee .
Test: TC62 Test limit clause .
Time: 111.139
There was 1 error:
1) [castor.postgresql.TC20] Type handling
java.lang.ClassCastException
at org.exolab.castor.jdo.engine.SQLTypes$64.convert(SQLTypes.java:982)
at org.exolab.castor.jdo.engine.SQLEngine.toJava(SQLEngine.java:470)
at org.exolab.castor.jdo.engine.SQLEngine.store(SQLEngine.java:921)
at org.exolab.castor.persist.ClassMolder.store(ClassMolder.java:1613)
at org.exolab.castor.persist.LockEngine.store(LockEngine.java:753)
at
org.exolab.castor.persist.TransactionContext.prepare(TransactionContext.java:1556)
at org.exolab.castor.jdo.engine.DatabaseImpl.commit(DatabaseImpl.java:518)
at jdo.TypeHandling.testNullIntegerAndLong(TypeHandling.java:219)
at jdo.TypeHandling.runTest(TypeHandling.java:417)
at harness.CastorTestCase.run(CastorTestCase.java:154)
at harness.TestHarness.run(TestHarness.java:140)
at harness.TestHarness.run(TestHarness.java:140)
at Main.run(Main.java:152)
at Main.main(Main.java:226)
There was 1 failure:
1) [castor.postgresql.TC66] Size
junit.framework.AssertionFailedError: Calling size() on a non-scrollable
ResultSet should fail (unless using PostgreSQL).
at jdo.Size.testSizeD(Size.java:184)
at jdo.Size.runTest(Size.java:87)
at harness.CastorTestCase.run(CastorTestCase.java:154)
at harness.TestHarness.run(TestHarness.java:140)
at harness.TestHarness.run(TestHarness.java:140)
at Main.run(Main.java:152)
at Main.main(Main.java:226)

FAILURES!!!
Tests run: 34, Failures: 1, Errors: 1

AFAICR, TC66 should fail on PostgreSQL. I investigated this a while back and I
placed the statement in the test:

Calling size() on a non-scrollable ResultSet should fail (unless using PostgreSQL).

Thanks Bruce, I think I'll have to install PostgreSQL before working on this code...

I've been thinking of rewriting the keygenerator-api, decoupling from Castor,
but it's problematic since so many databases is involved.

We must be able to create new versions of the keygenerators without touching the
old and configuring in castor.properties what version to use.

I did have a look at the JDO2 spec, but I can't understand how they/that handle
if the key is generated before, during or after commit and this is the part that
is tightly coupled in Castor.

Any thoughts?

Stein, do you have any ideas how to progress this issue ? There's so many things mentioned in this bug report that I wonder wwhat should be done about them.

Updated patch against head but not tested yet.

Fixed an issue at IdentityKeyGenerator which apeared at tests against mysql.

Have to test this also against oracle to recognize problems with SequenzeKeyGenerator changes.

There seams to be a failure in SequenceKeyGenerator that causes a NullPointerException that needs to be fixed before commiting.

Fixed the remaining issues with sequence keygenerator at oracle. Patch is now ready to be committed.