Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug? Wrong order in batch save #2377

Merged
merged 1 commit into from
Sep 19, 2021
Merged

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented Sep 16, 2021

Hello Rob,

we have a chain of related models which references each other: M1-> M2-> M3
We use a Transacton with enabled batch mode (Without batchMode, everything is fine)

When CascadeType.ALL is used, everything is fine, if just M1 is saved. (Ebean recognizes, that M3 has to be saved first, then M2 and finally M1). See testMultiSave1

But when M3 is saved manually and then M1, ebean recognizes, that M3 is already in batch (but with wrong depth) and will try to save M2, M1 and then M3 in the wrong order. This will not work, if the beans have already an ID, because in this case, no deferred relationship will be creaated by BindableAssocOne: See testMultiSave2

I could fix this case by modifiying the "alreadyInBatch" check. We have to flush, if the depth in the batch is bigger than the current order (or modify the order of the existing bean/beanHolder in the queue)

But there is one annoying testcase testMultiSave4 (This was also discovered in our application)
In this case, M2->M3 has no cascadeType set. Saving of M3 has no immediate effect, so saving of M1 will always fail, as there is no cascade-relation to M3

Do you think, this is a bug in Ebean or do we use the cascadeTypes in the wrong way?

Cheers
Roland

@rbygrave
Copy link
Member

testcase testMultiSave4 ... Do you think, this is a bug in Ebean or do we use the cascadeTypes in the wrong way?

Definitely a bug.

We can swap out Relation3 with say Relation4 that never has cascade and reproduce the issue. That is, to Relation3 we have both cascade and non-cascading paths but that does not matter here - definitely a bug.

If we look closely at the BatchControl flush log we kind of see the issue - the -100 for Relation2 vs 0 for Relation4.

14:19:47.590 [main] DEBUG io.ebean.SUM - txn[1001] BatchControl flush [Relation2:-100 i:1, Relation4:0 i:1, Relation1:1 i:1]

... Relation2:-100, Relation4:0 , Relation1:1

14:19:47.579 [main] TRACE io.ebean.TXN - txn[1001] Begin
14:19:47.590 [main] DEBUG io.ebean.SUM - txn[1001] BatchControl flush [Relation2:-100 i:1, Relation4:0 i:1, Relation1:1 i:1]
14:19:47.593 [main] DEBUG io.ebean.SQL - txn[1001] insert into relation2 (id, name, r4_no_cascade_id, no_cascade_id, with_cascade_id) values (?,?,?,?,?)
14:19:47.593 [main] DEBUG io.ebean.SQL - txn[1001]  -- bind(5ea60482-c445-42ca-a2c1-ed48dacb3e4f,R2,a142fe17-1454-476e-83b5-82269a3b762f,null,null)
14:19:47.596 [main] DEBUG io.ebean.TXN - txn[1001] Rollback error: io.ebean.DataIntegrityException: Error when batch flush on sql: insert into relation2 (id, name, r4_no_cascade_id, no_cascade_id, with_cascade_id) values (?,?,?,?,?) stack0: io.ebean.config.dbplatform.SqlCodeTranslator.translate(SqlCodeTranslator.java:49) cause: org.h2.jdbc.JdbcBatchUpdateException: Referential integrity constraint violation: "FK_RELATION2_R4_NO_CASCADE_ID: PUBLIC.RELATION2 FOREIGN KEY(R4_NO_CASCADE_ID) REFERENCES PUBLIC.RELATION4(ID) ('a142fe17-1454-476e-83b5-82269a3b762f')"; SQL statement:
insert into relation2 (id, name, r4_no_cascade_id, no_cascade_id, with_cascade_id) values (?,?,?,?,?) [23506-199] stack0: org.h2.jdbc.JdbcPreparedStatement.executeBatch(JdbcPreparedStatement.java:1298)

io.ebean.DataIntegrityException: Error when batch flush on sql: insert into relation2 (id, name, r4_no_cascade_id, no_cascade_id, with_cascade_id) values (?,?,?,?,?)

rbygrave added a commit that referenced this pull request Sep 17, 2021
The guts of this fix is in DefaultPersister.saveAssocOne() with the change to use the new transaction.depthDecrement() method. For our failing test cases this changes the depth from -1, 0 to be 0, 1.

We can see the impact of this change in the batch ordering via the logging of:
 io.ebean.SUM - txn[1001] BatchControl flush ...
rbygrave added a commit that referenced this pull request Sep 19, 2021
With the fix to the depth we no longer need these changes to BatchControl
rbygrave added a commit that referenced this pull request Sep 19, 2021
#2377 - Wrong order in batch save - Fix to transaction depth
@rbygrave rbygrave merged commit b38a21f into ebean-orm:master Sep 19, 2021
@rPraml rPraml deleted the save-cascade-order branch January 23, 2023 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants