diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssoc.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssoc.java
index f4f98740d..b7434b0c1 100644
--- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssoc.java
+++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssoc.java
@@ -1,6 +1,8 @@
package io.ebeaninternal.server.deploy;
import io.ebean.Query;
+import io.ebean.SqlUpdate;
+import io.ebean.Transaction;
import io.ebean.bean.EntityBean;
import io.ebean.core.type.DocPropertyType;
import io.ebean.text.PathProperties;
@@ -570,4 +572,26 @@ public abstract class BeanPropertyAssoc extends BeanProperty implements STree
+ " or a @JoinColumn needs an explicit referencedColumnName specified?";
throw new PersistenceException(msg);
}
+
+ /**
+ * Create SqlUpdate statement to delete all child beans of the parent id.
+ */
+ public abstract SqlUpdate deleteByParentId(Object id);
+
+ /**
+ * Create SqlUpdate statement to delete all child beans of the parent ids in idList.
+ */
+ public abstract SqlUpdate deleteByParentIdList(List
*/
void deleteManyDetails(SpiTransaction t, BeanDescriptor> desc, EntityBean parentBean,
- BeanPropertyAssocMany> many, List excludeDetailIds, DeleteMode deleteMode) {
+ BeanPropertyAssocMany> many, Set excludeDetailIds, DeleteMode deleteMode) {
if (many.cascadeInfo().isDelete()) {
// cascade delete the beans in the collection
BeanDescriptor> targetDesc = many.targetDescriptor();
if (deleteMode.isHard() || targetDesc.isSoftDelete()) {
- if (targetDesc.isDeleteByStatement()) {
+ if (targetDesc.isDeleteByStatement()
+ && (excludeDetailIds == null || excludeDetailIds.size() <= maxDeleteBatch)) { // TODO wait for #3176
// Just delete all the children with one statement
IntersectionRow intRow = many.buildManyDeleteChildren(parentBean, excludeDetailIds);
SqlUpdate sqlDelete = intRow.createDelete(server, deleteMode);
@@ -1022,7 +1099,16 @@ public final class DefaultPersister implements Persister {
// ... and only using findIdsByParentId() when the many property isn't loaded
// Delete recurse using the Id values of the children
Object parentId = desc.getId(parentBean);
- List idsByParentId = many.findIdsByParentId(parentId, null, t, excludeDetailIds, deleteMode.isHard());
+ List idsByParentId;
+ if (excludeDetailIds == null || excludeDetailIds.size() <= maxDeleteBatch) { // TODO: Wait for #3176
+ idsByParentId = many.findIdsByParentId(parentId, t, deleteMode.isHard(), excludeDetailIds);
+ } else {
+ // if we hit the parameter limit, we must filter that on the java side.
+ // There is no easy way to batch "not in" queries.
+ // checkme: We could pass the first 1000-2000 params to the DB and filter the rest
+ idsByParentId = many.findIdsByParentId(parentId, t, deleteMode.isHard(), null);
+ idsByParentId.removeIf(id -> excludeDetailIds.contains(id));
+ }
if (!idsByParentId.isEmpty()) {
deleteChildrenById(t, targetDesc, idsByParentId, deleteMode);
}
@@ -1046,7 +1132,7 @@ public final class DefaultPersister implements Persister {
deleteCascade(refList, t, deleteMode, true);
} else {
// perform delete by statement if possible
- delete(targetDesc, null, childIds, t, deleteMode);
+ delete(targetDesc, childIds, t, deleteMode);
}
}
diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBase.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBase.java
index 88c7ae6a7..9103340ff 100644
--- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBase.java
+++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBase.java
@@ -48,7 +48,7 @@ abstract class SaveManyBase implements SaveMany {
final void preElementCollectionUpdate() {
if (!insertedParent) {
request.preElementCollectionUpdate();
- persister.addToFlushQueue(many.deleteByParentId(request.beanId(), null), transaction, BatchControl.DELETE_QUEUE);
+ persister.addToFlushQueue(many.deleteByParentId(request.beanId()), transaction, BatchControl.DELETE_QUEUE);
}
}
diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java
index 68c446fed..00805a892 100644
--- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java
+++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java
@@ -9,7 +9,10 @@ import io.ebeaninternal.server.core.PersistRequestBean;
import io.ebeaninternal.server.deploy.*;
import javax.persistence.PersistenceException;
-import java.util.*;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
import static io.ebeaninternal.server.persist.DmlUtil.isNullOrZero;
import static java.lang.System.Logger.Level.WARNING;
@@ -205,9 +208,10 @@ final class SaveManyBeans extends SaveManyBase {
/**
* Return the Id values of beans we know are being updated (any others are orphans)
+ * If there are no IDs, null is returned.
*/
- private List detailIds() {
- final var detailIds = new ArrayList<>();
+ private Set detailIds() {
+ final var detailIds = new HashSet<>();
for (Object detailBean : collection) {
if (isMap) {
detailBean = ((Map.Entry, ?>) detailBean).getValue();
@@ -222,7 +226,7 @@ final class SaveManyBeans extends SaveManyBase {
}
}
}
- return detailIds;
+ return detailIds.isEmpty() ? null : detailIds;
}
/**
diff --git a/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java b/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java
index 7b3f85bfb..5d560d2ed 100644
--- a/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java
+++ b/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java
@@ -7,56 +7,147 @@ import org.junit.jupiter.api.Test;
import java.util.ArrayList;
import java.util.List;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
import static java.util.Objects.requireNonNull;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
class TestOrphanCollectionReplacement extends BaseTestCase {
@Test
- void replaceCollection_whenOrphan_expect_forcedInsert() {
- long parentId;
- { // setup
- List children = new ArrayList<>();
- children.add(new COOneMany("c0"));
- children.add(new COOneMany("c1"));
-
- COOne parent = new COOne("p0");
- parent.setChildren(children);
-
- DB.save(parent);
- parentId = parent.getId();
+ void replaceCollection_whenOrphan_expect_forcedInsertWithStatement() {
+ long parentId = setup(1000); // can be handled by statement for SqlServer
+ List sql = doUpdate(parentId, name -> !"c0".equals(name));
+ if (isH2() || isPostgresCompatible()) { // using deleted=true vs deleted=1
+ assertThat(sql).hasSize(4);
+ assertThat(sql.get(0)).contains("update coone_many set deleted=true where coone_id = ? and not ( id ");
+ assertThat(sql.get(1)).contains(" -- bind(");
+ assertThat(sql.get(2)).contains("insert into coone_many (coone_id, name, deleted) values (?,?,?)");
+ assertThat(sql.get(3)).contains(" -- bind(");
}
- { // act
- COOne fetchedParent = DB.find(COOne.class, parentId);
- assert fetchedParent != null;
+ if (isSqlServer()) {
+ // statement mode
+ assertThat(sql).hasSize(4);
+ assertThat(sql.get(0)).contains("update coone_many set deleted=1 where coone_id = ? and not ( id ");
+ assertThat(sql.get(1)).contains(" -- bind(");
+ assertThat(sql.get(2)).contains("insert into coone_many (id, coone_id, name, deleted) values (?,?,?,?)");
+ assertThat(sql.get(3)).contains(" -- bind(");
+ }
+ COOne fetchedUser2 = DB.find(COOne.class, parentId);
+ requireNonNull(fetchedUser2);
+ assertThat(fetchedUser2.getChildren())
+ .hasSize(1000)
+ .extracting(COOneMany::getName)
+ .doesNotContain("c0")// filtered
+ .contains("c1")
+ .contains("cTest"); // added
+ }
- COOneMany role = new COOneMany("c2");
+ @Test
+ void replaceCollection_whenOrphan_expect_forcedInsertWithFilter() {
+ long parentId = setup(2500); // we cannot make a "not in" query for so many params
+ List sql = doUpdate(parentId, name -> !"c0".equals(name));
+ if (isH2() || isPostgresCompatible()) { // using deleted=true vs deleted=1
+ // CHECKME: H2 would not require the batch mode here and could theoretically do it in fewer statements
+ assertThat(sql).hasSize(5);
+ assertThat(sql.get(0)).contains("select t0.id from coone_many t0 where coone_id=? and t0.deleted = false and t0.deleted = false; --bind");
+ assertThat(sql.get(1)).contains("update coone_many set deleted=true where id in (?)");
+ assertThat(sql.get(2)).contains(" -- bind(");
+ assertThat(sql.get(3)).contains("insert into coone_many (coone_id, name, deleted) values (?,?,?)");
+ assertThat(sql.get(4)).contains(" -- bind(");
+ }
- List filtered = fetchedParent.getChildren().stream().filter(r -> "c0".equals(r.getName())).collect(Collectors.toList());
+ if (isSqlServer()) {
+ // filter mode
+ assertThat(sql).hasSize(5);
+ assertThat(sql.get(0)).contains("select t0.id from coone_many t0 where coone_id=? and t0.deleted = 0 and t0.deleted = 0; --bind");
+ assertThat(sql.get(1)).contains("update coone_many set deleted=1 where id in (?)");
+ assertThat(sql.get(2)).contains(" -- bind(");
+ assertThat(sql.get(3)).contains("insert into coone_many (id, coone_id, name, deleted) values (?,?,?,?)");
+ assertThat(sql.get(4)).contains(" -- bind(");
+ }
+ COOne fetchedUser2 = DB.find(COOne.class, parentId);
+ requireNonNull(fetchedUser2);
+ assertThat(fetchedUser2.getChildren())
+ .hasSize(2500)
+ .extracting(COOneMany::getName)
+ .doesNotContain("c0")// filtered
+ .contains("c1")
+ .contains("cTest"); // added
+ }
- List updatedRoles = new ArrayList<>();
- updatedRoles.addAll(filtered);
- updatedRoles.addAll(List.of(role));
- fetchedParent.setChildren(updatedRoles);
+ @Test
+ void replaceCollection_whenOrphan_expect_forcedInsertWithManyReplacement() {
+ long parentId = setup(5000); // we will replace 2500 beans in this step
+ List sql = doUpdate(parentId, name -> Integer.parseInt(name.substring(1)) >= 2500);
+ if (isH2() || isPostgresCompatible()) { // using deleted=true vs deleted=1
+ // CHECKME: H2 would not require the batch mode here and could theoretically do it in fewer statements
+ assertThat(sql).hasSize(8);
+ assertThat(sql.get(0)).contains("select t0.id from coone_many t0 where coone_id=? and t0.deleted = false and t0.deleted = false; --bind"); // find all Ids
+ assertThat(sql.get(1)).contains("update coone_many set deleted=true where id in (?,?,?");
+ assertThat(sql.get(2)).contains(" -- bind(Array[1000]="); // update first 1000
+ assertThat(sql.get(3)).contains(" -- bind(Array[1000]="); // update second 1000
+ assertThat(sql.get(4)).contains("update coone_many set deleted=true where id in (?,?,?");
+ assertThat(sql.get(5)).contains(" -- bind(Array[500]="); // update last 500
+ assertThat(sql.get(6)).contains("insert into coone_many (coone_id, name, deleted) values (?,?,?)");
+ assertThat(sql.get(7)).contains(" -- bind(");
+ }
- LoggedSql.start();
- DB.save(fetchedParent);
- var sql = LoggedSql.stop();
- if (isH2() || isPostgresCompatible()) { // using deleted=true vs deleted=1
- assertThat(sql).hasSize(4);
- assertThat(sql.get(0)).contains("update coone_many set deleted=true where coone_id = ? and not ( id ");
- assertThat(sql.get(1)).contains(" -- bind(");
- assertThat(sql.get(2)).contains("insert into coone_many (coone_id, name, deleted) values (?,?,?)");
- assertThat(sql.get(3)).contains(" -- bind(");
- }
+ if (isSqlServer()) {
+ assertThat(sql).hasSize(7);
+ assertThat(sql.get(0)).contains("select t0.id from coone_many t0 where coone_id=? and t0.deleted = 0 and t0.deleted = 0; --bind"); // find all Ids
+ assertThat(sql.get(1)).contains("update coone_many set deleted=1 where id in (?,?,?");
+ assertThat(sql.get(2)).contains(" -- bind(Array[2000]="); // update first 2000
+ assertThat(sql.get(3)).contains("update coone_many set deleted=1 where id in (?,?,?");
+ assertThat(sql.get(4)).contains(" -- bind(Array[500]="); // update next 500
+ assertThat(sql.get(5)).contains("insert into coone_many (id, coone_id, name, deleted) values (?,?,?,?)");
+ assertThat(sql.get(6)).contains(" -- bind(");
}
COOne fetchedUser2 = DB.find(COOne.class, parentId);
requireNonNull(fetchedUser2);
- assertEquals(2, fetchedUser2.getChildren().size());
+ assertThat(fetchedUser2.getChildren())
+ .hasSize(2501)
+ .extracting(COOneMany::getName)
+ .doesNotContain("c0")// filtered
+ .contains("c2500")
+ .contains("cTest"); // added
+ }
+
+
+ private static List doUpdate(long parentId, Predicate filter) {
+ COOne fetchedParent = DB.find(COOne.class, parentId);
+ assert fetchedParent != null;
+
+
+ List filtered = fetchedParent.getChildren().stream().filter(r -> filter.test(r.getName())).collect(Collectors.toList());
+
+ List updatedRoles = new ArrayList<>();
+ updatedRoles.addAll(filtered);
+ updatedRoles.add(new COOneMany("cTest"));
+ fetchedParent.setChildren(updatedRoles);
+
+ LoggedSql.start();
+ DB.save(fetchedParent);
+ return LoggedSql.stop();
+ }
+
+ private static long setup(int count) {
+ long parentId;
+ // setup
+ List children = new ArrayList<>();
+ for (int i = 0; i < count; i++) {
+ children.add(new COOneMany("c" + i));
+
+ }
+
+ COOne parent = new COOne("p0");
+ parent.setChildren(children);
+
+ DB.save(parent);
+ parentId = parent.getId();
+ return parentId;
}
}
diff --git a/ebean-test/src/test/java/org/tests/compositekeys/TestOnCascadeDeleteChildrenWithCompositeKeys.java b/ebean-test/src/test/java/org/tests/compositekeys/TestOnCascadeDeleteChildrenWithCompositeKeys.java
index 0df82645c..7a1c8ae6c 100644
--- a/ebean-test/src/test/java/org/tests/compositekeys/TestOnCascadeDeleteChildrenWithCompositeKeys.java
+++ b/ebean-test/src/test/java/org/tests/compositekeys/TestOnCascadeDeleteChildrenWithCompositeKeys.java
@@ -1,10 +1,10 @@
package org.tests.compositekeys;
-import io.ebean.xtest.BaseTestCase;
import io.ebean.CountDistinctOrder;
import io.ebean.DB;
import io.ebean.Query;
import io.ebean.annotation.Identity;
+import io.ebean.xtest.BaseTestCase;
import io.ebeaninternal.api.SpiEbeanServer;
import io.ebeaninternal.server.deploy.BeanDescriptor;
import io.ebeaninternal.server.deploy.BeanPropertyAssocMany;
@@ -82,8 +82,8 @@ public class TestOnCascadeDeleteChildrenWithCompositeKeys extends BaseTestCase {
ids.add(1L);
ids.add(2L);
- beanProperty.findIdsByParentId(null, ids, null, null, true);
- beanProperty.findIdsByParentId(1L, null, null, null, true);
+ beanProperty.findIdsByParentIdList(ids, null, true);
+ beanProperty.findIdsByParentId(1L, null, true);
}
/**
@@ -107,18 +107,18 @@ public class TestOnCascadeDeleteChildrenWithCompositeKeys extends BaseTestCase {
if (isH2() || isMariaDB() || isPostgresCompatible()) {
assertThat(query1.getGeneratedSql()).contains("select distinct r1.attribute_, count(*) from "
- + "(select distinct t0.user_id, t0.role_id, t1.name as attribute_ "
- + "from em_user_role t0 join em_user t1 on t1.id = t0.user_id) r1 "
- + "group by r1.attribute_ order by count(*) desc, r1.attribute_ limit 20");
+ + "(select distinct t0.user_id, t0.role_id, t1.name as attribute_ "
+ + "from em_user_role t0 join em_user t1 on t1.id = t0.user_id) r1 "
+ + "group by r1.attribute_ order by count(*) desc, r1.attribute_ limit 20");
} else if (isDb2()) {
assertThat(query1.getGeneratedSql()).contains("select distinct r1.attribute_, count(*) from "
- + "(select distinct t0.user_id, t0.role_id, t1.name as attribute_ "
- + "from em_user_role t0 join em_user t1 on t1.id = t0.user_id) r1 "
- + "group by r1.attribute_ order by count(*) desc, r1.attribute_ fetch next 20 rows only");
+ + "(select distinct t0.user_id, t0.role_id, t1.name as attribute_ "
+ + "from em_user_role t0 join em_user t1 on t1.id = t0.user_id) r1 "
+ + "group by r1.attribute_ order by count(*) desc, r1.attribute_ fetch next 20 rows only");
} else if (isSqlServer()) {
assertThat(query1.getGeneratedSql()).contains("select distinct top 20 r1.attribute_, count(*) "
- + "from (select distinct t0.user_id, t0.role_id, t1.name as attribute_ from em_user_role t0 "
- + "join em_user t1 on t1.id = t0.user_id) r1 group by r1.attribute_ order by count(*) desc, r1.attribute_");
+ + "from (select distinct t0.user_id, t0.role_id, t1.name as attribute_ from em_user_role t0 "
+ + "join em_user t1 on t1.id = t0.user_id) r1 group by r1.attribute_ order by count(*) desc, r1.attribute_");
} else {
// no Oracle test yet
}