Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.9.9 M1
    • Fix Version/s: 0.9.9 M1
    • Component/s: JDO
    • Labels:
      None
    • Environment:
      Operating System: Windows XP
      Platform: PC
    • Number of attachments :
      36

      Description

      Add support for polymorphism. Based upon a meeting by myself and Ralf back in
      January, this will be place where we keep our meeting notes, design decisions,
      etc. For complex discussions, I'll point to a confluence page once required.

      1. patch.2
        70 kB
        Werner Guttmann
      2. patch.20050621.txt
        176 kB
        Werner Guttmann
      3. patch.20050621-02.txt
        197 kB
        Werner Guttmann
      4. patch.20050622.txt
        197 kB
        Werner Guttmann
      5. patch.20050622-02.txt
        198 kB
        Werner Guttmann
      6. patch.20050623.txt
        205 kB
        Werner Guttmann
      7. patch.20050624-02.txt
        275 kB
        Werner Guttmann
      8. patch.20050701.txt
        290 kB
        Werner Guttmann
      9. patch.20050705.txt
        295 kB
        Werner Guttmann
      10. patch.20050705-02.txt
        308 kB
        Werner Guttmann
      11. patch.20050708.txt
        319 kB
        Werner Guttmann
      12. patch.3
        93 kB
        Werner Guttmann
      13. patch.examples.1
        90 kB
        Werner Guttmann
      14. patch.examples.2
        90 kB
        Werner Guttmann
      15. patch.examples.2
        42 kB
        Werner Guttmann
      16. patch.examples.3
        75 kB
        Werner Guttmann
      17. patch.main
        6 kB
        Werner Guttmann
      18. patch.main.2
        8 kB
        Werner Guttmann
      19. patch.main.3
        27 kB
        Werner Guttmann
      20. patch.main.4
        34 kB
        Werner Guttmann
      21. patch.main.5
        50 kB
        Werner Guttmann
      22. patch.main.8
        104 kB
        Werner Guttmann
      23. patch.tests.1
        11 kB
        Werner Guttmann
      24. patch.txt
        14 kB
        Werner Guttmann
      25. patch-1137.txt
        11 kB
        Ralf Joachim
      26. patch-1197.txt
        12 kB
        Ralf Joachim
      27. patch-1881-3.txt
        6 kB
        Ralf Joachim
      28. patch-1893.txt
        17 kB
        Ralf Joachim
      29. patch-C1018-20050624.txt
        227 kB
        Ralf Joachim
      30. patch-C1018-20050627.txt
        280 kB
        Ralf Joachim
      31. patch-C1018-20050628.txt
        280 kB
        Ralf Joachim
      32. patch-C1018-20050704.txt
        298 kB
        Ralf Joachim
      33. patch-C1018-20050715.txt
        324 kB
        Ralf Joachim
      34. ptf-results.20050622-02.txt
        13 kB
        Ralf Joachim
      35. TransactionContext-Rewrite.diff
        820 kB
        Gregory Block
      1. 100loads.jpg
        66 kB

        Issue Links

          Activity

          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=922)
          Initial test cases

          Please expand this patch into src/examples for simplicity sake .. . Whilst
          this is not complete, it shows where we shoudl be heading wrt to supporting
          polymorphism.

          Show
          Werner Guttmann added a comment - Created an attachment (id=922) Initial test cases Please expand this patch into src/examples for simplicity sake .. . Whilst this is not complete, it shows where we shoudl be heading wrt to supporting polymorphism.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=926)
          Initial (documenting) patch for src/main

          • Adds several attributes/methods to ClassDescriptorImpl so that a class
            descriptor stores information about which class it extends and by what
            classes it's being extended.
          • Adds code (debug statements) to SQLEngine.buildFinder() to travers extendedBy
            relations as well, and insert left outer joins (in form of log.debug()
            statements) for the classes found.
          Show
          Werner Guttmann added a comment - Created an attachment (id=926) Initial (documenting) patch for src/main Adds several attributes/methods to ClassDescriptorImpl so that a class descriptor stores information about which class it extends and by what classes it's being extended. Adds code (debug statements) to SQLEngine.buildFinder() to travers extendedBy relations as well, and insert left outer joins (in form of log.debug() statements) for the classes found.
          Hide
          Ralf Joachim added a comment -

          RFC: Polymorphisum support for castor
          (Result of skiing meeting Januar 2005)

          Example mapping
          ===============

          <class name="myapp.Product" identity="id">
          <description>Product definition</description>
          <map-to table="prod"/>
          <field name="id" type="integer">
          <sql name="id" type="integer" />
          </field>
          <field name="name" type="string">
          <sql name="name" type="char" />
          </field>
          <field name="price" type="float">
          <sql name="price" type="numeric" />
          </field>
          </class>

          <class name="myapp.Computer" extends="myapp.Product" identity="id">
          <description>Computer definition, extends product</description>
          <map-to table="comp"/>
          <field name="id" type="integer">
          <sql name="id" type="integer" />
          </field>
          <field name="cpu" type="string">
          <sql name="cpu" type="char"/>
          </field>
          </class>

          <class name="myapp.Notebook" extends="myapp.Computer" identity="id">
          <description>Notebook definition, extends Computer</description>
          <map-to table="nb"/>
          <field name="id" type="integer">
          <sql name="id" type="integer" />
          </field>
          <field name="weight" type="float">
          <sql name="weight" type="numeric"/>
          </field>
          </class>

          Basic concept
          =============

          For classes without extends their classname together with their identity
          are used for unique identification of the object. If the class extends
          another we should use the classname of the extended class together with
          the identity. If the extended class also extends another the previous
          rule needs to be applied recursively until the top class of the extend.
          For above example classes this means:

          original class | top class
          ----------------+---------------
          myapp.Product | myapp.Product
          myapp.Computer | myapp.Product
          myapp.Notebook | myapp.Product

          The unique identification of the object will be used as key for caching.

          Database operations
          ===================

          1. db.load(Class, Object)
          -------------------------

          a. Cache lookup
          When using unique identification from above the object can be retrieved
          from cache independed if its a top, intermediate or leaf of the extend.

          b. Retrieve object from persistent storage
          If object is not in cache it should be retrieved from persistent storage
          by the sql query:

          select prod.id, prod.name, prod.price, comp.id, comp.cpu, nb.id, nb.weight
          from ((prod
          left join comp on prod.id=comp.id)
          left join nb on prod.id=nb.id)
          where (prod.id=$)

          c. Object creation
          Dependend on the returned id columns we can decide which object needs to
          be constructed:

          prod.id | comp.id | nb.id | class
          ---------------------+----------------
          id | null | null | myapp.Product
          id | id | null | myapp.Computer
          id | id | id | myapp.Notebook

          Other id combinations are invalid and should throw an exception.

          d. Result
          If object is loaded by:

          db.load(myapp.Product.class, new Integer(1));

          the object is always valid and returned as is, but when loaded by:

          db.load(myapp.Notebook.class, new Integer(1));

          we should throw an exception (ClassCast) if the object is not a Notebook.

          2. db.delete(Object)
          --------------------

          An object that extends another needs to be deleted in all tables begining
          at the table storing the leaf (Notebook) to the top (Product) recursively.

          3. db.create(Object)
          --------------------

          An object that extends another needs to be created in all tables begining
          at the table storing the top (Product) to the leaf (Notebook) recursively.

          4. db.execute(OQLQuery)
          -----------------------

          The oql query:

          select p from myapp.Product p where p.price>1000

          need to be transformed to the sql query:

          select prod.id, prod.name, prod.price, comp.id, comp.cpu, nb.id, nb.weight
          from ((prod
          left join comp on prod.id=comp.id)
          left join nb on prod.id=nb.id)
          where (prod.price>1000)

          But with the oql query:

          select c from myapp.Computer c where c.price>1000

          the join needs to be changed from left outer to inner:

          select prod.id, prod.name, prod.price, comp.id, comp.cpu, nb.id, nb.weight
          from ((prod
          inner join comp on prod.id=comp.id)
          left join nb on prod.id=nb.id)
          where (prod.price>1000)

          For a oql query of Notebook's the second join needs to be changed
          respectively. The object creation is similar to 1c.

          Refering to an extended object with 1 to 1 relation
          ===================================================

          To be defined out of handwriten concept paper.

          Refering to an extended object with 1 to many relation
          ======================================================

          To be defined out of handwriten concept paper.

          Show
          Ralf Joachim added a comment - RFC: Polymorphisum support for castor (Result of skiing meeting Januar 2005) Example mapping =============== <class name="myapp.Product" identity="id"> <description>Product definition</description> <map-to table="prod"/> <field name="id" type="integer"> <sql name="id" type="integer" /> </field> <field name="name" type="string"> <sql name="name" type="char" /> </field> <field name="price" type="float"> <sql name="price" type="numeric" /> </field> </class> <class name="myapp.Computer" extends="myapp.Product" identity="id"> <description>Computer definition, extends product</description> <map-to table="comp"/> <field name="id" type="integer"> <sql name="id" type="integer" /> </field> <field name="cpu" type="string"> <sql name="cpu" type="char"/> </field> </class> <class name="myapp.Notebook" extends="myapp.Computer" identity="id"> <description>Notebook definition, extends Computer</description> <map-to table="nb"/> <field name="id" type="integer"> <sql name="id" type="integer" /> </field> <field name="weight" type="float"> <sql name="weight" type="numeric"/> </field> </class> Basic concept ============= For classes without extends their classname together with their identity are used for unique identification of the object. If the class extends another we should use the classname of the extended class together with the identity. If the extended class also extends another the previous rule needs to be applied recursively until the top class of the extend. For above example classes this means: original class | top class ---------------- + --------------- myapp.Product | myapp.Product myapp.Computer | myapp.Product myapp.Notebook | myapp.Product The unique identification of the object will be used as key for caching. Database operations =================== 1. db.load(Class, Object) ------------------------- a. Cache lookup When using unique identification from above the object can be retrieved from cache independed if its a top, intermediate or leaf of the extend. b. Retrieve object from persistent storage If object is not in cache it should be retrieved from persistent storage by the sql query: select prod.id, prod.name, prod.price, comp.id, comp.cpu, nb.id, nb.weight from ((prod left join comp on prod.id=comp.id) left join nb on prod.id=nb.id) where (prod.id=$) c. Object creation Dependend on the returned id columns we can decide which object needs to be constructed: prod.id | comp.id | nb.id | class ------- ------- ------- + ---------------- id | null | null | myapp.Product id | id | null | myapp.Computer id | id | id | myapp.Notebook Other id combinations are invalid and should throw an exception. d. Result If object is loaded by: db.load(myapp.Product.class, new Integer(1)); the object is always valid and returned as is, but when loaded by: db.load(myapp.Notebook.class, new Integer(1)); we should throw an exception (ClassCast) if the object is not a Notebook. 2. db.delete(Object) -------------------- An object that extends another needs to be deleted in all tables begining at the table storing the leaf (Notebook) to the top (Product) recursively. 3. db.create(Object) -------------------- An object that extends another needs to be created in all tables begining at the table storing the top (Product) to the leaf (Notebook) recursively. 4. db.execute(OQLQuery) ----------------------- The oql query: select p from myapp.Product p where p.price>1000 need to be transformed to the sql query: select prod.id, prod.name, prod.price, comp.id, comp.cpu, nb.id, nb.weight from ((prod left join comp on prod.id=comp.id) left join nb on prod.id=nb.id) where (prod.price>1000) But with the oql query: select c from myapp.Computer c where c.price>1000 the join needs to be changed from left outer to inner: select prod.id, prod.name, prod.price, comp.id, comp.cpu, nb.id, nb.weight from ((prod inner join comp on prod.id=comp.id) left join nb on prod.id=nb.id) where (prod.price>1000) For a oql query of Notebook's the second join needs to be changed respectively. The object creation is similar to 1c. Refering to an extended object with 1 to 1 relation =================================================== To be defined out of handwriten concept paper. Refering to an extended object with 1 to many relation ====================================================== To be defined out of handwriten concept paper.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=927)
          Initial patch fro src/main

          Added some more code to SQLEngine.buildFinder, using addOuterJoin() for the
          first time. Still undecisive what to do about the fields required.

          Show
          Werner Guttmann added a comment - Created an attachment (id=927) Initial patch fro src/main Added some more code to SQLEngine.buildFinder, using addOuterJoin() for the first time. Still undecisive what to do about the fields required.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=928)
          Updated patch

          This new patch includes test cases as well source code modifications. I've
          added Car and Truck classes (incl. mapping definitions), and two test methods
          to load a Truck instance and a Car instance that happens to be a Truck
          instance.

          I have extended SQLEngine.buildFinder() to use LEFT OUTER JOINs for all classes
          that extend a given class to be included in the SQL generated. Have a look, and
          let me know what you think.

          To do:

          • Modify SQLEngine.load() to analyse the ResultSet returned by
            Statement.execute() for any 'occurrence' of extending entities, and as a result
            try to load the correct class, i.e. a leaf of the class hierarchy.
          Show
          Werner Guttmann added a comment - Created an attachment (id=928) Updated patch This new patch includes test cases as well source code modifications. I've added Car and Truck classes (incl. mapping definitions), and two test methods to load a Truck instance and a Car instance that happens to be a Truck instance. I have extended SQLEngine.buildFinder() to use LEFT OUTER JOINs for all classes that extend a given class to be included in the SQL generated. Have a look, and let me know what you think. To do: Modify SQLEngine.load() to analyse the ResultSet returned by Statement.execute() for any 'occurrence' of extending entities, and as a result try to load the correct class, i.e. a leaf of the class hierarchy.
          Hide
          Werner Guttmann added a comment -

          Here's my currently biggest problem: the SQL statement we use for loading a
          class only loads non-identity fields. In the case, where we are loading a class
          that is part of an extend relation, this is not sufficient, as any such field
          can be declared as 'nullable'. In other words, we need to find a way to include
          the identity fields as well, at least of the 'extending' classes. Before trying
          to add the relevant code, I'd like to hear opinions.

          Show
          Werner Guttmann added a comment - Here's my currently biggest problem: the SQL statement we use for loading a class only loads non-identity fields. In the case, where we are loading a class that is part of an extend relation, this is not sufficient, as any such field can be declared as 'nullable'. In other words, we need to find a way to include the identity fields as well, at least of the 'extending' classes. Before trying to add the relevant code, I'd like to hear opinions.
          Hide
          Werner Guttmann added a comment -

          Is there a particular reason why we are using

          ResultSet.getXXX (int)

          rather than

          ResultSet.getXXX (String)

          using the column name as stored in FieldInfo.ColumnInfo ? If we were to use the
          latter approach, add ing the code described above (to handle the inclusion of
          identity fields into the load statement) would be quite easy.

          Show
          Werner Guttmann added a comment - Is there a particular reason why we are using ResultSet.getXXX (int) rather than ResultSet.getXXX (String) using the column name as stored in FieldInfo.ColumnInfo ? If we were to use the latter approach, add ing the code described above (to handle the inclusion of identity fields into the load statement) would be quite easy.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=929)
          Updated patch (2)

          I have added code to SQLEngine.buildFinder() to include identity columns of
          extending classes/tables as well. For this, I had to add the ColumnInfo
          interface to Persistence.java and some other minor things.
          I have added code to SQLEngine.load() to analyse the complete set of
          (extending) ids for entries that are not null. As we ascend the hierarchy of
          potential candidates (towards a leaf), we memorize the last JDO class
          descriptor where the value of its id was not null. This holds (most likely) the
          type of the java class we actually need to load.

          Show
          Werner Guttmann added a comment - Created an attachment (id=929) Updated patch (2) I have added code to SQLEngine.buildFinder() to include identity columns of extending classes/tables as well. For this, I had to add the ColumnInfo interface to Persistence.java and some other minor things. I have added code to SQLEngine.load() to analyse the complete set of (extending) ids for entries that are not null. As we ascend the hierarchy of potential candidates (towards a leaf), we memorize the last JDO class descriptor where the value of its id was not null. This holds (most likely) the type of the java class we actually need to load.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=931)
          Updated patch (3)

          Added code to SQLEngine.load() to implement loading the actual 'leaf' object of
          the extends hierarchy identified by the id value provided. For this, I slightly
          had to change the signature of the load() method to allow changing the size of
          the Object array.

          Note: The code added at this state is admittedly quite ugly, and has some
          serious flaws with regards to compound identities. Iow, if you feel like trying
          things out, please stick with the samples provided .. .

          Show
          Werner Guttmann added a comment - Created an attachment (id=931) Updated patch (3) Added code to SQLEngine.load() to implement loading the actual 'leaf' object of the extends hierarchy identified by the id value provided. For this, I slightly had to change the signature of the load() method to allow changing the size of the Object array. Note: The code added at this state is admittedly quite ugly, and has some serious flaws with regards to compound identities. Iow, if you feel like trying things out, please stick with the samples provided .. .
          Hide
          Werner Guttmann added a comment -

          I had some more 'looks' at SQLEngineload and its calling stack, namely
          ClassMolder.load() and TransactionContext.load(). As it stands right now,
          TransactionContext.load() creates an instance of the requested class using e.g.
          ClassMolder.newInstance(), and then passes this object down the call stack.

          Here's my approach: by basically creating an 'enclosing structure' to pass down
          the object created (and potentially more information), I'll be able to 're-
          create' this object instance as required further downstream. As long as this
          object extends the requested object type, this should not be a problem.

          I'll provide a patch soon, but would appreciate any feedback already now .. .

          Show
          Werner Guttmann added a comment - I had some more 'looks' at SQLEngineload and its calling stack, namely ClassMolder.load() and TransactionContext.load(). As it stands right now, TransactionContext.load() creates an instance of the requested class using e.g. ClassMolder.newInstance(), and then passes this object down the call stack. Here's my approach: by basically creating an 'enclosing structure' to pass down the object created (and potentially more information), I'll be able to 're- create' this object instance as required further downstream. As long as this object extends the requested object type, this should not be a problem. I'll provide a patch soon, but would appreciate any feedback already now .. .
          Hide
          Ralf Joachim added a comment -

          Hi Werner, had a look at your problem this evening but got another idea how this
          may be handled.

          1. Object to load is created in TransactionContext.load() <688-701>.

          — begin snap —

          if ( objectToBeLoaded != null )
          object = objectToBeLoaded;
          else {
          if ( _instanceFactory != null )

          { object = _instanceFactory.newInstance( molder.getName(), _db.getClassLoader() ); }

          else

          { object = molder.newInstance( _db.getClassLoader() ); }

          }
          entry = addObjectEntry( oid, object );
          oid = engine.load( this, oid, object, suggestedAccessMode, _lockTimeout, results );

          // rehash the object entry, because oid might have changed!
          entry = rehash( object, oid );

          — end snap —

          Move this code from TransactionContext.load() over LockEngine.load() to
          ClassMolder.load(). I am not sure this will be possible but we should give it a try.

          2. Also move your new code to find the extend class from SQLEngine.load()
          <1105-1137> to ClassMolder.load(). This also seams to be possible and if both
          works you are able to create to right object for the first time.

          Hope this will help and not confuse you as its a difficult to describe in a
          few words.

          Show
          Ralf Joachim added a comment - Hi Werner, had a look at your problem this evening but got another idea how this may be handled. 1. Object to load is created in TransactionContext.load() <688-701>. — begin snap — if ( objectToBeLoaded != null ) object = objectToBeLoaded; else { if ( _instanceFactory != null ) { object = _instanceFactory.newInstance( molder.getName(), _db.getClassLoader() ); } else { object = molder.newInstance( _db.getClassLoader() ); } } entry = addObjectEntry( oid, object ); oid = engine.load( this, oid, object, suggestedAccessMode, _lockTimeout, results ); // rehash the object entry, because oid might have changed! entry = rehash( object, oid ); — end snap — Move this code from TransactionContext.load() over LockEngine.load() to ClassMolder.load(). I am not sure this will be possible but we should give it a try. 2. Also move your new code to find the extend class from SQLEngine.load() <1105-1137> to ClassMolder.load(). This also seams to be possible and if both works you are able to create to right object for the first time. Hope this will help and not confuse you as its a difficult to describe in a few words.
          Hide
          Ralf Joachim added a comment -

          Tried to move object creation code from TransactionContext.load() to
          LockEngine.load() as suggested at comment 11 with the result that all test cases
          except TC47 pass. Did not find out the reason why TC47 failed now.

          Show
          Ralf Joachim added a comment - Tried to move object creation code from TransactionContext.load() to LockEngine.load() as suggested at comment 11 with the result that all test cases except TC47 pass. Did not find out the reason why TC47 failed now.
          Hide
          Ralf Joachim added a comment -

          Created an attachment (id=940)
          Patch as described at comment 11

          Did not manage to get TC47 working, but here is the patch what I tried.

          Show
          Ralf Joachim added a comment - Created an attachment (id=940) Patch as described at comment 11 Did not manage to get TC47 working, but here is the patch what I tried.
          Hide
          Werner Guttmann added a comment -

          > Move this code from TransactionContext.load() over LockEngine.load()
          > to ClassMolder.load(). I am not sure this will be possible but we
          > should give it a try.
          Hmm .. not sure here, as each of the Classes mentioned has been designed with a
          specific responsibility in mind. I'll give it a try, though, and see where it
          takes me.

          Show
          Werner Guttmann added a comment - > Move this code from TransactionContext.load() over LockEngine.load() > to ClassMolder.load(). I am not sure this will be possible but we > should give it a try. Hmm .. not sure here, as each of the Classes mentioned has been designed with a specific responsibility in mind. I'll give it a try, though, and see where it takes me.
          Hide
          Werner Guttmann added a comment -

          FYI, a quick update in other words .. .

          I am already capable of loading classes using Database.load() with single and
          compound identities, with Database.load() returning the actual leaf classes
          rather than the class type specified in the call to Database.load(). Iow,

          LaptopMulti laptop = (LaptopMulti) db.load (Computer.class, new Complex (new
          Identity (1), new Identity (1));

          works fine, as well as

          Laptop laptop = (Laptop) db.load (Computer.class, new Identity (1));

          My problem currently is that I am not able to create a patch as I do not have
          connectivity to the CVS repository right now.

          Show
          Werner Guttmann added a comment - FYI, a quick update in other words .. . I am already capable of loading classes using Database.load() with single and compound identities, with Database.load() returning the actual leaf classes rather than the class type specified in the call to Database.load(). Iow, LaptopMulti laptop = (LaptopMulti) db.load (Computer.class, new Complex (new Identity (1), new Identity (1)); works fine, as well as Laptop laptop = (Laptop) db.load (Computer.class, new Identity (1)); My problem currently is that I am not able to create a patch as I do not have connectivity to the CVS repository right now.
          Hide
          Ralf Joachim added a comment -
              • Bug 1893 has been marked as a duplicate of this bug. ***
          Show
          Ralf Joachim added a comment - Bug 1893 has been marked as a duplicate of this bug. ***
          Hide
          Ralf Joachim added a comment -

          Created an attachment (id=958)
          Refactored test from bug 1893

          After refactoring there are no more failures because I fixed sequence of object
          creation. But problems will arise when testing to load created objects. Can be
          used as use case for polymorphism.

          Show
          Ralf Joachim added a comment - Created an attachment (id=958) Refactored test from bug 1893 After refactoring there are no more failures because I fixed sequence of object creation. But problems will arise when testing to load created objects. Can be used as use case for polymorphism.
          Hide
          Werner Guttmann added a comment -

          Ralf, can I assume that the last attachment was meant to go somewhere elese ?

          Show
          Werner Guttmann added a comment - Ralf, can I assume that the last attachment was meant to go somewhere elese ?
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=959)
          Updated patch (4)

          This patch adds (complete?) support for polymorphism when using
          Database.load(). It desperately needs some refactoring, btu I'd like to hear
          opinions first with regards to the approach chosen. Updated examples will
          follow en unsegundo.

          Show
          Werner Guttmann added a comment - Created an attachment (id=959) Updated patch (4) This patch adds (complete?) support for polymorphism when using Database.load(). It desperately needs some refactoring, btu I'd like to hear opinions first with regards to the approach chosen. Updated examples will follow en unsegundo.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=960)
          Updated examples (2)

          Updated examples to showcase teh use of Database.load with various extend
          hierarchies.

          Show
          Werner Guttmann added a comment - Created an attachment (id=960) Updated examples (2) Updated examples to showcase teh use of Database.load with various extend hierarchies.
          Hide
          Ralf Joachim added a comment -

          Werner your assumption is wrong. As bug 1893 is a duplicate of bug 1881 I
          intensionally added its test cases hare after cleaning them up.

          Show
          Ralf Joachim added a comment - Werner your assumption is wrong. As bug 1893 is a duplicate of bug 1881 I intensionally added its test cases hare after cleaning them up.
          Hide
          Werner Guttmann added a comment -

          Thanks, Ralf.

          Show
          Werner Guttmann added a comment - Thanks, Ralf.
          Hide
          Werner Guttmann added a comment -

          Just to let you know that the refactored test case you attached from bug 1893
          are fully functional.

          Show
          Werner Guttmann added a comment - Just to let you know that the refactored test case you attached from bug 1893 are fully functional.
          Hide
          Werner Guttmann added a comment -

          I am just looking at ParseTreeWalker, and I wonder what's a good place to add
          code to deal with extend relations, as clearly ParseTreeWalker does not deal
          with such relations at all at the moment. addJoinsForPathExpression() looks
          like a good place to me .. ?

          Show
          Werner Guttmann added a comment - I am just looking at ParseTreeWalker, and I wonder what's a good place to add code to deal with extend relations, as clearly ParseTreeWalker does not deal with such relations at all at the moment. addJoinsForPathExpression() looks like a good place to me .. ?
          Hide
          Werner Guttmann added a comment -

          Actually, there's code in ParseTreeWalker.getFieldAndClassDesc() that deals
          with EXTEND relations. But somehow this code seems to be quite buggy to me.
          Right after ...

          // prepare the return value
          retVal = new Object[]

          {field, cd}

          ;
          if (cd != clsDesc)

          { ... }

          we add inner joins if the field required was found in a parent class. But
          strangely enough, we do not add joins for ALL extend relations, in case there's
          more than one .. . Maybe I should prepare a test case for this ....

          Show
          Werner Guttmann added a comment - Actually, there's code in ParseTreeWalker.getFieldAndClassDesc() that deals with EXTEND relations. But somehow this code seems to be quite buggy to me. Right after ... // prepare the return value retVal = new Object[] {field, cd} ; if (cd != clsDesc) { ... } we add inner joins if the field required was found in a parent class. But strangely enough, we do not add joins for ALL extend relations, in case there's more than one .. . Maybe I should prepare a test case for this ....
          Hide
          Werner Guttmann added a comment -

          What's actually the difference between the members _sqlLoad und _sqlFinder in
          SQLEngine. _sqlLoad seems to be storing a SQL statement for loading a complete
          object from the database, but what about _sqlFinder ?

          Show
          Werner Guttmann added a comment - What's actually the difference between the members _sqlLoad und _sqlFinder in SQLEngine. _sqlLoad seems to be storing a SQL statement for loading a complete object from the database, but what about _sqlFinder ?
          Hide
          Ralf Joachim added a comment -
              • Bug 1137 has been marked as a duplicate of this bug. ***
          Show
          Ralf Joachim added a comment - Bug 1137 has been marked as a duplicate of this bug. ***
          Hide
          Ralf Joachim added a comment -

          Created an attachment (id=964)
          Refactored test from bug 1137

          Test to query extended classes.

          Show
          Ralf Joachim added a comment - Created an attachment (id=964) Refactored test from bug 1137 Test to query extended classes.
          Hide
          Ralf Joachim added a comment -
              • Bug 1197 has been marked as a duplicate of this bug. ***
          Show
          Ralf Joachim added a comment - Bug 1197 has been marked as a duplicate of this bug. ***
          Hide
          Ralf Joachim added a comment -

          Created an attachment (id=965)
          Refactored test from bug 1197

          Test to load objects of extended classes in one transaction or by 2 parallel
          threads.

          Show
          Ralf Joachim added a comment - Created an attachment (id=965) Refactored test from bug 1197 Test to load objects of extended classes in one transaction or by 2 parallel threads.
          Hide
          Werner Guttmann added a comment -

          Ralf, just added the patch from bug 1997 to my test suite, and things run
          without a problem. I just fixed the (imho) last bug that was left over from my
          'hacking' session last week, but as it stands right now, I have got code ready
          to be reviewed (and most likely refactored) ... .

          Show
          Werner Guttmann added a comment - Ralf, just added the patch from bug 1997 to my test suite, and things run without a problem. I just fixed the (imho) last bug that was left over from my 'hacking' session last week, but as it stands right now, I have got code ready to be reviewed (and most likely refactored) ... .
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=966)
          Updated examples (3)

          Updated working examples, incl. (for the first time) several calls to OQLQuery.

          Show
          Werner Guttmann added a comment - Created an attachment (id=966) Updated examples (3) Updated working examples, incl. (for the first time) several calls to OQLQuery.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=967)
          Updated patch (5)

          Fully working patch for adding polymorphism, based upon the test cases attached
          to this bug report. I have not managed to verify whether the test cases are
          still executing as normal.

          Show
          Werner Guttmann added a comment - Created an attachment (id=967) Updated patch (5) Fully working patch for adding polymorphism, based upon the test cases attached to this bug report. I have not managed to verify whether the test cases are still executing as normal.
          Hide
          Werner Guttmann added a comment -

          I just came to realize that we have some 'silent' listeners here .. . Which,
          to be honest, I really appreciate. Would anybody be able and willing to help us
          a bit and test what we have so far ?

          Show
          Werner Guttmann added a comment - I just came to realize that we have some 'silent' listeners here .. . Which, to be honest, I really appreciate. Would anybody be able and willing to help us a bit and test what we have so far ?
          Hide
          Werner Guttmann added a comment -

          It actually looks like att test cases bug TC06 (Deadlock) run successfully. And
          I actually do have an idea why TC06 might not finish ... .

          Show
          Werner Guttmann added a comment - It actually looks like att test cases bug TC06 (Deadlock) run successfully. And I actually do have an idea why TC06 might not finish ... .
          Hide
          Werner Guttmann added a comment -

          Unfortunately, please ignore my last comment. There's actually more test cases
          that don't complete successfully, mostly around loading dependent obejcts.

          Show
          Werner Guttmann added a comment - Unfortunately, please ignore my last comment. There's actually more test cases that don't complete successfully, mostly around loading dependent obejcts.
          Hide
          Werner Guttmann added a comment -

          Down to two failing test only (TC30 and TC47). Apparently, there was some real
          mess in SQLQuery.loadMultiField() left from ... <blush> a copy-and-paste job.

          Show
          Werner Guttmann added a comment - Down to two failing test only (TC30 and TC47). Apparently, there was some real mess in SQLQuery.loadMultiField() left from ... <blush> a copy-and-paste job.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=975)
          Updated patch (6)

          This patch basically tursn all CTF tests funcitonal (but TC06, but that seems
          to be a problem by design).

          Show
          Werner Guttmann added a comment - Created an attachment (id=975) Updated patch (6) This patch basically tursn all CTF tests funcitonal (but TC06, but that seems to be a problem by design).
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=976)
          Updated examples (4)

          Updated examples, incl. all tests as posted by Ralf.

          Show
          Werner Guttmann added a comment - Created an attachment (id=976) Updated examples (4) Updated examples, incl. all tests as posted by Ralf.
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=977)
          Updated tests.

          Some unstructured changes to allow running test caes from within Eclipse.

          Show
          Werner Guttmann added a comment - Created an attachment (id=977) Updated tests. Some unstructured changes to allow running test caes from within Eclipse.
          Hide
          Werner Guttmann added a comment -

          All, any opinions welcome. And this includes comments/suggestions about
          potential refactoring targets.

          Show
          Werner Guttmann added a comment - All, any opinions welcome. And this includes comments/suggestions about potential refactoring targets.
          Hide
          Werner Guttmann added a comment -

          Btw, after creating the database, please execute the following query:

          update owner set product = 1 where id = 1

          And don't ask me why .... . After executing that query, all tests run fine ...

          Show
          Werner Guttmann added a comment - Btw, after creating the database, please execute the following query: update owner set product = 1 where id = 1 And don't ask me why .... . After executing that query, all tests run fine ...
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=980)
          Updated examples (5)

          Updated examples, incl. yet more tests added to TestPolymorphism. In addition,
          I have moved all tests from test1997 and test1893 in the polymorphism package
          (incl. the DDLs and the mappings). Plus there's a test suite now named
          AllTests.

          Show
          Werner Guttmann added a comment - Created an attachment (id=980) Updated examples (5) Updated examples, incl. yet more tests added to TestPolymorphism. In addition, I have moved all tests from test1997 and test1893 in the polymorphism package (incl. the DDLs and the mappings). Plus there's a test suite now named AllTests.
          Hide
          Werner Guttmann added a comment -

          I have just done a bit of profiling, and in general, it looks like we spend a
          lot of time in LockEngine.load(), ClassMolder.load, ClassMolder.loadFields(),
          SQLEngine.load(), etc. Parts of this is due to establishing JDBC connections,
          but most of it is a result of inefficient code, I guess. All these for () loops
          wuth switch statemets don't really make the story any faster .. . Looks like
          it is about time to start refactoring ....

          Show
          Werner Guttmann added a comment - I have just done a bit of profiling, and in general, it looks like we spend a lot of time in LockEngine.load(), ClassMolder.load, ClassMolder.loadFields(), SQLEngine.load(), etc. Parts of this is due to establishing JDBC connections, but most of it is a result of inefficient code, I guess. All these for () loops wuth switch statemets don't really make the story any faster .. . Looks like it is about time to start refactoring ....
          Hide
          Werner Guttmann added a comment -

          Created an attachment (id=981)
          Thread call graph

          This thread call graph (courtesy of Eclipse Colourer) quite nicely shows where
          a lot of time is spent (ignoring DatabaseRegistry.createConnection(), of
          course).

          Show
          Werner Guttmann added a comment - Created an attachment (id=981) Thread call graph This thread call graph (courtesy of Eclipse Colourer) quite nicely shows where a lot of time is spent (ignoring DatabaseRegistry.createConnection(), of course).
          Hide
          gblock@ctoforaday.com added a comment -

          Be really careful with that section - that's never been good anyways, and there's all kinds of ugly stuff
          in there.

          I'm temtped to say create a new bug and close this one off; then we'll look at each class in turn and see
          if there's something we can do to end the loop-walking that doesn't involve getting everything to do
          the loop walking. My goal of removing all the loops from my previous attempt at a TransactionManager
          rewrite make me believe that they're all interdependent, and that getting rid of loopwalking is going to
          need some pretty fundamental, strategic changes to the way we examine and process status
          information.

          Show
          gblock@ctoforaday.com added a comment - Be really careful with that section - that's never been good anyways, and there's all kinds of ugly stuff in there. I'm temtped to say create a new bug and close this one off; then we'll look at each class in turn and see if there's something we can do to end the loop-walking that doesn't involve getting everything to do the loop walking. My goal of removing all the loops from my previous attempt at a TransactionManager rewrite make me believe that they're all interdependent, and that getting rid of loopwalking is going to need some pretty fundamental, strategic changes to the way we examine and process status information.
          Hide
          Bruce Snyder (bugzilla) added a comment -

          Inefficient code down in those areas is why Thomas and I have talked about
          refactoring everything around the ClassMolder/FieldMolder area. I just sent an
          email with Thomas' refactoring docs attached.

          Show
          Bruce Snyder (bugzilla) added a comment - Inefficient code down in those areas is why Thomas and I have talked about refactoring everything around the ClassMolder/FieldMolder area. I just sent an email with Thomas' refactoring docs attached.
          Hide
          Werner Guttmann added a comment -

          Thanks, Bruce. Gregory, what's your comment referring to specifically ?

          Show
          Werner Guttmann added a comment - Thanks, Bruce. Gregory, what's your comment referring to specifically ?
          Hide
          Werner Guttmann added a comment -

          Gregory ?

          Show
          Werner Guttmann added a comment - Gregory ?
          Hide
          Werner Guttmann added a comment -

          Gregory, I can now see where you are coming from, as that's what we have been trying to resolve last weekend. Point taken ..

          Show
          Werner Guttmann added a comment - Gregory, I can now see where you are coming from, as that's what we have been trying to resolve last weekend. Point taken ..
          Hide
          Werner Guttmann added a comment -

          Gregory, I'd really like to see a separate bug report for the TransactionContext(impl) rewrite, as I'd like to discuss making a new preview release available, etc. Please, pretty please. Please ?

          Show
          Werner Guttmann added a comment - Gregory, I'd really like to see a separate bug report for the TransactionContext(impl) rewrite, as I'd like to discuss making a new preview release available, etc. Please, pretty please. Please ?
          Hide
          Werner Guttmann added a comment -

          Based upon my work to add support for polymorphism, I think that this code really is not used at all, and hence should/could be removed.

          Show
          Werner Guttmann added a comment - Based upon my work to add support for polymorphism, I think that this code really is not used at all, and hence should/could be removed.
          Hide
          Gregory Block added a comment -

          If there's code in there that you think isn't being used, IMO, comment it out; we're in desperate need of reducing the complexity of this software, and knowing what doesn't need to get refactored out would be a huge boon.

          (Sorry for spamming this bug; I misread on the previous patch comments, now deleted.)

          We're going to have to work through each of these classes and normalise their behavior. In each, it's likely that we can make use of the ObjectTracker to track the state in a way that should allow us to get rid of the loopwalking - with the potentially damaging side-effects that occur when you get it wrong, as you've now witnessed while we were working on the other patch.

          I say we find the next-biggest-pig and shoot it in the head. In all likelihood, that's going to mean probably starting at the top and working our way in, rather than starting at the bottom (sqlengine) and working our way up, and doing so (ideally) in the same way we have with TransactionManager - clean up the codebase so it makes sense, and then once we have something clean and behaves in a sane manner, we can start moving responsibilities around in the codebase and try to improve their APIs and interlocking (which at the moment is way, way too deeply interconnected).

          Show
          Gregory Block added a comment - If there's code in there that you think isn't being used, IMO, comment it out; we're in desperate need of reducing the complexity of this software, and knowing what doesn't need to get refactored out would be a huge boon. (Sorry for spamming this bug; I misread on the previous patch comments, now deleted.) We're going to have to work through each of these classes and normalise their behavior. In each, it's likely that we can make use of the ObjectTracker to track the state in a way that should allow us to get rid of the loopwalking - with the potentially damaging side-effects that occur when you get it wrong, as you've now witnessed while we were working on the other patch. I say we find the next-biggest-pig and shoot it in the head. In all likelihood, that's going to mean probably starting at the top and working our way in, rather than starting at the bottom (sqlengine) and working our way up, and doing so (ideally) in the same way we have with TransactionManager - clean up the codebase so it makes sense, and then once we have something clean and behaves in a sane manner, we can start moving responsibilities around in the codebase and try to improve their APIs and interlocking (which at the moment is way, way too deeply interconnected).
          Hide
          Werner Guttmann added a comment -

          > If there's code in there that you think isn't being used, IMO,
          > comment it out; we're in desperate need of reducing the
          > complexity of this software, and knowing what doesn't need
          > to get refactored out would be a huge boon.
          I'll definitely get this commented/removed once polymorphism has been added to the codebase. If you don't mind, I'd like to see this delayed by a week or so, as I feel like we should provide for a maintenance release (0.9.6.1) within 14 days or so.

          Show
          Werner Guttmann added a comment - > If there's code in there that you think isn't being used, IMO, > comment it out; we're in desperate need of reducing the > complexity of this software, and knowing what doesn't need > to get refactored out would be a huge boon. I'll definitely get this commented/removed once polymorphism has been added to the codebase. If you don't mind, I'd like to see this delayed by a week or so, as I feel like we should provide for a maintenance release (0.9.6.1) within 14 days or so.
          Hide
          Werner Guttmann added a comment -

          Updated patch too allow for changes to TransactionContext after introducing ObjectTracker.

          To-Do:

          • move tests from src/examples/polymorphism to src/tests/jdo (or a sub-package thereof).
          • check whetehr test suite runs
          • checkstye
          • documentation
          • release notes
          Show
          Werner Guttmann added a comment - Updated patch too allow for changes to TransactionContext after introducing ObjectTracker. To-Do: move tests from src/examples/polymorphism to src/tests/jdo (or a sub-package thereof). check whetehr test suite runs checkstye documentation release notes
          Hide
          Werner Guttmann added a comment -

          Final patch, incl. small modifications to src/tests/jdo/CallSql.java to cater for new select statement syntax, new src/tests/ctf/jdo/tc9x package with several new tests for polymorphism, etc. Code is checkstyled, and all CTF tests (incl. the new ones) complete sucessfully.

          To-do:

          a) Transfer SQL script changes to all remaining databases & test on those.
          b) Code review ...

          Show
          Werner Guttmann added a comment - Final patch, incl. small modifications to src/tests/jdo/CallSql.java to cater for new select statement syntax, new src/tests/ctf/jdo/tc9x package with several new tests for polymorphism, etc. Code is checkstyled, and all CTF tests (incl. the new ones) complete sucessfully. To-do: a) Transfer SQL script changes to all remaining databases & test on those. b) Code review ...
          Hide
          Ralf Joachim added a comment -

          At a first test i found following problems:
          1. PTF fails.
          2. CTF TC97 fails with following stacktrace:

          java.sql.SQLException: Base table or view not found message from server: "Table 'test.order_product' doesn't exist"
          at com.mysql.jdbc.MysqlIO.checkErrorPacket(MysqlIO.java:1997)
          at com.mysql.jdbc.MysqlIO.sendCommand(MysqlIO.java:1167)
          at com.mysql.jdbc.MysqlIO.sqlQueryDirect(MysqlIO.java:1278)
          at com.mysql.jdbc.Connection.execSQL(Connection.java:2251)
          at com.mysql.jdbc.PreparedStatement.executeQuery(PreparedStatement.java:1586)
          at org.exolab.castor.jdo.engine.SQLEngine.load(SQLEngine.java:1180)
          at org.exolab.castor.persist.ClassMolder.loadFields(ClassMolder.java:750)
          at org.exolab.castor.persist.ClassMolder.load(ClassMolder.java:792)
          at org.exolab.castor.persist.LockEngine.load(LockEngine.java:368)
          at org.castor.persist.TransactionContext.load(TransactionContext.java:718)
          at org.castor.persist.TransactionContext.load(TransactionContext.java:554)
          at org.exolab.castor.jdo.engine.DatabaseImpl.load(DatabaseImpl.java:351)
          at org.exolab.castor.jdo.engine.DatabaseImpl.load(DatabaseImpl.java:315)
          at ctf.jdo.tc9x.TestPolymorphism.testLoadLaptop(TestPolymorphism.java:85)
          at ctf.jdo.tc9x.TestPolymorphism.runTest(TestPolymorphism.java:58)
          at junit.framework.TestCase.runBare(TestCase.java:127)
          at junit.framework.TestResult$1.protect(TestResult.java:106)
          at junit.framework.TestResult.runProtected(TestResult.java:124)
          at junit.framework.TestResult.run(TestResult.java:109)
          at junit.framework.TestCase.run(TestCase.java:118)
          at harness.CastorTestCase.run(CastorTestCase.java:154)
          at junit.framework.TestSuite.runTest(TestSuite.java:208)
          at junit.framework.TestSuite.run(TestSuite.java:203)
          at harness.TestHarness.run(TestHarness.java:140)
          at junit.framework.TestSuite.runTest(TestSuite.java:208)
          at junit.framework.TestSuite.run(TestSuite.java:203)
          at harness.TestHarness.run(TestHarness.java:140)
          at Main.run(Main.java:152)
          at junit.textui.TestRunner.doRun(TestRunner.java:116)
          at junit.textui.TestRunner.doRun(TestRunner.java:109)
          at junit.textui.TestRunner.run(TestRunner.java:72)
          at Main.main(Main.java:226)

          Show
          Ralf Joachim added a comment - At a first test i found following problems: 1. PTF fails. 2. CTF TC97 fails with following stacktrace: java.sql.SQLException: Base table or view not found message from server: "Table 'test.order_product' doesn't exist" at com.mysql.jdbc.MysqlIO.checkErrorPacket(MysqlIO.java:1997) at com.mysql.jdbc.MysqlIO.sendCommand(MysqlIO.java:1167) at com.mysql.jdbc.MysqlIO.sqlQueryDirect(MysqlIO.java:1278) at com.mysql.jdbc.Connection.execSQL(Connection.java:2251) at com.mysql.jdbc.PreparedStatement.executeQuery(PreparedStatement.java:1586) at org.exolab.castor.jdo.engine.SQLEngine.load(SQLEngine.java:1180) at org.exolab.castor.persist.ClassMolder.loadFields(ClassMolder.java:750) at org.exolab.castor.persist.ClassMolder.load(ClassMolder.java:792) at org.exolab.castor.persist.LockEngine.load(LockEngine.java:368) at org.castor.persist.TransactionContext.load(TransactionContext.java:718) at org.castor.persist.TransactionContext.load(TransactionContext.java:554) at org.exolab.castor.jdo.engine.DatabaseImpl.load(DatabaseImpl.java:351) at org.exolab.castor.jdo.engine.DatabaseImpl.load(DatabaseImpl.java:315) at ctf.jdo.tc9x.TestPolymorphism.testLoadLaptop(TestPolymorphism.java:85) at ctf.jdo.tc9x.TestPolymorphism.runTest(TestPolymorphism.java:58) at junit.framework.TestCase.runBare(TestCase.java:127) at junit.framework.TestResult$1.protect(TestResult.java:106) at junit.framework.TestResult.runProtected(TestResult.java:124) at junit.framework.TestResult.run(TestResult.java:109) at junit.framework.TestCase.run(TestCase.java:118) at harness.CastorTestCase.run(CastorTestCase.java:154) at junit.framework.TestSuite.runTest(TestSuite.java:208) at junit.framework.TestSuite.run(TestSuite.java:203) at harness.TestHarness.run(TestHarness.java:140) at junit.framework.TestSuite.runTest(TestSuite.java:208) at junit.framework.TestSuite.run(TestSuite.java:203) at harness.TestHarness.run(TestHarness.java:140) at Main.run(Main.java:152) at junit.textui.TestRunner.doRun(TestRunner.java:116) at junit.textui.TestRunner.doRun(TestRunner.java:109) at junit.textui.TestRunner.run(TestRunner.java:72) at Main.main(Main.java:226)
          Hide
          Werner Guttmann added a comment -

          Updated patch (modified src/tests/ctf/jdo/tc9x/mapping.xml to cater for renamed tables).

          Show
          Werner Guttmann added a comment - Updated patch (modified src/tests/ctf/jdo/tc9x/mapping.xml to cater for renamed tables).
          Hide
          Werner Guttmann added a comment -

          Updated patch, fixing the problem with ArrayIndexOutOfBoundExceptions when running the PTF:

          Show
          Werner Guttmann added a comment - Updated patch, fixing the problem with ArrayIndexOutOfBoundExceptions when running the PTF:
          Hide
          Ralf Joachim added a comment -

          Current result of performance test suite.

          Show
          Ralf Joachim added a comment - Current result of performance test suite.
          Hide
          Werner Guttmann added a comment -

          This patch includes three small performance improvements:

          a) Introduced a SQLEngine cache in BaseFactory that will be used in BaseFactory.getPersistence(ClassDescriptor) to store ClassDescriptor --> SQLEngine mappings once a SQLEngine has been generated.
          b) Moved code to calculate the number and collection of extending class descriptors for a given class from SQLEngine.load() to SQLEngine.<init>, thus avoiding this code to be executed on every call to SQLEngine.load().

          Show
          Werner Guttmann added a comment - This patch includes three small performance improvements: a) Introduced a SQLEngine cache in BaseFactory that will be used in BaseFactory.getPersistence(ClassDescriptor) to store ClassDescriptor --> SQLEngine mappings once a SQLEngine has been generated. b) Moved code to calculate the number and collection of extending class descriptors for a given class from SQLEngine.load() to SQLEngine.<init>, thus avoiding this code to be executed on every call to SQLEngine.load().
          Hide
          Ralf Joachim added a comment -

          The bad news is that your changes didn't make any difference for PTF but, what may be more important, I found the problem. It had been the logging statments in the ClassMolder.load() methods. Therefore i removed them and also most of the others you intruduced e.g. in SQLEngine. Now the Polymorphism patch has very small influence on PTF.

          In addition I reviewed the whole main code and didn't found issues except some small fixes included in the new patch. The missing parts IMO are:

          a) PTF results for 20000 objects
          b) Review of test cases
          c) SQL scripts for all the other engines

          Show
          Ralf Joachim added a comment - The bad news is that your changes didn't make any difference for PTF but, what may be more important, I found the problem. It had been the logging statments in the ClassMolder.load() methods. Therefore i removed them and also most of the others you intruduced e.g. in SQLEngine. Now the Polymorphism patch has very small influence on PTF. In addition I reviewed the whole main code and didn't found issues except some small fixes included in the new patch. The missing parts IMO are: a) PTF results for 20000 objects b) Review of test cases c) SQL scripts for all the other engines
          Hide
          Werner Guttmann added a comment -

          Added release notes and copied changes to src/tests/jdo/mysql.sql to all other <database>.sql. files.

          Show
          Werner Guttmann added a comment - Added release notes and copied changes to src/tests/jdo/mysql.sql to all other <database>.sql. files.
          Hide
          Ralf Joachim added a comment -

          Updated patch with finished PTF results and reviewed CTF test cases. Found 2 situations where I am not sure if we should add more tests:

          1. test of polymorphism when using keygen
          2. test long transactions

          are missing. In addition I fixed issues in hsql.sql script and tried to run CTF tests on hsql. I had been very surrprised to see that most of the tests that could successfully be executed before applying this patch now fail. In most cases they throw a SQLException 'column not found:7' which brings me to the conclusion that there most still be something wrong in SQLEngine that needs to be resolved before commit. As hsql isn't the most important db engine to support we at minimum need to execute tests on most of the other supported db engines.

          I'll try to find the time to execute tests against oracle and postgres.

          Show
          Ralf Joachim added a comment - Updated patch with finished PTF results and reviewed CTF test cases. Found 2 situations where I am not sure if we should add more tests: 1. test of polymorphism when using keygen 2. test long transactions are missing. In addition I fixed issues in hsql.sql script and tried to run CTF tests on hsql. I had been very surrprised to see that most of the tests that could successfully be executed before applying this patch now fail. In most cases they throw a SQLException 'column not found:7' which brings me to the conclusion that there most still be something wrong in SQLEngine that needs to be resolved before commit. As hsql isn't the most important db engine to support we at minimum need to execute tests on most of the other supported db engines. I'll try to find the time to execute tests against oracle and postgres.
          Hide
          Ralf Joachim added a comment -

          Werner, found the problem why CTF fails at hsql. Patch now also passes CTF tests with hsql in addition to mysql.

          Show
          Ralf Joachim added a comment - Werner, found the problem why CTF fails at hsql. Patch now also passes CTF tests with hsql in addition to mysql.
          Hide
          Ralf Joachim added a comment -

          Something is going horrobly wrong with oracle. At SQLEngine.loadSingleField() (line 2281) the variable 'columnTableName' is always empty and the for loop therefore never finishes. In addition we now get a FATAL exception according to the wrong KeyGen that had been a WARN before.

          The over all concequence is that NO test case of CTF can be executed and I don't realy know what to do.

          Show
          Ralf Joachim added a comment - Something is going horrobly wrong with oracle. At SQLEngine.loadSingleField() (line 2281) the variable 'columnTableName' is always empty and the for loop therefore never finishes. In addition we now get a FATAL exception according to the wrong KeyGen that had been a WARN before. The over all concequence is that NO test case of CTF can be executed and I don't realy know what to do.
          Hide
          Werner Guttmann added a comment -

          Added test case for using MAX key generator with polymorphism.

          Show
          Werner Guttmann added a comment - Added test case for using MAX key generator with polymorphism.
          Hide
          Ralf Joachim added a comment -

          Problem introduced by issue CASTOR-1018.

          Show
          Ralf Joachim added a comment - Problem introduced by issue CASTOR-1018 .
          Hide
          Ralf Joachim added a comment -

          Fixed ORACLE problem with one exception: it is not possible to use LIMIT clause at extended classes.
          New patch is tested against mysql and oracle. Previous tests on other db's need to be executed again.

          Show
          Ralf Joachim added a comment - Fixed ORACLE problem with one exception: it is not possible to use LIMIT clause at extended classes. New patch is tested against mysql and oracle. Previous tests on other db's need to be executed again.
          Hide
          Werner Guttmann added a comment -

          Added yet another test case (TC94) to test Database.update(Object) with polymorphism included. So far, I have only added the test case to the 'mysql' category of src/tests/tests.xml.

          Show
          Werner Guttmann added a comment - Added yet another test case (TC94) to test Database.update(Object) with polymorphism included. So far, I have only added the test case to the 'mysql' category of src/tests/tests.xml.
          Hide
          Werner Guttmann added a comment -

          Patch updated after commit of CASTOR-1110.

          Show
          Werner Guttmann added a comment - Patch updated after commit of CASTOR-1110 .
          Hide
          Werner Guttmann added a comment -

          Updated patch, incl. changes to db2.sql to get the CTF test suite running for the new test cases TC94 to TC99. Having said that, there's one or two minor issues with some SQL statements that are a result of committing bug CASTOR-1110.

          Show
          Werner Guttmann added a comment - Updated patch, incl. changes to db2.sql to get the CTF test suite running for the new test cases TC94 to TC99. Having said that, there's one or two minor issues with some SQL statements that are a result of committing bug CASTOR-1110 .
          Hide
          Ralf Joachim added a comment -

          Final patch with some additional code cleanups at test objects. Will commit that in a minute.

          Show
          Ralf Joachim added a comment - Final patch with some additional code cleanups at test objects. Will commit that in a minute.

            People

            • Assignee:
              Werner Guttmann
              Reporter:
              Werner Guttmann
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 hour
                1h