Bugfix/3876 - #3877
Open
AntoineDuComptoirDesPharmacies wants to merge 4 commits into
Open
Conversation
… from a persist callback
A join entity owns the foreign key to the bean its delete cascades to, so the join row
has to be deleted first. When a BeanPersistController writes to the database from
preDelete, that write flushes the batch from inside the flush that is already running :
the outer flush has taken the join rows out of their bean holder, so the inner flush
finds only the assets and executes them first, which fails on the foreign key.
BatchControl flush [DcoLink:0 d:2, DcoAsset:1 d:2] <- outer flush
BatchControl flush [DcoLink:0 d:0, DcoAsset:1 d:2, DcoAudit:2 i:1] <- from preDelete
The scenario is fixed as a side effect of a2f954a (ebean-orm#3830, released in 18.3.0) which
moved controllerPreDelete() ahead of the cascade. The test pins that down : it fails
with a DataIntegrityException on a2f954a~1 and passes on master. The graph is fetched
up front on purpose, a lazy load would flush the batch on its own and hide the ordering.
…elete on a self referencing tree
The shape reported on the issue in 2019 : a container cascades the delete down a tree of
TreeBean, and the deletes are not issued deepest first. On 18.4.0 :
delete from dco_tree where id in (?) -- the root, whose children are still there
delete from dco_tree where id in (?,?,?)
delete from dco_tree where id in (?,?)
Referential integrity constraint violation: FK_DCO_TREE_PARENT_ID.
This is a different defect from the batch reordering fixed by a2f954a : nothing is batched
here, the recursion itself walks the tree in the wrong order. Disabled so it does not break
the build, remove the annotation to see the failure.
… batch mid-execution ebean-orm#3148 stopped a query performed from a callback from flushing the batch that is already executing : BatchControl.executeNow disables flushOnQuery for the duration. A persist done from the same callback is not covered. It reaches BatchControl.executeOrQueue, which flushes, and the statements queued behind the one currently executing are issued early. Saving a parent/child graph in batch while an audit row is written from preInsert issues the children before their parent has an id : insert into dco_link (parent_id, asset_id) values (?,?) NULL not allowed for column "PARENT_ID" Same defect on the delete side, where the join rows are issued after the beans they reference (ebean-orm#1852, ebean-orm#3185) — that path no longer reproduces since a2f954a moved controllerPreDelete() ahead of the cascade, but only preDelete was moved, so preInsert and preUpdate still run inside the flush. Guard executeOrQueue with the same reasoning as the existing flushOnQuery guard : while the batch is executing, queue rather than flush. The statements added meanwhile are picked up by the do/while loop in executeAll().
Member
|
Looks good to me. I am happy with it. Only pondering if |
…ck also flushes mid-execution
executeStatementOrBatch() flushes on (batchFlushOnMixed && !isBeansEmpty()), and persistedBeans
is only cleared once executeAll() returns, so during the batch execution that condition holds and
a SqlUpdate run from a BeanPersistController callback re-enters the flush the same way a save does.
Reproduced with the same test, the callback running a SqlUpdate instead of a save :
insert into dco_link (parent_id, asset_id) values (?,?)
NULL not allowed for column "PARENT_ID"
The second flush of that method, on pstmtHolder.maxSize() >= batchSize, is left untouched : it can
only trigger when the pstmt holder fills up mid-execution and there is no test covering it.
AntoineDuComptoirDesPharmacies
force-pushed
the
bugfix/3876
branch
from
August 14, 2026 07:47
b3af9d4 to
d6d4744
Compare
Contributor
Author
|
Good catch ! Yes indeed it will produce the same problem. I added a test to cover the use case and corrected the code. 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Aim to fix the following issue : #3876
Would like to know if this is the good direction or if there is some downside of disabling flush at this step of the execution.
Thanks !