From 2f97ff2211db6884d04714ca8606af5229851449 Mon Sep 17 00:00:00 2001 From: JaeYeon Kim Date: Fri, 11 Jul 2025 13:05:43 +0900 Subject: [PATCH 1/2] JdbcAggregateOperations delete by query Issue link: #1978 Add deleteAllByQuery method to JdbcAggregateOperations This method enables deleting aggregates based on a query by performing the following steps: 1. Select root IDs matching the query with a SELECT ... FOR UPDATE to lock the rows. 2. Delete all sub-entities associated with the selected root IDs. 3. Delete the root entities identified by the selected IDs. Signed-off-by: JaeYeon Kim --- .../jdbc/core/AggregateChangeExecutor.java | 7 ++ .../JdbcAggregateChangeExecutionContext.java | 38 ++++++ .../jdbc/core/JdbcAggregateOperations.java | 10 ++ .../data/jdbc/core/JdbcAggregateTemplate.java | 19 +++ .../convert/CascadingDataAccessStrategy.java | 6 + .../jdbc/core/convert/DataAccessStrategy.java | 13 +++ .../convert/DefaultDataAccessStrategy.java | 24 ++++ .../convert/DelegatingDataAccessStrategy.java | 6 + .../data/jdbc/core/convert/SqlGenerator.java | 30 +++++ .../mybatis/MyBatisDataAccessStrategy.java | 6 + ...AggregateTemplateHsqlIntegrationTests.java | 22 ++++ .../core/convert/SqlGeneratorUnitTests.java | 13 +++ .../relational/core/conversion/DbAction.java | 109 ++++++++++++++++++ .../RelationalEntityDeleteWriter.java | 32 +++++ .../SelectIdsDbActionExecutionResult.java | 44 +++++++ ...RelationalEntityDeleteWriterUnitTests.java | 22 ++++ 16 files changed, 401 insertions(+) create mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SelectIdsDbActionExecutionResult.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java index 45b139b7ab..7f8f81b650 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java @@ -30,6 +30,7 @@ * @author Myeonghyeon Lee * @author Chirag Tailor * @author Mikhail Polivakha + * @author Jaeyeon Kim * @since 2.0 */ class AggregateChangeExecutor { @@ -101,10 +102,16 @@ private void execute(DbAction action, JdbcAggregateChangeExecutionContext exe executionContext.executeBatchDeleteRoot(batchDeleteRoot); } else if (action instanceof DbAction.DeleteAllRoot deleteAllRoot) { executionContext.executeDeleteAllRoot(deleteAllRoot); + } else if (action instanceof DbAction.DeleteRootByIdIn deleteRootByIdIn) { + executionContext.executeDeleteRootByIdIn(deleteRootByIdIn); + } else if (action instanceof DbAction.DeleteByRootIdIn deleteByRootIdIn) { + executionContext.executeDeleteByRootIdIn(deleteByRootIdIn); } else if (action instanceof DbAction.AcquireLockRoot acquireLockRoot) { executionContext.executeAcquireLock(acquireLockRoot); } else if (action instanceof DbAction.AcquireLockAllRoot acquireLockAllRoot) { executionContext.executeAcquireLockAllRoot(acquireLockAllRoot); + } else if (action instanceof DbAction.AcquireLockAllRootByQuery acquireLockAllRootByQuery) { + executionContext.executeAcquireLockRootByQuery(acquireLockAllRootByQuery); } else { throw new RuntimeException("unexpected action"); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java index 75579d83a4..4b1b9ea9d3 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java @@ -40,6 +40,7 @@ import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PersistentPropertyPathAccessor; import org.springframework.data.relational.core.conversion.DbAction; +import org.springframework.data.relational.core.conversion.SelectIdsDbActionExecutionResult; import org.springframework.data.relational.core.conversion.DbActionExecutionResult; import org.springframework.data.relational.core.conversion.IdValueSource; import org.springframework.data.relational.core.mapping.AggregatePath; @@ -60,6 +61,7 @@ * @author Myeonghyeon Lee * @author Chirag Tailor * @author Mark Paluch + * @author Jaeyeon Kim */ @SuppressWarnings("rawtypes") class JdbcAggregateChangeExecutionContext { @@ -72,6 +74,7 @@ class JdbcAggregateChangeExecutionContext { private final DataAccessStrategy accessStrategy; private final Map, DbActionExecutionResult> results = new LinkedHashMap<>(); + private final Map, SelectIdsDbActionExecutionResult> selectIdsDbActionExecutionResult = new LinkedHashMap<>(); JdbcAggregateChangeExecutionContext(JdbcConverter converter, DataAccessStrategy accessStrategy) { @@ -169,6 +172,34 @@ void executeDeleteAll(DbAction.DeleteAll delete) { accessStrategy.deleteAll(delete.getPropertyPath()); } + void executeDeleteRootByIdIn(DbAction.DeleteRootByIdIn deleteRootByIdIn) { + SelectIdsDbActionExecutionResult result = getRequiredSelectIdsResult(deleteRootByIdIn.getSelectIdsAction()); + + List rootIds = new ArrayList<>(result.getSelectedIds()); + if (rootIds.isEmpty()) { + return; + } + accessStrategy.delete(rootIds, deleteRootByIdIn.getEntityType()); + } + + void executeDeleteByRootIdIn(DbAction.DeleteByRootIdIn deleteByRootIdIn) { + SelectIdsDbActionExecutionResult result = getRequiredSelectIdsResult(deleteByRootIdIn.getSelectIdsAction()); + + List rootIds = new ArrayList<>(result.getSelectedIds()); + if (rootIds.isEmpty()) { + return; + } + accessStrategy.delete(rootIds, deleteByRootIdIn.getPropertyPath()); + } + + private SelectIdsDbActionExecutionResult getRequiredSelectIdsResult(DbAction.SelectIds selectIdsAction) { + SelectIdsDbActionExecutionResult result = selectIdsDbActionExecutionResult.get(selectIdsAction); + if (result == null) { + throw new IllegalArgumentException("Expected SelectIdsDbActionExecutionResult for given selectIdsAction but found none"); + } + return result; + } + void executeAcquireLock(DbAction.AcquireLockRoot acquireLock) { accessStrategy.acquireLockById(acquireLock.getId(), LockMode.PESSIMISTIC_WRITE, acquireLock.getEntityType()); } @@ -177,6 +208,13 @@ void executeAcquireLockAllRoot(DbAction.AcquireLockAllRoot acquireLock) { accessStrategy.acquireLockAll(LockMode.PESSIMISTIC_WRITE, acquireLock.getEntityType()); } + void executeAcquireLockRootByQuery(DbAction.AcquireLockAllRootByQuery acquireLock) { + + List rootIds = accessStrategy.acquireLockAndFindIdsByQuery(acquireLock.getQuery(), LockMode.PESSIMISTIC_WRITE, acquireLock.getEntityType()); + + selectIdsDbActionExecutionResult.put(acquireLock, new SelectIdsDbActionExecutionResult(rootIds, acquireLock)); + } + private void add(DbActionExecutionResult result) { results.put(result.getAction(), result); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java index ef6844ad23..653ad8966a 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateOperations.java @@ -37,6 +37,7 @@ * @author Diego Krupitza * @author Myeonghyeon Lee * @author Sergey Korotaev + * @author Jaeyeon Kim */ public interface JdbcAggregateOperations { @@ -324,4 +325,13 @@ public interface JdbcAggregateOperations { * @param the type of the aggregate roots. */ void deleteAll(Iterable aggregateRoots); + + /** + * Deletes all aggregates of the given type that match the provided query. + * + * @param query Must not be {@code null}. + * @param domainType the type of the aggregate root. Must not be {@code null}. + * @param the type of the aggregate root. + */ + void deleteAllByQuery(Query query, Class domainType); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java index 2db2a3f18e..ab91fd9ffe 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateTemplate.java @@ -70,6 +70,7 @@ * @author Diego Krupitza * @author Sergey Korotaev * @author Mikhail Polivakha + * @author Jaeyeon Kim */ public class JdbcAggregateTemplate implements JdbcAggregateOperations { @@ -461,6 +462,17 @@ public void deleteAll(Iterable instances) { } } + @Override + public void deleteAllByQuery(Query query, Class domainType) { + + Assert.notNull(query, "Query must not be null"); + Assert.notNull(domainType, "Domain type must not be null"); + + MutableAggregateChange change = createDeletingChange(query, domainType); + + executor.executeDelete(change); + } + private void verifyIdProperty(T instance) { // accessing the id property just to raise an exception in the case it does not exist. context.getRequiredPersistentEntity(instance.getClass()).getRequiredIdProperty(); @@ -639,6 +651,13 @@ private MutableAggregateChange createDeletingChange(Class domainType) { return aggregateChange; } + private MutableAggregateChange createDeletingChange(Query query, Class domainType) { + + MutableAggregateChange aggregateChange = MutableAggregateChange.forDelete(domainType); + jdbcEntityDeleteWriter.writeForQuery(query, aggregateChange); + return aggregateChange; + } + private List triggerAfterConvert(Iterable all) { List result = new ArrayList<>(); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java index d3c3124a20..f9cfb74849 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java @@ -44,6 +44,7 @@ * @author Chirag Tailor * @author Diego Krupitza * @author Sergey Korotaev + * @author Jaeyeon Kim * @since 1.1 */ public class CascadingDataAccessStrategy implements DataAccessStrategy { @@ -119,6 +120,11 @@ public void acquireLockAll(LockMode lockMode, Class domainType) { collectVoid(das -> das.acquireLockAll(lockMode, domainType)); } + @Override + public List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType) { + return collect(das -> das.acquireLockAndFindIdsByQuery(query, lockMode, domainType)); + } + @Override public long count(Class domainType) { return collect(das -> das.count(domainType)); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java index 560e3bdef0..98734e19c7 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java @@ -43,6 +43,7 @@ * @author Chirag Tailor * @author Diego Krupitza * @author Sergey Korotaev + * @author Jaeyeon Kim */ public interface DataAccessStrategy extends ReadingDataAccessStrategy, RelationResolver { @@ -194,6 +195,18 @@ public interface DataAccessStrategy extends ReadingDataAccessStrategy, RelationR */ void acquireLockAll(LockMode lockMode, Class domainType); + /** + * Acquire a lock on all aggregates that match the given {@link Query} and return their identifiers. + * The resulting SQL will include a {@code SELECT id FROM … WHERE … (LOCK CLAUSE)} to retrieve and lock the matching rows. + * + * @param query the query specifying which entities to lock. Must not be {@code null}. + * @param lockMode the lock mode to apply to the query (e.g. {@code FOR UPDATE}). Must not be {@code null}. + * @param domainType the domain type of the entities to be locked. Must not be {@code null}. + * @param the type of the domain entity. + * @return a {@link List} of ids corresponding to the rows locked by the query. + */ + List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType); + /** * Counts the rows in the table representing the given domain type. * diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index f0febcad79..28620d7640 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -39,6 +39,7 @@ import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.SingleColumnRowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.jdbc.core.namedparam.SqlParameterSource; @@ -63,6 +64,7 @@ * @author Diego Krupitza * @author Sergey Korotaev * @author Mikhail Polivakha + * @author Jaeyeon Kim * @since 1.1 */ public class DefaultDataAccessStrategy implements DataAccessStrategy { @@ -259,6 +261,28 @@ public void acquireLockAll(LockMode lockMode, Class domainType) { operations.getJdbcOperations().query(acquireLockAllSql, ResultSet::next); } + @Override + public List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType) { + + MapSqlParameterSource parameterSource = new MapSqlParameterSource(); + String acquireLockByQuerySql = sql(domainType).getAcquireLockAndFindIdsByQuery(query, parameterSource, lockMode); + + RelationalPersistentEntity entity = context.getRequiredPersistentEntity(domainType); + RelationalPersistentProperty idProperty = entity.getRequiredIdProperty(); + + return operations.query(acquireLockByQuerySql, parameterSource, getIdRowMapper(idProperty)); + } + + private RowMapper getIdRowMapper(RelationalPersistentProperty idProperty) { + RelationalPersistentEntity complexId = context.getPersistentEntity(idProperty.getType()); + + if (complexId == null) { + return SingleColumnRowMapper.newInstance(idProperty.getType(), converter.getConversionService()); + } else { + return new EntityRowMapper<>(context.getRequiredPersistentEntity(idProperty.getType()), converter); + } + } + @Override public long count(Class domainType) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java index 1bec8222f0..97a7244bab 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java @@ -39,6 +39,7 @@ * @author Chirag Tailor * @author Diego Krupitza * @author Sergey Korotaev + * @author Jaeyeon Kim * @since 1.1 */ public class DelegatingDataAccessStrategy implements DataAccessStrategy { @@ -119,6 +120,11 @@ public void acquireLockAll(LockMode lockMode, Class domainType) { delegate.acquireLockAll(lockMode, domainType); } + @Override + public List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType) { + return delegate.acquireLockAndFindIdsByQuery(query, lockMode, domainType); + } + @Override public long count(Class domainType) { return delegate.count(domainType); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index 82f2db5158..71b6c585e0 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -70,6 +70,7 @@ * @author Hari Ohm Prasath * @author Viktor Ardelean * @author Kurt Niemi + * @author Jaeyeon Kim */ public class SqlGenerator { @@ -377,6 +378,18 @@ String getAcquireLockAll(LockMode lockMode) { return this.createAcquireLockAll(lockMode); } + /** + * Create a {@code SELECT id FROM … WHERE … (LOCK CLAUSE)} statement based on the given query. + * + * @param query the query to base the select on. Must not be null. + * @param parameterSource the source for holding the bindings. + * @param lockMode Lock clause mode. + * @return the SQL statement as a {@link String}. Guaranteed to be not {@literal null}. + */ + String getAcquireLockAndFindIdsByQuery(Query query, MapSqlParameterSource parameterSource, LockMode lockMode) { + return this.createAcquireLockByQuery(query, parameterSource, lockMode); + } + /** * Create a {@code INSERT INTO … (…) VALUES(…)} statement. * @@ -594,6 +607,23 @@ private String createAcquireLockAll(LockMode lockMode) { return render(select); } + private String createAcquireLockByQuery(Query query, MapSqlParameterSource parameterSource, LockMode lockMode) { + + Assert.notNull(parameterSource, "parameterSource must not be null"); + + Table table = this.getTable(); + + SelectBuilder.SelectWhere selectBuilder = StatementBuilder + .select(getIdColumns()) + .from(table); + + Select select = applyQueryOnSelect(query, parameterSource, selectBuilder) + .lock(lockMode) + .build(); + + return render(select); + } + private String createFindAllSql() { return render(selectBuilder().build()); } diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index 0f619dcc59..d32bf9ab2d 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -63,6 +63,7 @@ * @author Christopher Klein * @author Mikhail Polivakha * @author Sergey Korotaev + * @author Jaeyeon Kim */ public class MyBatisDataAccessStrategy implements DataAccessStrategy { @@ -253,6 +254,11 @@ public void acquireLockAll(LockMode lockMode, Class domainType) { sqlSession().selectOne(statement, parameter); } + @Override + public List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType) { + throw new UnsupportedOperationException("Not implemented"); + } + @Override public T findById(Object id, Class domainType) { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/CompositeIdAggregateTemplateHsqlIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/CompositeIdAggregateTemplateHsqlIntegrationTests.java index 9b5df44a80..d23565f710 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/CompositeIdAggregateTemplateHsqlIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/CompositeIdAggregateTemplateHsqlIntegrationTests.java @@ -35,6 +35,7 @@ import org.springframework.data.jdbc.testing.TestConfiguration; import org.springframework.data.relational.core.mapping.Embedded; import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.relational.core.query.Criteria; import org.springframework.data.relational.core.query.Query; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; @@ -42,6 +43,7 @@ * Integration tests for {@link JdbcAggregateTemplate} and it's handling of entities with embedded entities as keys. * * @author Jens Schauder + * @author Jaeyeon Kim */ @IntegrationTest @EnabledOnDatabase(DatabaseType.HSQL) @@ -132,6 +134,26 @@ void deleteMultipleSimpleEntityWithEmbeddedPk() { assertThat(reloaded).containsExactly(entities.get(2)); } + @Test // GH-1978 + void deleteAllByQueryWithEmbeddedPk() { + + List entities = (List) template.insertAll(List.of( + new SimpleEntityWithEmbeddedPk(new EmbeddedPk(1L, "a"), "alpha"), + new SimpleEntityWithEmbeddedPk(new EmbeddedPk(2L, "b"), "beta"), + new SimpleEntityWithEmbeddedPk(new EmbeddedPk(3L, "b"), "gamma") + )); + + Query query = Query.query(Criteria.where("name").is("beta")); + template.deleteAllByQuery(query, SimpleEntityWithEmbeddedPk.class); + + assertThat( + template.findAll(SimpleEntityWithEmbeddedPk.class)) + .containsExactlyInAnyOrder( + entities.get(0), // alpha + entities.get(2) // gamma + ); + } + @Test // GH-574 void existsSingleSimpleEntityWithEmbeddedPk() { diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index 08831d801a..914ec1e547 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -74,6 +74,7 @@ * @author Diego Krupitza * @author Hari Ohm Prasath * @author Viktor Ardelean + * @author Jaeyeon Kim */ @SuppressWarnings("Convert2MethodRef") class SqlGeneratorUnitTests { @@ -166,6 +167,18 @@ void getAcquireLockAll() { .doesNotContain("Element AS elements")); } + @Test // GH-1978 + void getAcquireLockAndFindIdsByQuery(){ + + Query query = Query.query(Criteria.where("id").is(23L)) + .sort(Sort.by(Sort.Order.asc("id"))) + .limit(5); + + String sql = sqlGenerator.getAcquireLockAndFindIdsByQuery(query, new MapSqlParameterSource(), LockMode.PESSIMISTIC_WRITE); + + assertThat(sql).isEqualTo("SELECT dummy_entity.id1 AS id1 FROM dummy_entity WHERE dummy_entity.id1 = :id1 ORDER BY dummy_entity.id1 ASC LIMIT 5 FOR UPDATE"); + } + @Test // DATAJDBC-112 void cascadingDeleteFirstLevel() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java index ec5ab5106a..d7c8cddc01 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java @@ -25,6 +25,7 @@ import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; +import org.springframework.data.relational.core.query.Query; import org.springframework.data.util.Pair; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -39,6 +40,7 @@ * @author Tyler Van Gorder * @author Myeonghyeon Lee * @author Chirag Tailor + * @author Jaeyeon Kim */ public interface DbAction { @@ -297,6 +299,63 @@ public String toString() { } } + /** + * Represents a delete statement for all entities reachable via a given property path from a list of aggregate root IDs. + */ + final class DeleteByRootIdIn implements WithPropertyPath, WithSelectIds { + + private final SelectIds selectIds; + + private final PersistentPropertyPath propertyPath; + + public DeleteByRootIdIn(SelectIds selectIds, PersistentPropertyPath propertyPath) { + this.selectIds = selectIds; + this.propertyPath = propertyPath; + } + + public PersistentPropertyPath getPropertyPath() { + return this.propertyPath; + } + + @Override + public SelectIds getSelectIdsAction() { + return selectIds; + } + + public String toString() { + return "DeleteByRootIdIn{propertyPath=" + this.propertyPath + ", selectIds=" + this.selectIds + "}"; + } + } + + /** + * Represents a delete statement for multiple aggregate roots identified by their IDs. + */ + final class DeleteRootByIdIn implements WithSelectIds { + + private final Class entityType; + + private final SelectIds selectIds; + + public DeleteRootByIdIn(Class entityType, SelectIds selectIds) { + this.entityType = entityType; + this.selectIds = selectIds; + } + + @Override + public Class getEntityType() { + return this.entityType; + } + + @Override + public SelectIds getSelectIdsAction() { + return selectIds; + } + + public String toString() { + return "DeleteRootByIdIn{entityType=" + this.entityType + ", selectIds=" + this.selectIds + "}"; + } + } + /** * Represents an acquire lock statement for a aggregate root when only the ID is known. * @@ -347,6 +406,38 @@ public String toString() { } } + /** + * Represents an acquire lock statement on all aggregate roots of a given type, constrained by a query. + * This is used to select and lock aggregate root IDs matching a given {@link Query}, which can be reused + * in follow-up actions like batch deletion. + * + * @param type of the root entity for which this represents a database interaction. + */ + final class AcquireLockAllRootByQuery implements SelectIds { + + private final Class entityType; + + private final Query query; + + AcquireLockAllRootByQuery(Class entityType, Query query) { + this.entityType = entityType; + this.query = query; + } + + @Override + public Class getEntityType() { + return this.entityType; + } + + public Query getQuery() { + return query; + } + + public String toString() { + return "DbAction.AcquireLockAllRootByQuery(entityType=" + this.entityType + ", query=" + this.query + ")"; + } + } + /** * Represents a batch of {@link DbAction} that share a common value for a property of the action. * @@ -561,4 +652,22 @@ default Class getEntityType() { return (Class) getPropertyPath().getLeafProperty().getActualType(); } } + + /** + * A {@link DbAction} that depends on a {@link SelectIds} action + * + * @author Jaeyeon Kim + */ + interface WithSelectIds extends DbAction { + + DbAction.SelectIds getSelectIdsAction(); + } + + /** + * A {@link DbAction} that represents a query to select a list of root IDs. + * + * @author Jaeyeon Kim + */ + interface SelectIds extends DbAction { + } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java index cc706f7cb5..01ce4c4d0a 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java @@ -25,6 +25,7 @@ import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; import org.springframework.data.relational.core.mapping.RelationalPredicates; +import org.springframework.data.relational.core.query.Query; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -40,6 +41,7 @@ * @author Tyler Van Gorder * @author Myeonghyeon Lee * @author Chirag Tailor + * @author Jaeyeon Kim */ public class RelationalEntityDeleteWriter implements EntityWriter> { @@ -70,6 +72,36 @@ public void write(@Nullable Object id, MutableAggregateChange aggregateChange } } + /** + * Fills the provided {@link MutableAggregateChange} with the necessary {@link DbAction}s + * to delete all aggregate roots matching the given {@link Query}. + * This includes acquiring locks, deleting referenced entities, and deleting the root entities themselves. + * + * @param query the query used to select aggregate root IDs to delete. Must not be {@code null}. + * @param aggregateChange The change object to which delete actions will be added. Must not be {@code null}. + */ + public void writeForQuery(Query query, MutableAggregateChange aggregateChange) { + + Class entityType = aggregateChange.getEntityType(); + + DbAction.SelectIds selectIds = new DbAction.AcquireLockAllRootByQuery<>(entityType, query); + + List> actions = new ArrayList<>(); + actions.add(selectIds); + + List> paths = new ArrayList<>(); + forAllTableRepresentingPaths(entityType, paths::add); + Collections.reverse(paths); + + for (PersistentPropertyPath path : paths) { + actions.add(new DbAction.DeleteByRootIdIn<>(selectIds, path)); + } + + actions.add(new DbAction.DeleteRootByIdIn<>(entityType, selectIds)); + + actions.forEach(aggregateChange::addAction); + } + private List> deleteAll(Class entityType) { List> deleteReferencedActions = new ArrayList<>(); diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SelectIdsDbActionExecutionResult.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SelectIdsDbActionExecutionResult.java new file mode 100644 index 0000000000..5824be6ba2 --- /dev/null +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SelectIdsDbActionExecutionResult.java @@ -0,0 +1,44 @@ +/* + * Copyright 2017-2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.relational.core.conversion; + +import java.util.List; + +/** + * Represents the result of executing a {@link DbAction.SelectIds} operation. + * Typically used when selecting IDs (e.g., with a lock) as a pre-step to a delete operation. + * + * @author Jaeyeon Kim + * @since 4.0 + */ +public class SelectIdsDbActionExecutionResult { + + private final List selectedIds; + private final DbAction.SelectIds action; + + public SelectIdsDbActionExecutionResult(List selectedIds, DbAction.SelectIds action) { + this.selectedIds = selectedIds; + this.action = action; + } + + public List getSelectedIds() { + return this.selectedIds; + } + + public DbAction.SelectIds getAction() { + return action; + } +} diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java index 11e0238b95..6c4e78d67f 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java @@ -21,6 +21,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.data.annotation.Id; import org.springframework.data.annotation.ReadOnlyProperty; +import org.springframework.data.domain.Sort; import org.springframework.data.relational.core.conversion.DbAction.AcquireLockAllRoot; import org.springframework.data.relational.core.conversion.DbAction.AcquireLockRoot; import org.springframework.data.relational.core.conversion.DbAction.Delete; @@ -28,6 +29,8 @@ import org.springframework.data.relational.core.conversion.DbAction.DeleteAllRoot; import org.springframework.data.relational.core.conversion.DbAction.DeleteRoot; import org.springframework.data.relational.core.mapping.RelationalMappingContext; +import org.springframework.data.relational.core.query.Criteria; +import org.springframework.data.relational.core.query.Query; import java.util.ArrayList; import java.util.List; @@ -40,6 +43,7 @@ * @author Jens Schauder * @author Myeonghyeon Lee * @author Chirag Tailor + * @author Jaeyeon Kim */ @ExtendWith(MockitoExtension.class) public class RelationalEntityDeleteWriterUnitTests { @@ -142,6 +146,24 @@ public void deleteAllDoesNotDeleteReadOnlyReferences() { ); } + @Test // GH-1978 + void writeForQueryDeletesEntitiesByQueryAndReferencedEntities() { + + MutableAggregateChange aggregateChange = MutableAggregateChange.forDelete(SomeEntity.class); + Query query = Query.query(Criteria.empty()).limit(10).sort(Sort.by("name")); + + converter.writeForQuery(query, aggregateChange); + + assertThat(extractActions(aggregateChange)) + .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) + .containsExactly( + Tuple.tuple(DbAction.AcquireLockAllRootByQuery.class, SomeEntity.class, ""), + Tuple.tuple(DbAction.DeleteByRootIdIn.class, YetAnother.class, "other.yetAnother"), + Tuple.tuple(DbAction.DeleteByRootIdIn.class, OtherEntity.class, "other"), + Tuple.tuple(DbAction.DeleteRootByIdIn.class, SomeEntity.class, "") + ); + } + private List> extractActions(MutableAggregateChange aggregateChange) { List> actions = new ArrayList<>(); From 5420ba2ae3884247aeb61aabdbc20dbb288f54a3 Mon Sep 17 00:00:00 2001 From: JaeYeon Kim Date: Thu, 17 Jul 2025 09:58:14 +0900 Subject: [PATCH 2/2] GH-1978 JdbcAggregateOperations delete by query Changed the steps as follows: 1. Lock the target rows using SELECT ... FOR UPDATE based on the query conditions. 2. Delete sub-entities by leveraging a subquery that selects the matching root rows. 3. Delete the root entities using the same subquery criteria. Signed-off-by: JaeYeon Kim --- .../jdbc/core/AggregateChangeExecutor.java | 8 +- .../JdbcAggregateChangeExecutionContext.java | 33 ++----- .../convert/CascadingDataAccessStrategy.java | 14 ++- .../jdbc/core/convert/DataAccessStrategy.java | 22 ++++- .../convert/DefaultDataAccessStrategy.java | 44 ++++++---- .../convert/DelegatingDataAccessStrategy.java | 14 ++- .../data/jdbc/core/convert/SqlGenerator.java | 84 +++++++++++++++++- .../mybatis/MyBatisDataAccessStrategy.java | 12 ++- .../core/convert/SqlGeneratorUnitTests.java | 51 +++++++++-- .../relational/core/conversion/DbAction.java | 87 ++++++++----------- .../RelationalEntityDeleteWriter.java | 18 ++-- .../SelectIdsDbActionExecutionResult.java | 44 ---------- ...RelationalEntityDeleteWriterUnitTests.java | 8 +- 13 files changed, 264 insertions(+), 175 deletions(-) delete mode 100644 spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SelectIdsDbActionExecutionResult.java diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java index 7f8f81b650..c547b6bfba 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/AggregateChangeExecutor.java @@ -102,10 +102,10 @@ private void execute(DbAction action, JdbcAggregateChangeExecutionContext exe executionContext.executeBatchDeleteRoot(batchDeleteRoot); } else if (action instanceof DbAction.DeleteAllRoot deleteAllRoot) { executionContext.executeDeleteAllRoot(deleteAllRoot); - } else if (action instanceof DbAction.DeleteRootByIdIn deleteRootByIdIn) { - executionContext.executeDeleteRootByIdIn(deleteRootByIdIn); - } else if (action instanceof DbAction.DeleteByRootIdIn deleteByRootIdIn) { - executionContext.executeDeleteByRootIdIn(deleteByRootIdIn); + } else if (action instanceof DbAction.DeleteRootByQuery deleteRootByQuery) { + executionContext.excuteDeleteRootByQuery(deleteRootByQuery); + } else if (action instanceof DbAction.DeleteByQuery deleteByQuery) { + executionContext.excuteDeleteByQuery(deleteByQuery); } else if (action instanceof DbAction.AcquireLockRoot acquireLockRoot) { executionContext.executeAcquireLock(acquireLockRoot); } else if (action instanceof DbAction.AcquireLockAllRoot acquireLockAllRoot) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java index 4b1b9ea9d3..53f1b0f835 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/JdbcAggregateChangeExecutionContext.java @@ -40,7 +40,6 @@ import org.springframework.data.mapping.PersistentPropertyPath; import org.springframework.data.mapping.PersistentPropertyPathAccessor; import org.springframework.data.relational.core.conversion.DbAction; -import org.springframework.data.relational.core.conversion.SelectIdsDbActionExecutionResult; import org.springframework.data.relational.core.conversion.DbActionExecutionResult; import org.springframework.data.relational.core.conversion.IdValueSource; import org.springframework.data.relational.core.mapping.AggregatePath; @@ -74,7 +73,6 @@ class JdbcAggregateChangeExecutionContext { private final DataAccessStrategy accessStrategy; private final Map, DbActionExecutionResult> results = new LinkedHashMap<>(); - private final Map, SelectIdsDbActionExecutionResult> selectIdsDbActionExecutionResult = new LinkedHashMap<>(); JdbcAggregateChangeExecutionContext(JdbcConverter converter, DataAccessStrategy accessStrategy) { @@ -172,32 +170,14 @@ void executeDeleteAll(DbAction.DeleteAll delete) { accessStrategy.deleteAll(delete.getPropertyPath()); } - void executeDeleteRootByIdIn(DbAction.DeleteRootByIdIn deleteRootByIdIn) { - SelectIdsDbActionExecutionResult result = getRequiredSelectIdsResult(deleteRootByIdIn.getSelectIdsAction()); + void excuteDeleteRootByQuery(DbAction.DeleteRootByQuery deleteRootByQuery) { - List rootIds = new ArrayList<>(result.getSelectedIds()); - if (rootIds.isEmpty()) { - return; - } - accessStrategy.delete(rootIds, deleteRootByIdIn.getEntityType()); + accessStrategy.deleteRootByQuery(deleteRootByQuery.getQuery(), deleteRootByQuery.getEntityType()); } - void executeDeleteByRootIdIn(DbAction.DeleteByRootIdIn deleteByRootIdIn) { - SelectIdsDbActionExecutionResult result = getRequiredSelectIdsResult(deleteByRootIdIn.getSelectIdsAction()); + void excuteDeleteByQuery(DbAction.DeleteByQuery deleteByQuery) { - List rootIds = new ArrayList<>(result.getSelectedIds()); - if (rootIds.isEmpty()) { - return; - } - accessStrategy.delete(rootIds, deleteByRootIdIn.getPropertyPath()); - } - - private SelectIdsDbActionExecutionResult getRequiredSelectIdsResult(DbAction.SelectIds selectIdsAction) { - SelectIdsDbActionExecutionResult result = selectIdsDbActionExecutionResult.get(selectIdsAction); - if (result == null) { - throw new IllegalArgumentException("Expected SelectIdsDbActionExecutionResult for given selectIdsAction but found none"); - } - return result; + accessStrategy.deleteByQuery(deleteByQuery.getQuery(), deleteByQuery.getPropertyPath()); } void executeAcquireLock(DbAction.AcquireLockRoot acquireLock) { @@ -209,10 +189,7 @@ void executeAcquireLockAllRoot(DbAction.AcquireLockAllRoot acquireLock) { } void executeAcquireLockRootByQuery(DbAction.AcquireLockAllRootByQuery acquireLock) { - - List rootIds = accessStrategy.acquireLockAndFindIdsByQuery(acquireLock.getQuery(), LockMode.PESSIMISTIC_WRITE, acquireLock.getEntityType()); - - selectIdsDbActionExecutionResult.put(acquireLock, new SelectIdsDbActionExecutionResult(rootIds, acquireLock)); + accessStrategy.acquireLockByQuery(acquireLock.getQuery(), LockMode.PESSIMISTIC_WRITE, acquireLock.getEntityType()); } private void add(DbActionExecutionResult result) { diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java index f9cfb74849..d2d1ced8b9 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/CascadingDataAccessStrategy.java @@ -110,6 +110,16 @@ public void deleteAll(PersistentPropertyPath prope collectVoid(das -> das.deleteAll(propertyPath)); } + @Override + public void deleteRootByQuery(Query query, Class domainType) { + collectVoid(das -> das.deleteRootByQuery(query, domainType)); + } + + @Override + public void deleteByQuery(Query query, PersistentPropertyPath propertyPath) { + collectVoid(das -> das.deleteByQuery(query, propertyPath)); + } + @Override public void acquireLockById(Object id, LockMode lockMode, Class domainType) { collectVoid(das -> das.acquireLockById(id, lockMode, domainType)); @@ -121,8 +131,8 @@ public void acquireLockAll(LockMode lockMode, Class domainType) { } @Override - public List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType) { - return collect(das -> das.acquireLockAndFindIdsByQuery(query, lockMode, domainType)); + public void acquireLockByQuery(Query query, LockMode lockMode, Class domainType) { + collectVoid(das -> das.acquireLockByQuery(query, lockMode, domainType)); } @Override diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java index 98734e19c7..d4a2dec35a 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DataAccessStrategy.java @@ -178,6 +178,22 @@ public interface DataAccessStrategy extends ReadingDataAccessStrategy, RelationR */ void deleteAll(PersistentPropertyPath propertyPath); + /** + * Deletes all root entities of the given domain type that match the given {@link Query}. + * + * @param query the query specifying which rows to delete. Must not be {@code null}. + * @param domainType the domain type of the entity. Must not be {@code null}. + */ + void deleteRootByQuery(Query query, Class domainType); + + /** + * Deletes entities reachable via the given {@link PersistentPropertyPath} from root entities that match the given {@link Query}. + * + * @param query the query specifying which root entities to consider for deleting related entities. Must not be {@code null}. + * @param propertyPath Leading from the root object to the entities to be deleted. Must not be {@code null}. + */ + void deleteByQuery(Query query, PersistentPropertyPath propertyPath); + /** * Acquire a lock on the aggregate specified by id. * @@ -196,16 +212,14 @@ public interface DataAccessStrategy extends ReadingDataAccessStrategy, RelationR void acquireLockAll(LockMode lockMode, Class domainType); /** - * Acquire a lock on all aggregates that match the given {@link Query} and return their identifiers. - * The resulting SQL will include a {@code SELECT id FROM … WHERE … (LOCK CLAUSE)} to retrieve and lock the matching rows. + * Acquire a lock on all aggregates that match the given {@link Query}. * * @param query the query specifying which entities to lock. Must not be {@code null}. * @param lockMode the lock mode to apply to the query (e.g. {@code FOR UPDATE}). Must not be {@code null}. * @param domainType the domain type of the entities to be locked. Must not be {@code null}. * @param the type of the domain entity. - * @return a {@link List} of ids corresponding to the rows locked by the query. */ - List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType); + void acquireLockByQuery(Query query, LockMode lockMode, Class domainType); /** * Counts the rows in the table representing the given domain type. diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java index 28620d7640..862ef75f74 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DefaultDataAccessStrategy.java @@ -39,7 +39,6 @@ import org.springframework.data.relational.core.sql.LockMode; import org.springframework.data.relational.core.sql.SqlIdentifier; import org.springframework.jdbc.core.RowMapper; -import org.springframework.jdbc.core.SingleColumnRowMapper; import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; import org.springframework.jdbc.core.namedparam.NamedParameterJdbcOperations; import org.springframework.jdbc.core.namedparam.SqlParameterSource; @@ -245,6 +244,30 @@ public void deleteAll(PersistentPropertyPath prope operations.getJdbcOperations().update(sql(getBaseType(propertyPath)).createDeleteAllSql(propertyPath)); } + @Override + public void deleteRootByQuery(Query query, Class domainType) { + + MapSqlParameterSource parameterSource = new MapSqlParameterSource(); + String deleteSql = sql(domainType).createDeleteByQuery(query, parameterSource); + + operations.update(deleteSql, parameterSource); + } + + @Override + public void deleteByQuery(Query query, PersistentPropertyPath propertyPath) { + + RelationalPersistentEntity rootEntity = context.getRequiredPersistentEntity(getBaseType(propertyPath)); + + RelationalPersistentProperty referencingProperty = propertyPath.getLeafProperty(); + + Assert.notNull(referencingProperty, "No property found matching the PropertyPath " + propertyPath); + + MapSqlParameterSource parameterSource = new MapSqlParameterSource(); + String deleteSql = sql(rootEntity.getType()).createDeleteInSubselectByPath(query, parameterSource, propertyPath); + + operations.update(deleteSql, parameterSource); + } + @Override public void acquireLockById(Object id, LockMode lockMode, Class domainType) { @@ -262,25 +285,12 @@ public void acquireLockAll(LockMode lockMode, Class domainType) { } @Override - public List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType) { + public void acquireLockByQuery(Query query, LockMode lockMode, Class domainType) { MapSqlParameterSource parameterSource = new MapSqlParameterSource(); - String acquireLockByQuerySql = sql(domainType).getAcquireLockAndFindIdsByQuery(query, parameterSource, lockMode); + String acquireLockByQuerySql = sql(domainType).getAcquireLockByQuery(query, parameterSource, lockMode); - RelationalPersistentEntity entity = context.getRequiredPersistentEntity(domainType); - RelationalPersistentProperty idProperty = entity.getRequiredIdProperty(); - - return operations.query(acquireLockByQuerySql, parameterSource, getIdRowMapper(idProperty)); - } - - private RowMapper getIdRowMapper(RelationalPersistentProperty idProperty) { - RelationalPersistentEntity complexId = context.getPersistentEntity(idProperty.getType()); - - if (complexId == null) { - return SingleColumnRowMapper.newInstance(idProperty.getType(), converter.getConversionService()); - } else { - return new EntityRowMapper<>(context.getRequiredPersistentEntity(idProperty.getType()), converter); - } + operations.query(acquireLockByQuerySql, parameterSource, ResultSet::next); } @Override diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java index 97a7244bab..9eaadcf08b 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/DelegatingDataAccessStrategy.java @@ -110,6 +110,16 @@ public void deleteAll(PersistentPropertyPath prope delegate.deleteAll(propertyPath); } + @Override + public void deleteRootByQuery(Query query, Class domainType) { + delegate.deleteRootByQuery(query, domainType); + } + + @Override + public void deleteByQuery(Query query, PersistentPropertyPath propertyPath) { + delegate.deleteByQuery(query, propertyPath); + } + @Override public void acquireLockById(Object id, LockMode lockMode, Class domainType) { delegate.acquireLockById(id, lockMode, domainType); @@ -121,8 +131,8 @@ public void acquireLockAll(LockMode lockMode, Class domainType) { } @Override - public List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType) { - return delegate.acquireLockAndFindIdsByQuery(query, lockMode, domainType); + public void acquireLockByQuery(Query query, LockMode lockMode, Class domainType) { + delegate.acquireLockByQuery(query, lockMode, domainType); } @Override diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index 71b6c585e0..e31dec9da4 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -386,7 +386,7 @@ String getAcquireLockAll(LockMode lockMode) { * @param lockMode Lock clause mode. * @return the SQL statement as a {@link String}. Guaranteed to be not {@literal null}. */ - String getAcquireLockAndFindIdsByQuery(Query query, MapSqlParameterSource parameterSource, LockMode lockMode) { + String getAcquireLockByQuery(Query query, MapSqlParameterSource parameterSource, LockMode lockMode) { return this.createAcquireLockByQuery(query, parameterSource, lockMode); } @@ -505,6 +505,76 @@ String createDeleteInByPath(PersistentPropertyPath return createDeleteByPathAndCriteria(mappingContext.getAggregatePath(path), this::inCondition); } + /** + * Create a {@code DELETE FROM ... WHERE ...} SQL statement based on the given {@link Query}. + * + * @param query the query object defining filter criteria; must not be {@literal null}. + * @param parameterSource the parameter bindings for the query; must not be {@literal null}. + * @return the SQL DELETE statement as a {@link String}; guaranteed to be not {@literal null}. + */ + public String createDeleteByQuery(Query query, MapSqlParameterSource parameterSource) { + Assert.notNull(parameterSource, "parameterSource must not be null"); + + Table table = this.getTable(); + + DeleteBuilder.DeleteWhere builder = Delete.builder() + .from(table); + + query.getCriteria() + .filter(criteria -> !criteria.isEmpty()) + .ifPresent(criteria -> + builder.where(queryMapper.getMappedObject(parameterSource, criteria, table, entity)) + ); + + return render(builder.build()); + } + + /** + * Creates a {@code DELETE} SQL query that targets a specific table defined by the given {@link PersistentPropertyPath}, + * and applies filtering using a subselect based on the provided {@link Query}. + * + * @param query the query object containing the filtering criteria; must not be {@literal null}. + * @param parameterSource the source for parameter bindings used in the query; must not be {@literal null}. + * @param propertyPath must not be {@literal null}. + * @return the DELETE SQL statement as a {@link String}. Guaranteed to be not {@literal null}. + */ + public String createDeleteInSubselectByPath(Query query, MapSqlParameterSource parameterSource, + PersistentPropertyPath propertyPath) { + + Assert.notNull(parameterSource, "parameterSource must not be null"); + + AggregatePath path = mappingContext.getAggregatePath(propertyPath); + + return createDeleteByPathAndCriteria(path, columnMap -> { + Select subSelect = createRootIdSubSelectByQuery(query, parameterSource); + Collection columns = columnMap.values(); + Expression expression = columns.size() == 1 ? columns.iterator().next() : TupleExpression.create(columns); + return Conditions.in(expression, subSelect); + }); + } + + /** + * Creates a subselect that retrieves root entity IDs filtered by the given query. + */ + private Select createRootIdSubSelectByQuery(Query query, MapSqlParameterSource parameterSource) { + + Table table = this.getTable(); + + SelectBuilder.SelectWhere selectBuilder = StatementBuilder + .select(getIdColumns()) + .from(table); + + query.getCriteria() + .filter(criteria -> !criteria.isEmpty()) + .ifPresent(criteria -> + selectBuilder.where( + queryMapper.getMappedObject(parameterSource, criteria, table, entity) + ) + ); + + return selectBuilder.build(); + } + /** * Constructs a where condition. The where condition will be of the form {@literal IN :bind-marker} */ @@ -614,10 +684,18 @@ private String createAcquireLockByQuery(Query query, MapSqlParameterSource param Table table = this.getTable(); SelectBuilder.SelectWhere selectBuilder = StatementBuilder - .select(getIdColumns()) + .select(getSingleNonNullColumn()) .from(table); - Select select = applyQueryOnSelect(query, parameterSource, selectBuilder) + query.getCriteria() + .filter(criteria -> !criteria.isEmpty()) + .ifPresent(criteria -> + selectBuilder.where( + queryMapper.getMappedObject(parameterSource, criteria, table, entity) + ) + ); + + Select select = selectBuilder .lock(lockMode) .build(); diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java index d32bf9ab2d..7745925aaf 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/mybatis/MyBatisDataAccessStrategy.java @@ -231,6 +231,16 @@ public void deleteAll(PersistentPropertyPath prope sqlSession().delete(statement, parameter); } + @Override + public void deleteRootByQuery(Query query, Class domainType) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public void deleteByQuery(Query query, PersistentPropertyPath propertyPath) { + throw new UnsupportedOperationException("Not implemented"); + } + @Override public void acquireLockById(Object id, LockMode lockMode, Class domainType) { @@ -255,7 +265,7 @@ public void acquireLockAll(LockMode lockMode, Class domainType) { } @Override - public List acquireLockAndFindIdsByQuery(Query query, LockMode lockMode, Class domainType) { + public void acquireLockByQuery(Query query, LockMode lockMode, Class domainType) { throw new UnsupportedOperationException("Not implemented"); } diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index 914ec1e547..4e4ebfdd0e 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -168,15 +168,13 @@ void getAcquireLockAll() { } @Test // GH-1978 - void getAcquireLockAndFindIdsByQuery(){ + void getAcquireLockByQuery(){ - Query query = Query.query(Criteria.where("id").is(23L)) - .sort(Sort.by(Sort.Order.asc("id"))) - .limit(5); + Query query = Query.query(Criteria.where("id").is(23L)); - String sql = sqlGenerator.getAcquireLockAndFindIdsByQuery(query, new MapSqlParameterSource(), LockMode.PESSIMISTIC_WRITE); + String sql = sqlGenerator.getAcquireLockByQuery(query, new MapSqlParameterSource(), LockMode.PESSIMISTIC_WRITE); - assertThat(sql).isEqualTo("SELECT dummy_entity.id1 AS id1 FROM dummy_entity WHERE dummy_entity.id1 = :id1 ORDER BY dummy_entity.id1 ASC LIMIT 5 FOR UPDATE"); + assertThat(sql).isEqualTo("SELECT dummy_entity.id1 AS id1 FROM dummy_entity WHERE dummy_entity.id1 = :id1 FOR UPDATE"); } @Test // DATAJDBC-112 @@ -254,6 +252,47 @@ void deleteMapByPath() { assertThat(sql).isEqualTo("DELETE FROM element WHERE element.dummy_entity = :id1"); } + @Test // GH-1978 + void deleteByQuery() { + + Query query = Query.query(Criteria.where("id").greaterThan(23L)); + MapSqlParameterSource parameterSource = new MapSqlParameterSource(); + + String sql = sqlGenerator.createDeleteByQuery(query, parameterSource); + + assertThat(sql).isEqualTo("DELETE FROM dummy_entity WHERE dummy_entity.id1 > :id1"); + } + + @Test // GH-1978 + void cascadingDeleteInSubselectByPathFirstLevel() { + + Query query = Query.query(Criteria.where("id").is(23L)); + MapSqlParameterSource parameterSource = new MapSqlParameterSource(); + + String sql = sqlGenerator.createDeleteInSubselectByPath(query, parameterSource, + getPath("ref", DummyEntity.class)); + + assertThat(sql).isEqualTo( + "DELETE FROM referenced_entity WHERE referenced_entity.dummy_entity IN " + + "(SELECT dummy_entity.id1 AS id1 FROM dummy_entity WHERE dummy_entity.id1 = :id1)"); + } + + @Test // GH-1978 + void cascadingDeleteInSubselectByPathSecondLevel() { + + Query query = Query.query(Criteria.where("id").is(23L)); + MapSqlParameterSource parameterSource = new MapSqlParameterSource(); + + String sql = sqlGenerator.createDeleteInSubselectByPath(query, parameterSource, + getPath("ref.further", DummyEntity.class)); + + assertThat(sql).isEqualTo( + "DELETE FROM second_level_referenced_entity " + + "WHERE second_level_referenced_entity.referenced_entity IN " + + "(SELECT referenced_entity.x_l1id FROM referenced_entity WHERE referenced_entity.dummy_entity IN " + + "(SELECT dummy_entity.id1 AS id1 FROM dummy_entity WHERE dummy_entity.id1 = :id1))"); + } + @Test // DATAJDBC-101 void findAllSortedByUnsorted() { diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java index d7c8cddc01..51cfb46073 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/DbAction.java @@ -300,59 +300,63 @@ public String toString() { } /** - * Represents a delete statement for all entities reachable via a given property path from a list of aggregate root IDs. + * Represents a delete statement for aggregate root entities matching a given {@link Query}. + * + * @param type of the entity for which this represents a database interaction. */ - final class DeleteByRootIdIn implements WithPropertyPath, WithSelectIds { - - private final SelectIds selectIds; + final class DeleteRootByQuery implements DbAction { - private final PersistentPropertyPath propertyPath; + private final Class entityType; - public DeleteByRootIdIn(SelectIds selectIds, PersistentPropertyPath propertyPath) { - this.selectIds = selectIds; - this.propertyPath = propertyPath; - } + private final Query query; - public PersistentPropertyPath getPropertyPath() { - return this.propertyPath; + DeleteRootByQuery(Class entityType, Query query) { + this.entityType = entityType; + this.query = query; } @Override - public SelectIds getSelectIdsAction() { - return selectIds; + public Class getEntityType() { + return this.entityType; + } + + public Query getQuery() { + return query; } public String toString() { - return "DeleteByRootIdIn{propertyPath=" + this.propertyPath + ", selectIds=" + this.selectIds + "}"; + return "DbAction.DeleteRootByQuery(entityType=" + this.entityType + ", query=" + this.query + ")"; } } /** - * Represents a delete statement for multiple aggregate roots identified by their IDs. + * Represents a delete statement for all entities that are reachable via a given path from the aggregate root, + * filtered by a {@link Query}. + * + * @param type of the entity for which this represents a database interaction. */ - final class DeleteRootByIdIn implements WithSelectIds { + final class DeleteByQuery implements WithPropertyPath { - private final Class entityType; - - private final SelectIds selectIds; + private final Query query; - public DeleteRootByIdIn(Class entityType, SelectIds selectIds) { - this.entityType = entityType; - this.selectIds = selectIds; - } + private final PersistentPropertyPath propertyPath; - @Override - public Class getEntityType() { - return this.entityType; + DeleteByQuery(Query query, PersistentPropertyPath propertyPath) { + this.query = query; + this.propertyPath = propertyPath; } @Override - public SelectIds getSelectIdsAction() { - return selectIds; + public PersistentPropertyPath getPropertyPath() { + return this.propertyPath; + } + + public Query getQuery() { + return query; } public String toString() { - return "DeleteRootByIdIn{entityType=" + this.entityType + ", selectIds=" + this.selectIds + "}"; + return "DbAction.DeleteByQuery(propertyPath=" + this.getPropertyPath() + ", query=" + this.query + ")"; } } @@ -407,13 +411,12 @@ public String toString() { } /** - * Represents an acquire lock statement on all aggregate roots of a given type, constrained by a query. - * This is used to select and lock aggregate root IDs matching a given {@link Query}, which can be reused - * in follow-up actions like batch deletion. + * Represents a {@code SELECT ... FOR UPDATE} statement on all aggregate roots of a given type, + * filtered by a {@link Query}. * * @param type of the root entity for which this represents a database interaction. */ - final class AcquireLockAllRootByQuery implements SelectIds { + final class AcquireLockAllRootByQuery implements DbAction { private final Class entityType; @@ -652,22 +655,4 @@ default Class getEntityType() { return (Class) getPropertyPath().getLeafProperty().getActualType(); } } - - /** - * A {@link DbAction} that depends on a {@link SelectIds} action - * - * @author Jaeyeon Kim - */ - interface WithSelectIds extends DbAction { - - DbAction.SelectIds getSelectIdsAction(); - } - - /** - * A {@link DbAction} that represents a query to select a list of root IDs. - * - * @author Jaeyeon Kim - */ - interface SelectIds extends DbAction { - } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java index 01ce4c4d0a..ffbc9cd360 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriter.java @@ -84,20 +84,20 @@ public void writeForQuery(Query query, MutableAggregateChange aggregateChange Class entityType = aggregateChange.getEntityType(); - DbAction.SelectIds selectIds = new DbAction.AcquireLockAllRootByQuery<>(entityType, query); + List> deleteReferencedActions = new ArrayList<>(); - List> actions = new ArrayList<>(); - actions.add(selectIds); + forAllTableRepresentingPaths(entityType, p -> deleteReferencedActions.add(new DbAction.DeleteByQuery<>(query, p))); - List> paths = new ArrayList<>(); - forAllTableRepresentingPaths(entityType, paths::add); - Collections.reverse(paths); + Collections.reverse(deleteReferencedActions); - for (PersistentPropertyPath path : paths) { - actions.add(new DbAction.DeleteByRootIdIn<>(selectIds, path)); + List> actions = new ArrayList<>(); + if (!deleteReferencedActions.isEmpty()) { + actions.add(new DbAction.AcquireLockAllRootByQuery<>(entityType, query)); } + actions.addAll(deleteReferencedActions); - actions.add(new DbAction.DeleteRootByIdIn<>(entityType, selectIds)); + DbAction.DeleteRootByQuery deleteRootByQuery = new DbAction.DeleteRootByQuery<>(entityType, query); + actions.add(deleteRootByQuery); actions.forEach(aggregateChange::addAction); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SelectIdsDbActionExecutionResult.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SelectIdsDbActionExecutionResult.java deleted file mode 100644 index 5824be6ba2..0000000000 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/conversion/SelectIdsDbActionExecutionResult.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Copyright 2017-2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.relational.core.conversion; - -import java.util.List; - -/** - * Represents the result of executing a {@link DbAction.SelectIds} operation. - * Typically used when selecting IDs (e.g., with a lock) as a pre-step to a delete operation. - * - * @author Jaeyeon Kim - * @since 4.0 - */ -public class SelectIdsDbActionExecutionResult { - - private final List selectedIds; - private final DbAction.SelectIds action; - - public SelectIdsDbActionExecutionResult(List selectedIds, DbAction.SelectIds action) { - this.selectedIds = selectedIds; - this.action = action; - } - - public List getSelectedIds() { - return this.selectedIds; - } - - public DbAction.SelectIds getAction() { - return action; - } -} diff --git a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java index 6c4e78d67f..1ff775e2fb 100644 --- a/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java +++ b/spring-data-relational/src/test/java/org/springframework/data/relational/core/conversion/RelationalEntityDeleteWriterUnitTests.java @@ -150,7 +150,7 @@ public void deleteAllDoesNotDeleteReadOnlyReferences() { void writeForQueryDeletesEntitiesByQueryAndReferencedEntities() { MutableAggregateChange aggregateChange = MutableAggregateChange.forDelete(SomeEntity.class); - Query query = Query.query(Criteria.empty()).limit(10).sort(Sort.by("name")); + Query query = Query.query(Criteria.empty()); converter.writeForQuery(query, aggregateChange); @@ -158,9 +158,9 @@ void writeForQueryDeletesEntitiesByQueryAndReferencedEntities() { .extracting(DbAction::getClass, DbAction::getEntityType, DbActionTestSupport::extractPath) .containsExactly( Tuple.tuple(DbAction.AcquireLockAllRootByQuery.class, SomeEntity.class, ""), - Tuple.tuple(DbAction.DeleteByRootIdIn.class, YetAnother.class, "other.yetAnother"), - Tuple.tuple(DbAction.DeleteByRootIdIn.class, OtherEntity.class, "other"), - Tuple.tuple(DbAction.DeleteRootByIdIn.class, SomeEntity.class, "") + Tuple.tuple(DbAction.DeleteByQuery.class, YetAnother.class, "other.yetAnother"), + Tuple.tuple(DbAction.DeleteByQuery.class, OtherEntity.class, "other"), + Tuple.tuple(DbAction.DeleteRootByQuery.class, SomeEntity.class, "") ); }