Uploaded image for project: 'Cayenne'
  1. Cayenne
  2. CAY-2122

Vertical Inheritance: Cannot Insert Record For Implementing Class with Attribute And Relationship

Details

    Description

      When using the following DbEntities:

      IV_OTHER (id, name)
      IV_BASE (id, type, name)
      IV_IMPL (id, attr1, other_id) (vertically inherits IV_BASE)

      all fields are non-null

      We end up with ObjEntities:
      IvOther
      IvBase (abstract)
      IvImpl extends IvBase

      When running the following code:

      IvOther other = context.newObject(IvOther.class);
      other.setName("other");
      
      IvImpl impl = context.newObject(IvImpl.class);
      impl.setName("Impl 1");
      impl.setAttr1("attr1");
      impl.setOther(other);
      
      context.commitChanges();
      

      Committing this errors in the database, because its trying to insert into the IV_IMPL table with a NULL value for OTHER_ID

      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - INSERT INTO IV_OTHER (ID, NAME) VALUES (?, ?)
      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - [bind: 1->ID:200, 2->NAME:'other']
      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - === updated 1 row.
      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - INSERT INTO IV_BASE (ID, NAME, TYPE) VALUES (?, ?, ?)
      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - [bind: 1->ID:200, 2->NAME:'Impl 1', 3->TYPE:'I']
      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - === updated 1 row.
      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - INSERT INTO IV_IMPL (ATTR1, ID, OTHER_ID) VALUES (?, ?, ?)
      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - [bind: 1->ATTR1:'attr1', 2->ID:200, 3->OTHER_ID:NULL]
      [main] INFO org.apache.cayenne.log.CommonsJdbcEventLogger - *** error.
      java.sql.SQLException: Attempt to insert null into a non-nullable column: column: OTHER_ID table: IV_IMPL in statement [INSERT INTO IV_IMPL (ATTR1, ID, OTHER_ID) VALUES (?, ?, ?)]
      

      If there was only an attribute in IV_IMPL, or only a relationship on IV_IMPL it would work fine. But the DataDomainFlushAction is using separate InsertBatchQuery to handle the attributes and another one to handle the relationships.

      I have not yet figured out how to solve, but will continue to look into it.

      Attachments

        1. CAY-2122-test.patch
          12 kB
          Matt Watson
        2. CAY-2122-test-with-fix.patch
          19 kB
          Matt Watson

        Issue Links

          Activity

            mattraydub Matt Watson added a comment -

            Breaking test for issue above

            mattraydub Matt Watson added a comment - Breaking test for issue above
            mattraydub Matt Watson added a comment -

            Here is the fix for the breaking test. All other tests in cayenne-server (on master branch) still pass.

            The fix basically identifies duplicate InsertBatchQuery and merges them together.

            mattraydub Matt Watson added a comment - Here is the fix for the breaking test. All other tests in cayenne-server (on master branch) still pass. The fix basically identifies duplicate InsertBatchQuery and merges them together.

            Hi Matt,

            thanks for the bug report and the patch. It almost works The following line is problematic for many-to-many flattened case:

             
                     DataNode node = parent.getDomain().lookupDataNode(flattenedEntity.getDataMap());
                     Map flattenedSnapshot = flattenedArcKey.buildJoinSnapshotForInsert(node);
            -        relationInsertQuery.add(flattenedSnapshot);
            +        relationInsertQuery.add(flattenedSnapshot, flattenedArcKey.id1.getSourceId());
            

            as it messes up source ObjectId , loading join table values in it. We didn't have a direct test for this condition, but PostCommitFilter_All_FlattenedIТ from cayenne-lifecycle failed as a result. I think we might need to redo DataDomainFlattenedBucket, storing FlattenedArcKey's instead of insert batches inside addFlattenedInsert method. And then generate the queries out of them in appendInserts.. I think this is a good idea in general and should help us to avoid this issue.

            andrus Andrus Adamchik added a comment - Hi Matt, thanks for the bug report and the patch. It almost works The following line is problematic for many-to-many flattened case: DataNode node = parent.getDomain().lookupDataNode(flattenedEntity.getDataMap()); Map flattenedSnapshot = flattenedArcKey.buildJoinSnapshotForInsert(node); - relationInsertQuery.add(flattenedSnapshot); + relationInsertQuery.add(flattenedSnapshot, flattenedArcKey.id1.getSourceId()); as it messes up source ObjectId , loading join table values in it. We didn't have a direct test for this condition, but PostCommitFilter_All_FlattenedIТ from cayenne-lifecycle failed as a result. I think we might need to redo DataDomainFlattenedBucket, storing FlattenedArcKey's instead of insert batches inside addFlattenedInsert method. And then generate the queries out of them in appendInserts.. I think this is a good idea in general and should help us to avoid this issue.
            mattraydub Matt Watson added a comment -

            Ok, I see what you mean. I will take a stab at that today.

            mattraydub Matt Watson added a comment - Ok, I see what you mean. I will take a stab at that today.
            mattraydub Matt Watson added a comment -

            Modified the code as suggested by andrus and replaced the previous patch with the fix

            mattraydub Matt Watson added a comment - Modified the code as suggested by andrus and replaced the previous patch with the fix

            I committed the latest patch. Looks like it does the right thing.

            Added a comment on performance impact that will need to be solved in the future with deeper refactoring:

            // TODO: see "O(N) lookups" TODO's below. The first is relatively benign, as N is the number of DbEntities in the
            // preceeding DataDomainInsertBucket processing. The second nested one is potentially much worse, as it may
            // result in a linear scan of thousands of objects. E.g. it will affect cases with vertical inheritance with
            // relationships in subclasses...

            // The fix to the above is to merge this code into DataDomainInsertBucket, so that we can combine regular and
            // flattened snapshots at the point of InsertBatchQuery creation.

            andrus Andrus Adamchik added a comment - I committed the latest patch. Looks like it does the right thing. Added a comment on performance impact that will need to be solved in the future with deeper refactoring: // TODO: see "O(N) lookups" TODO's below. The first is relatively benign, as N is the number of DbEntities in the // preceeding DataDomainInsertBucket processing. The second nested one is potentially much worse, as it may // result in a linear scan of thousands of objects. E.g. it will affect cases with vertical inheritance with // relationships in subclasses... // The fix to the above is to merge this code into DataDomainInsertBucket, so that we can combine regular and // flattened snapshots at the point of InsertBatchQuery creation.
            mattraydub Matt Watson added a comment -

            andrus I just discovered a bug directly related to this. My original issue (and test) was referring to an Entity that only had 1 flattened relationship. Yesterday I had to add another relationship to this Entity, and discovered the same bug pops up again. This time its because the new code does not handle merging the flattenedArcKeys if more than one exists per ObjectId.

            I know what I need to do to fix it. Should I reopen this issue or create a new one?

            mattraydub Matt Watson added a comment - andrus I just discovered a bug directly related to this. My original issue (and test) was referring to an Entity that only had 1 flattened relationship. Yesterday I had to add another relationship to this Entity, and discovered the same bug pops up again. This time its because the new code does not handle merging the flattenedArcKeys if more than one exists per ObjectId. I know what I need to do to fix it. Should I reopen this issue or create a new one?
            mattraydub Matt Watson added a comment -

            I just went ahead and created a new issue CAY-2242

            mattraydub Matt Watson added a comment - I just went ahead and created a new issue CAY-2242

            People

              andrus Andrus Adamchik
              mattraydub Matt Watson
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: