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 bdb8400fe..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 @@ -586,12 +586,12 @@ public abstract class BeanPropertyAssoc extends BeanProperty implements STree /** * Find child beans of the parent id. */ - public abstract List findIdsByParentId(Object id, Transaction transaction, boolean hard); + public abstract List findIdsByParentId(Object id, Transaction transaction, boolean includeSoftDeletes); /** * Find child beans of the parent ids in idList. */ - public abstract List findIdsByParentIdList(List idList, Transaction transaction, boolean hard); + public abstract List findIdsByParentIdList(List idList, Transaction transaction, boolean includeSoftDeletes); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java index 3990235fc..c9899888c 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocMany.java @@ -311,23 +311,23 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc implements ST * Find the Id's of detail beans given a parent Id */ @Override - public List findIdsByParentId(Object parentId, Transaction t, boolean hard) { - return sqlHelp.findIdsByParentId(parentId, t, hard, null); + public List findIdsByParentId(Object parentId, Transaction t, boolean includeSoftDeletes) { + return sqlHelp.findIdsByParentId(parentId, t, includeSoftDeletes, null); } /** * Find the Id's of detail beans given a parent Id and optionally exclude detail IDs */ - public List findIdsByParentId(Object parentId, Transaction t, boolean hard, Set excludeDetailIds) { - return sqlHelp.findIdsByParentId(parentId, t, hard, excludeDetailIds); + public List findIdsByParentId(Object parentId, Transaction t, boolean includeSoftDeletes, Set excludeDetailIds) { + return sqlHelp.findIdsByParentId(parentId, t, includeSoftDeletes, excludeDetailIds); } /** * Find the Id's of detail beans given a list of parent Id's. */ @Override - public List findIdsByParentIdList(List parentIdList, Transaction t, boolean hard) { - return sqlHelp.findIdsByParentIdList(parentIdList, t, hard); + public List findIdsByParentIdList(List parentIdList, Transaction t, boolean includeSoftDeletes) { + return sqlHelp.findIdsByParentIdList(parentIdList, t, includeSoftDeletes); } /** diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManySqlHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManySqlHelp.java index b66250aac..f3355e1d5 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManySqlHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocManySqlHelp.java @@ -124,33 +124,25 @@ class BeanPropertyAssocManySqlHelp { many.bindParentIdsIn(rawWhere, parentIds, query); } - List findIdsByParentId(Object parentId, Transaction t, boolean hard, Set excludeDetailIds) { + List findIdsByParentId(Object parentId, Transaction t, boolean includeSoftDeletes, Set excludeDetailIds) { final SpiEbeanServer server = descriptor.ebeanServer(); final SpiQuery query = many.newQuery(server); many.bindParentIdEq(rawParentIdEQ(""), parentId, query); - if (hard) { + if (includeSoftDeletes) { query.setIncludeSoftDeletes(); } if (excludeDetailIds != null && !excludeDetailIds.isEmpty()) { - if (excludeDetailIds.size() > 1000) { // TODO: Wait for #3176 - // 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 - List ret = server.findIds(query, t); - ret.removeIf(id -> excludeDetailIds.contains(id)); - return ret; - } query.where().not(query.getExpressionFactory().idIn(excludeDetailIds)); } return server.findIds(query, t); } - List findIdsByParentIdList(List parentIds, Transaction t, boolean hard) { + List findIdsByParentIdList(List parentIds, Transaction t, boolean includeSoftDeletes) { final SpiEbeanServer server = descriptor.ebeanServer(); final SpiQuery query = many.newQuery(server); many.bindParentIdsIn(rawParentIdIN("", parentIds.size()), parentIds, query); - if (hard) { + if (includeSoftDeletes) { query.setIncludeSoftDeletes(); } return server.findIds(query, t); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java index ad12b495d..d4dcab4f9 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java @@ -254,21 +254,29 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr } - public List findIdsByParentId(Object parentId, Transaction t, boolean hard) { + @Override + public List findIdsByParentId(Object parentId, Transaction t, boolean includeSoftDeletes) { String rawWhere = deriveWhereParentIdSql(false); SpiEbeanServer server = server(); Query q = server.find(type()); bindParentIdEq(rawWhere, parentId, q); + if (includeSoftDeletes) { + q.setIncludeSoftDeletes(); + } return server.findIds(q, t); } - public List findIdsByParentIdList(List parentIds, Transaction t, boolean hard) { + @Override + public List findIdsByParentIdList(List parentIds, Transaction t, boolean includeSoftDeletes) { String rawWhere = deriveWhereParentIdSql(true); String inClause = idBinder().idInValueExpr(false, parentIds.size()); String expr = rawWhere + inClause; SpiEbeanServer server = server(); Query q = server.find(type()); bindParentIdsIn(expr, parentIds, q); + if (includeSoftDeletes) { + q.setIncludeSoftDeletes(); + } return server.findIds(q, t); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java index e813d8511..8cb2eef03 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java @@ -697,7 +697,7 @@ public final class DefaultPersister implements Persister { if (deleteMode.isHard() && targetDesc.isDeleteByStatement()) { executeSqlUpdate(sqlDeleteChildren(expOne), transaction); } else { - List childIds = findChildIds(expOne, true); // CHECMKE: do we need something for soft/hard delete here + List childIds = findChildIds(expOne, deleteMode.isHard()); if (childIds != null && !childIds.isEmpty()) { deleteChildrenById(transaction, targetDesc, childIds, deleteMode); } @@ -746,7 +746,7 @@ public final class DefaultPersister implements Persister { abstract SqlUpdate sqlDeleteChildren(BeanPropertyAssoc prop); - abstract List findChildIds(BeanPropertyAssoc prop, boolean hard); + abstract List findChildIds(BeanPropertyAssoc prop, boolean includeSoftDeletes); abstract int deleteBeans(); } @@ -783,8 +783,8 @@ public final class DefaultPersister implements Persister { } @Override - List findChildIds(BeanPropertyAssoc prop, boolean hard) { - return prop.findIdsByParentId(id, transaction, hard); + List findChildIds(BeanPropertyAssoc prop, boolean includeSoftDeletes) { + return prop.findIdsByParentId(id, transaction, includeSoftDeletes); } int deleteBeans() { @@ -834,8 +834,8 @@ public final class DefaultPersister implements Persister { } @Override - List findChildIds(BeanPropertyAssoc prop, boolean hard) { - return prop.findIdsByParentIdList(idList, transaction, hard); + List findChildIds(BeanPropertyAssoc prop, boolean includeSoftDeletes) { + return prop.findIdsByParentIdList(idList, transaction, includeSoftDeletes); } int deleteBeans() { @@ -1088,7 +1088,7 @@ public final class DefaultPersister implements Persister { BeanDescriptor targetDesc = many.targetDescriptor(); if (deleteMode.isHard() || targetDesc.isSoftDelete()) { if (targetDesc.isDeleteByStatement() - && (excludeDetailIds == null || excludeDetailIds.size() <= 1000)) { // TODO wait for #3176 + && (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); @@ -1099,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, t, deleteMode.isHard(), excludeDetailIds); + 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); } 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 9c9b552df..5d560d2ed 100644 --- a/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java +++ b/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java @@ -50,11 +50,13 @@ class TestOrphanCollectionReplacement extends BaseTestCase { 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 - 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("); + // 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("); } if (isSqlServer()) { @@ -81,15 +83,19 @@ class TestOrphanCollectionReplacement extends BaseTestCase { 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 - 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("); + // 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("); } if (isSqlServer()) { - // filter mode 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 (?,?,?");