From d9d09b31b4c0c725a564e50e9c64b8dae2eaddd6 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 21 Aug 2023 15:36:05 +0200 Subject: [PATCH 1/3] Refactored delete-by-id --- .../server/deploy/BeanPropertyAssoc.java | 24 ++ .../server/deploy/BeanPropertyAssocMany.java | 41 ++- .../deploy/BeanPropertyAssocManySqlHelp.java | 9 +- .../server/deploy/BeanPropertyAssocOne.java | 24 +- .../server/persist/DefaultPersister.java | 282 +++++++++++------- .../server/persist/SaveManyBase.java | 2 +- ...ascadeDeleteChildrenWithCompositeKeys.java | 22 +- 7 files changed, 252 insertions(+), 152 deletions(-) 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..bdb8400fe 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 idList); + + /** + * Find child beans of the parent id. + */ + public abstract List findIdsByParentId(Object id, Transaction transaction, boolean hard); + + /** + * Find child beans of the parent ids in idList. + */ + public abstract List findIdsByParentIdList(List idList, Transaction transaction, boolean hard); + } + 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 61c9e4ecb..fa00497d9 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 @@ -296,23 +296,38 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc implements ST // do not add to the selectChain at the top level of the Many bean } - public SpiSqlUpdate deleteByParentId(Object parentId, List parentIdist) { - if (parentId != null) { - return sqlHelp.deleteByParentId(parentId); - } else { - return sqlHelp.deleteByParentIdList(parentIdist); - } + @Override + public SpiSqlUpdate deleteByParentId(Object parentId) { + return sqlHelp.deleteByParentId(parentId); + } + + @Override + public SpiSqlUpdate deleteByParentIdList(List parentIdist) { + return sqlHelp.deleteByParentIdList(parentIdist); + } + + + /** + * 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); } /** - * Find the Id's of detail beans given a parent Id or list of parent Id's. + * Find the Id's of detail beans given a parent Id and optionally exclude detail IDs */ - public List findIdsByParentId(Object parentId, List parentIdList, Transaction t, List excludeDetailIds, boolean hard) { - if (parentId != null) { - return sqlHelp.findIdsByParentId(parentId, t, excludeDetailIds, hard); - } else { - return sqlHelp.findIdsByParentIdList(parentIdList, t, excludeDetailIds, hard); - } + public List findIdsByParentId(Object parentId, Transaction t, boolean hard, List excludeDetailIds) { + return sqlHelp.findIdsByParentId(parentId, t, hard, 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); } /** 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 918d04d85..de50f63f1 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 @@ -4,9 +4,9 @@ import io.ebean.Transaction; import io.ebeaninternal.api.SpiEbeanServer; import io.ebeaninternal.api.SpiQuery; import io.ebeaninternal.api.SpiSqlUpdate; +import io.ebeaninternal.server.core.DefaultSqlUpdate; import io.ebeaninternal.server.deploy.visitor.BaseTablePropertyVisitor; import io.ebeaninternal.server.deploy.visitor.VisitProperties; -import io.ebeaninternal.server.core.DefaultSqlUpdate; import io.ebeaninternal.server.util.Str; import java.util.List; @@ -123,7 +123,7 @@ class BeanPropertyAssocManySqlHelp { many.bindParentIdsIn(rawWhere, parentIds, query); } - List findIdsByParentId(Object parentId, Transaction t, List excludeDetailIds, boolean hard) { + List findIdsByParentId(Object parentId, Transaction t, boolean hard, List excludeDetailIds) { final SpiEbeanServer server = descriptor.ebeanServer(); final SpiQuery query = many.newQuery(server); many.bindParentIdEq(rawParentIdEQ(""), parentId, query); @@ -136,16 +136,13 @@ class BeanPropertyAssocManySqlHelp { return server.findIds(query, t); } - List findIdsByParentIdList(List parentIds, Transaction t, List excludeDetailIds, boolean hard) { + List findIdsByParentIdList(List parentIds, Transaction t, boolean hard) { final SpiEbeanServer server = descriptor.ebeanServer(); final SpiQuery query = many.newQuery(server); many.bindParentIdsIn(rawParentIdIN("", parentIds.size()), parentIds, query); if (hard) { query.setIncludeSoftDeletes(); } - if (excludeDetailIds != null && !excludeDetailIds.isEmpty()) { - query.where().not(query.getExpressionFactory().idIn(excludeDetailIds)); - } 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 af7a8efb5..ad12b495d 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 @@ -237,36 +237,24 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr return encrypted ? elPlaceHolderEncrypted : elPlaceHolder; } - public SqlUpdate deleteByParentId(Object parentId, List parentIdist) { - if (parentId != null) { - return deleteByParentId(parentId); - } else { - return deleteByParentIdList(parentIdist); - } - } - private SqlUpdate deleteByParentIdList(List parentIds) { + @Override + public SqlUpdate deleteByParentIdList(List parentIds) { String sql = deleteByParentIdInSql + targetIdBinder.idInValueExpr(false, parentIds.size()); DefaultSqlUpdate delete = new DefaultSqlUpdate(sql); bindParentIds(delete, parentIds); return delete; } - private SqlUpdate deleteByParentId(Object parentId) { + @Override + public SqlUpdate deleteByParentId(Object parentId) { DefaultSqlUpdate delete = new DefaultSqlUpdate(deleteByParentIdSql); bindParentId(delete, parentId); return delete; } - public List findIdsByParentId(Object parentId, List parentIds, Transaction t) { - if (parentId != null) { - return findIdsByParentId(parentId, t); - } else { - return findIdsByParentIdList(parentIds, t); - } - } - private List findIdsByParentId(Object parentId, Transaction t) { + public List findIdsByParentId(Object parentId, Transaction t, boolean hard) { String rawWhere = deriveWhereParentIdSql(false); SpiEbeanServer server = server(); Query q = server.find(type()); @@ -274,7 +262,7 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr return server.findIds(q, t); } - private List findIdsByParentIdList(List parentIds, Transaction t) { + public List findIdsByParentIdList(List parentIds, Transaction t, boolean hard) { String rawWhere = deriveWhereParentIdSql(true); String inClause = idBinder().idInValueExpr(false, parentIds.size()); String expr = rawWhere + inClause; 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 cada1318b..ef897b02d 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 @@ -605,7 +605,7 @@ public final class DefaultPersister implements Persister { // convert to appropriate type if required idList.add(descriptor.convertId(id)); } - return delete(descriptor, null, idList, transaction, deleteMode); + return delete(descriptor, idList, transaction, deleteMode); } /** @@ -642,146 +642,222 @@ public final class DefaultPersister implements Persister { id = descriptor.convertId(id); DeleteMode deleteMode = (permanent || !descriptor.isSoftDelete()) ? DeleteMode.HARD : DeleteMode.SOFT; - return delete(descriptor, id, null, transaction, deleteMode); + return delete(descriptor, id, transaction, deleteMode); } @Override public int deleteByIds(BeanDescriptor descriptor, List idList, Transaction transaction, boolean permanent) { DeleteMode deleteMode = (permanent || !descriptor.isSoftDelete()) ? DeleteMode.HARD : DeleteMode.SOFT; - return delete(descriptor, null, idList, transaction, deleteMode); + return delete(descriptor, idList, transaction, deleteMode); + } + + private int delete(BeanDescriptor descriptor, List idList, Transaction transaction, DeleteMode deleteMode) { + if (idList == null || idList.size() <= maxDeleteBatch) { + return new DeleteBatchHelpMultiple(descriptor, idList, transaction, deleteMode).deleteBatch(); + } + int rows = 0; + for (List batchOfIds : Lists.partition(idList, maxDeleteBatch)) { + rows += new DeleteBatchHelpMultiple(descriptor, batchOfIds, transaction, deleteMode).deleteBatch(); + } + return rows; } /** * Delete by Id or a List of Id's. */ - private int delete(BeanDescriptor descriptor, Object id, List idList, Transaction transaction, DeleteMode deleteMode) { - if (idList == null || idList.size() <= maxDeleteBatch) { - return deleteBatch(descriptor, id, idList, transaction, deleteMode); - } - int rows = 0; - for (List batchOfIds : Lists.partition(idList, maxDeleteBatch)) { - rows += deleteBatch(descriptor, id, batchOfIds, transaction, deleteMode); - } - return rows; + private int delete(BeanDescriptor descriptor, Object id, Transaction transaction, DeleteMode deleteMode) { + return new DeleteBatchHelpSingle(descriptor, id, transaction, deleteMode).deleteBatch(); } - private int deleteBatch(BeanDescriptor descriptor, Object id, List idList, Transaction transaction, DeleteMode deleteMode) { - SpiTransaction t = (SpiTransaction) transaction; - if (t.isPersistCascade()) { - BeanPropertyAssocOne[] propImportDelete = descriptor.propertiesOneImportedDelete(); - if (propImportDelete.length > 0) { - // We actually need to execute a query to get the foreign key values - // as they are required for the delete cascade. Query back just the - // Id and the appropriate foreign key values - Query q = deleteRequiresQuery(descriptor, propImportDelete, deleteMode); - if (idList != null) { - q.where().idIn(idList); - if (t.isLogSummary()) { - t.logSummary("-- DeleteById of {0} ids[{1}] requires fetch of foreign key values", descriptor.name(), idList); - } - List beanList = server.findList(q, t); - deleteCascade(beanList, t, deleteMode, false); - return beanList.size(); + private abstract class DeleteBatchHelp { + final BeanDescriptor descriptor; + final SpiTransaction transaction; + final DeleteMode deleteMode; - } else { - q.where().idEq(id); - if (t.isLogSummary()) { - t.logSummary("-- DeleteById of {0} id[{1}] requires fetch of foreign key values", descriptor.name(), id); - } - EntityBean bean = (EntityBean) server.findOne(q, t); - if (bean == null) { - return 0; - } else { - return deleteRecurse(bean, t, deleteMode); - } - } - } + public DeleteBatchHelp(BeanDescriptor descriptor, Transaction transaction, DeleteMode deleteMode) { + this.descriptor = descriptor; + this.transaction = (SpiTransaction) transaction; + this.deleteMode = deleteMode; } - if (t.isPersistCascade()) { - // OneToOne exported side with delete cascade - BeanPropertyAssocOne[] expOnes = descriptor.propertiesOneExportedDelete(); - for (BeanPropertyAssocOne expOne : expOnes) { - BeanDescriptor targetDesc = expOne.targetDescriptor(); - // only cascade soft deletes when supported by target - if (deleteMode.isHard() || targetDesc.isSoftDelete()) { - if (deleteMode.isHard() && targetDesc.isDeleteByStatement()) { - SqlUpdate sqlDelete = expOne.deleteByParentId(id, idList); - executeSqlUpdate(sqlDelete, t); - } else { - List childIds = expOne.findIdsByParentId(id, idList, t); - if (childIds != null && !childIds.isEmpty()) { - deleteChildrenById(t, targetDesc, childIds, deleteMode); - } - } - } - } + int deleteBatch() { - // OneToMany's with delete cascade - BeanPropertyAssocMany[] manys = descriptor.propertiesManyDelete(); - for (BeanPropertyAssocMany many : manys) { - if (!many.isManyToMany()) { - BeanDescriptor targetDesc = many.targetDescriptor(); + if (transaction.isPersistCascade()) { + BeanPropertyAssocOne[] propImportDelete = descriptor.propertiesOneImportedDelete(); + if (propImportDelete.length > 0) { + return deleteImported(propImportDelete); + } + + // OneToOne exported side with delete cascade + BeanPropertyAssocOne[] expOnes = descriptor.propertiesOneExportedDelete(); + for (BeanPropertyAssocOne expOne : expOnes) { + BeanDescriptor targetDesc = expOne.targetDescriptor(); // only cascade soft deletes when supported by target if (deleteMode.isHard() || targetDesc.isSoftDelete()) { if (deleteMode.isHard() && targetDesc.isDeleteByStatement()) { - // we can just delete children with a single statement - SqlUpdate sqlDelete = many.deleteByParentId(id, idList); - executeSqlUpdate(sqlDelete, t); + executeSqlUpdate(sqlDeleteChildren(expOne), transaction); } else { - // we need to fetch the Id's to delete (recurse or notify L2 cache) - List childIds = many.findIdsByParentId(id, idList, t, null, deleteMode.isHard()); - if (!childIds.isEmpty()) { - delete(targetDesc, null, childIds, t, deleteMode); + List childIds = findChildIds(expOne, true); // CHECMKE: do we need something for soft/hard delete here + if (childIds != null && !childIds.isEmpty()) { + deleteChildrenById(transaction, targetDesc, childIds, deleteMode); + } + } + } + } + + // OneToMany's with delete cascade + BeanPropertyAssocMany[] manys = descriptor.propertiesManyDelete(); + for (BeanPropertyAssocMany many : manys) { + if (!many.isManyToMany()) { + BeanDescriptor targetDesc = many.targetDescriptor(); + // only cascade soft deletes when supported by target + if (deleteMode.isHard() || targetDesc.isSoftDelete()) { + if (deleteMode.isHard() && targetDesc.isDeleteByStatement()) { + // we can just delete children with a single statement + executeSqlUpdate(sqlDeleteChildren(many), transaction); + } else { + // we need to fetch the Id's to delete (recurse or notify L2 cache) + List childIds = findChildIds(many, deleteMode.isHard()); + if (!childIds.isEmpty()) { + delete(targetDesc, childIds, transaction, deleteMode); + } } } } } } - } - if (deleteMode.isHard()) { - // ManyToMany's ... delete from intersection table - BeanPropertyAssocMany[] manys = descriptor.propertiesManyToMany(); - for (BeanPropertyAssocMany many : manys) { - SqlUpdate sqlDelete = many.deleteByParentId(id, idList); - if (t.isLogSummary()) { - t.logSummary("-- Deleting intersection table entries: {0}", many.fullName()); + if (deleteMode.isHard()) { + // ManyToMany's ... delete from intersection table + BeanPropertyAssocMany[] manys = descriptor.propertiesManyToMany(); + for (BeanPropertyAssocMany many : manys) { + if (transaction.isLogSummary()) { + transaction.logSummary("-- Deleting intersection table entries: {0}", many.fullName()); + } + executeSqlUpdate(sqlDeleteChildren(many), transaction); } - executeSqlUpdate(sqlDelete, t); } + + // delete the bean(s) + return deleteBeans(); } - // delete the bean(s) - SqlUpdate deleteById = descriptor.deleteById(id, idList, deleteMode); - if (t.isLogSummary()) { - if (idList != null) { - t.logSummary("-- Deleting {0} Ids: {1}", descriptor.name(), idList); + abstract int deleteImported(BeanPropertyAssocOne[] propImportDelete); + + abstract SqlUpdate sqlDeleteChildren(BeanPropertyAssoc prop); + + abstract List findChildIds(BeanPropertyAssoc prop, boolean hard); + + abstract int deleteBeans(); + } + + private class DeleteBatchHelpSingle extends DeleteBatchHelp { + private final Object id; + + public DeleteBatchHelpSingle(BeanDescriptor descriptor, Object id, Transaction transaction, DeleteMode deleteMode) { + super(descriptor, transaction, deleteMode); + this.id = id; + } + + int deleteImported(BeanPropertyAssocOne[] propImportDelete) { + // We actually need to execute a query to get the foreign key values + // as they are required for the delete cascade. Query back just the + // Id and the appropriate foreign key values + Query q = deleteRequiresQuery(descriptor, propImportDelete, deleteMode); + q.where().idEq(id); + if (transaction.isLogSummary()) { + transaction.logSummary("-- DeleteById of {0} id[{1}] requires fetch of foreign key values", descriptor.name(), id); + } + EntityBean bean = (EntityBean) server.findOne(q, transaction); + if (bean == null) { + return 0; } else { - t.logSummary("-- Deleting {0} Id: {1}", descriptor.name(), id); + return deleteRecurse(bean, transaction, deleteMode); } + } - // use Id's to update L2 cache rather than Bulk table event - notifyDeleteById(descriptor, id, idList, transaction); - deleteById.setAutoTableMod(false); - if (idList != null) { - t.event().addDeleteByIdList(descriptor, idList); - } else { - t.event().addDeleteById(descriptor, id); + @Override + SqlUpdate sqlDeleteChildren(BeanPropertyAssoc prop) { + return prop.deleteByParentId(id); } - int rows = executeSqlUpdate(deleteById, t); - // Delete from the persistence context so that it can't be fetched again later - PersistenceContext persistenceContext = t.persistenceContext(); - if (idList != null) { + @Override + List findChildIds(BeanPropertyAssoc prop, boolean hard) { + return prop.findIdsByParentId(id, transaction, hard); + } + + int deleteBeans() { + SqlUpdate deleteById = descriptor.deleteById(id, null, deleteMode); + if (transaction.isLogSummary()) { + transaction.logSummary("-- Deleting {0} Id: {1}", descriptor.name(), id); + } + + // use Id's to update L2 cache rather than Bulk table event + notifyDeleteById(descriptor, id, null, transaction); + deleteById.setAutoTableMod(false); + transaction.event().addDeleteById(descriptor, id); + + int rows = executeSqlUpdate(deleteById, transaction); + + // Delete from the persistence context so that it can't be fetched again later + PersistenceContext persistenceContext = transaction.persistenceContext(); + descriptor.contextDeleted(persistenceContext, id); + return rows; + } + } + + private class DeleteBatchHelpMultiple extends DeleteBatchHelp { + private final List idList; + + public DeleteBatchHelpMultiple(BeanDescriptor descriptor, List idList, Transaction transaction, DeleteMode deleteMode) { + super(descriptor, transaction, deleteMode); + this.idList = idList; + } + + int deleteImported(BeanPropertyAssocOne[] propImportDelete) { + // We actually need to execute a query to get the foreign key values + // as they are required for the delete cascade. Query back just the + // Id and the appropriate foreign key values + Query q = deleteRequiresQuery(descriptor, propImportDelete, deleteMode); + q.where().idIn(idList); + if (transaction.isLogSummary()) { + transaction.logSummary("-- DeleteById of {0} ids[{1}] requires fetch of foreign key values", descriptor.name(), idList); + } + List beanList = server.findList(q, transaction); + deleteCascade(beanList, transaction, deleteMode, false); + return beanList.size(); + } + + SqlUpdate sqlDeleteChildren(BeanPropertyAssoc prop) { + return prop.deleteByParentIdList(idList); + } + + @Override + List findChildIds(BeanPropertyAssoc prop, boolean hard) { + return prop.findIdsByParentIdList(idList, transaction, hard); + } + + int deleteBeans() { + SqlUpdate deleteById = descriptor.deleteById(null, idList, deleteMode); + if (transaction.isLogSummary()) { + transaction.logSummary("-- Deleting {0} Ids: {1}", descriptor.name(), idList); + } + + // use Id's to update L2 cache rather than Bulk table event + notifyDeleteById(descriptor, null, idList, transaction); + deleteById.setAutoTableMod(false); + transaction.event().addDeleteByIdList(descriptor, idList); + + int rows = executeSqlUpdate(deleteById, transaction); + + // Delete from the persistence context so that it can't be fetched again later + PersistenceContext persistenceContext = transaction.persistenceContext(); for (Object idValue : idList) { descriptor.contextDeleted(persistenceContext, idValue); } - } else { - descriptor.contextDeleted(persistenceContext, id); + return rows; } - return rows; } private void notifyDeleteById(BeanDescriptor descriptor, Object id, List idList, Transaction transaction) { @@ -1022,7 +1098,7 @@ 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 = many.findIdsByParentId(parentId, t, deleteMode.isHard(), excludeDetailIds); if (!idsByParentId.isEmpty()) { deleteChildrenById(t, targetDesc, idsByParentId, deleteMode); } @@ -1046,7 +1122,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-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 } From 2a24ec3d49c210927d236c2601fd3202eab7eda0 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 21 Aug 2023 16:51:19 +0200 Subject: [PATCH 2/3] Added some tests --- .../server/deploy/BeanPropertyAssocMany.java | 4 +- .../deploy/BeanPropertyAssocManySqlHelp.java | 12 +- .../server/deploy/IntersectionRow.java | 5 +- .../server/persist/DefaultPersister.java | 5 +- .../server/persist/SaveManyBeans.java | 12 +- .../TestOrphanCollectionReplacement.java | 151 ++++++++++++++---- 6 files changed, 145 insertions(+), 44 deletions(-) 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 fa00497d9..3990235fc 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 @@ -318,7 +318,7 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc implements ST /** * 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, List excludeDetailIds) { + public List findIdsByParentId(Object parentId, Transaction t, boolean hard, Set excludeDetailIds) { return sqlHelp.findIdsByParentId(parentId, t, hard, excludeDetailIds); } @@ -755,7 +755,7 @@ public class BeanPropertyAssocMany extends BeanPropertyAssoc implements ST throw new PersistenceException(from + ": Could not find mapKey property " + mapKey + " on " + to); } - public IntersectionRow buildManyDeleteChildren(EntityBean parentBean, List excludeDetailIds) { + public IntersectionRow buildManyDeleteChildren(EntityBean parentBean, Set excludeDetailIds) { IntersectionRow row = new IntersectionRow(tableJoin.getTable(), targetDescriptor); if (excludeDetailIds != null && !excludeDetailIds.isEmpty()) { row.setExcludeIds(excludeDetailIds, targetDescriptor()); 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 de50f63f1..b66250aac 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 @@ -10,6 +10,7 @@ import io.ebeaninternal.server.deploy.visitor.VisitProperties; import io.ebeaninternal.server.util.Str; import java.util.List; +import java.util.Set; class BeanPropertyAssocManySqlHelp { @@ -123,14 +124,23 @@ class BeanPropertyAssocManySqlHelp { many.bindParentIdsIn(rawWhere, parentIds, query); } - List findIdsByParentId(Object parentId, Transaction t, boolean hard, List excludeDetailIds) { + List findIdsByParentId(Object parentId, Transaction t, boolean hard, Set excludeDetailIds) { final SpiEbeanServer server = descriptor.ebeanServer(); final SpiQuery query = many.newQuery(server); many.bindParentIdEq(rawParentIdEQ(""), parentId, query); if (hard) { 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); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/IntersectionRow.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/IntersectionRow.java index 76c732603..1475ef636 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/IntersectionRow.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/IntersectionRow.java @@ -11,13 +11,14 @@ import io.ebeaninternal.server.persist.DeleteMode; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; public final class IntersectionRow { private final String tableName; private final BeanDescriptor targetDescriptor; private final LinkedHashMap values = new LinkedHashMap<>(); - private List excludeIds; + private Set excludeIds; private BeanDescriptor excludeDescriptor; IntersectionRow(String tableName, BeanDescriptor targetDescriptor) { @@ -33,7 +34,7 @@ public final class IntersectionRow { /** * Set Id's to exclude. This is for deleting non-attached detail Id's. */ - void setExcludeIds(List excludeIds, BeanDescriptor excludeDescriptor) { + void setExcludeIds(Set excludeIds, BeanDescriptor excludeDescriptor) { this.excludeIds = excludeIds; this.excludeDescriptor = excludeDescriptor; } 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 ef897b02d..e813d8511 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 @@ -1082,12 +1082,13 @@ public final class DefaultPersister implements Persister { *

*/ 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() <= 1000)) { // TODO wait for #3176 // Just delete all the children with one statement IntersectionRow intRow = many.buildManyDeleteChildren(parentBean, excludeDetailIds); SqlUpdate sqlDelete = intRow.createDelete(server, deleteMode); 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..9c9b552df 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,141 @@ 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 + 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("); + } - 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 + 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("); + } - 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()) { + // 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 (?,?,?"); + 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; } } From 77aeda17532e10e00ce1f4ac35151e22b0740f42 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Mon, 21 Aug 2023 17:10:23 +0200 Subject: [PATCH 3/3] draft: elliminate the "not in" query that may 'explode' on SqlServer --- .../server/deploy/BeanPropertyAssoc.java | 4 +-- .../server/deploy/BeanPropertyAssocMany.java | 12 ++++---- .../deploy/BeanPropertyAssocManySqlHelp.java | 16 +++-------- .../server/deploy/BeanPropertyAssocOne.java | 12 ++++++-- .../server/persist/DefaultPersister.java | 25 +++++++++++------ .../TestOrphanCollectionReplacement.java | 28 +++++++++++-------- 6 files changed, 56 insertions(+), 41 deletions(-) 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 (?,?,?");