From 094f2434131cf4f379a0b96cc18e2bdd80e148f7 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Thu, 6 Apr 2023 14:18:21 +0200 Subject: [PATCH] HHH-16392 Fix where clause in collection cleanup subqueries --- .../userguide/appendices/Annotations.adoc | 2 +- .../java/org/hibernate/annotations/Where.java | 2 +- .../sqm/internal/SimpleDeleteQueryPlan.java | 10 ++- .../internal/SqmMutationStrategyHelper.java | 89 +++++++++++++++++++ .../MutationQueriesWhereAndFilterTest.java | 10 +-- .../MutationQueriesWhereTest.java | 10 +-- 6 files changed, 108 insertions(+), 15 deletions(-) diff --git a/documentation/src/main/asciidoc/userguide/appendices/Annotations.adoc b/documentation/src/main/asciidoc/userguide/appendices/Annotations.adoc index 12435bdf33..1ab54652ed 100644 --- a/documentation/src/main/asciidoc/userguide/appendices/Annotations.adoc +++ b/documentation/src/main/asciidoc/userguide/appendices/Annotations.adoc @@ -1427,7 +1427,7 @@ See the <> section for more info. diff --git a/hibernate-core/src/main/java/org/hibernate/annotations/Where.java b/hibernate-core/src/main/java/org/hibernate/annotations/Where.java index e507221a05..b517e5b436 100644 --- a/hibernate-core/src/main/java/org/hibernate/annotations/Where.java +++ b/hibernate-core/src/main/java/org/hibernate/annotations/Where.java @@ -16,7 +16,7 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME; /** * Specifies a restriction written in native SQL to add to the generated - * SQL when querying an entity or collection. + * SQL for entities or collections. *

* For example, {@code @Where} could be used to hide entity instances which * have been soft-deleted, either for the entity class itself: diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SimpleDeleteQueryPlan.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SimpleDeleteQueryPlan.java index fe12e52967..ec7ed955f9 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SimpleDeleteQueryPlan.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SimpleDeleteQueryPlan.java @@ -124,8 +124,7 @@ public class SimpleDeleteQueryPlan implements NonSelectQueryPlan { jdbcDelete = deleteTranslator.translate( jdbcParameterBindings, executionContext.getQueryOptions() ); } - final boolean missingRestriction = sqmDelete.getWhereClause() == null - || sqmDelete.getWhereClause().getPredicate() == null; + final boolean missingRestriction = sqmInterpretation.getSqlAst().getRestriction() == null; if ( missingRestriction ) { assert domainParameterXref.getSqmParameterCount() == 0; assert jdbcParamsXref.isEmpty(); @@ -171,7 +170,12 @@ public class SimpleDeleteQueryPlan implements NonSelectQueryPlan { tableGroup ); - matchingIdSubQuery.applyPredicate( sqmInterpretation.getSqlAst().getRestriction() ); + matchingIdSubQuery.applyPredicate( SqmMutationStrategyHelper.getIdSubqueryPredicate( + sqmInterpretation.getSqlAst().getRestriction(), + entityDescriptor, + tableGroup, + session + ) ); return new InSubQueryPredicate( fkColumnExpression, matchingIdSubQuery, false ); }, diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/SqmMutationStrategyHelper.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/SqmMutationStrategyHelper.java index 6ad9f6d8ca..d53bd21fa7 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/SqmMutationStrategyHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/mutation/internal/SqmMutationStrategyHelper.java @@ -6,17 +6,24 @@ */ package org.hibernate.query.sqm.mutation.internal; +import java.util.ArrayList; +import java.util.List; import java.util.function.BiFunction; import java.util.function.Consumer; import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.metamodel.mapping.EntityMappingType; import org.hibernate.metamodel.mapping.PluralAttributeMapping; import org.hibernate.metamodel.mapping.internal.EmbeddedAttributeMapping; +import org.hibernate.persister.internal.SqlFragmentPredicate; import org.hibernate.sql.ast.tree.delete.DeleteStatement; import org.hibernate.sql.ast.tree.from.NamedTableReference; +import org.hibernate.sql.ast.tree.from.TableGroup; import org.hibernate.sql.ast.tree.from.TableReference; +import org.hibernate.sql.ast.tree.predicate.FilterPredicate; +import org.hibernate.sql.ast.tree.predicate.Junction; import org.hibernate.sql.ast.tree.predicate.Predicate; import org.hibernate.sql.exec.spi.ExecutionContext; import org.hibernate.sql.exec.spi.JdbcParameterBindings; @@ -179,4 +186,86 @@ public class SqmMutationStrategyHelper { ); } } + + /** + * Translates the original delete predicate to be used in the id subquery + * forcing the use of the table alias qualifier + */ + public static Predicate getIdSubqueryPredicate( + Predicate predicate, + EntityMappingType entityDescriptor, + TableGroup tableGroup, + SharedSessionContractImplementor session) { + if ( predicate instanceof FilterPredicate || predicate instanceof SqlFragmentPredicate ) { + return getBaseRestrictions( entityDescriptor, tableGroup, session ).get( 0 ); + } + else if ( predicate instanceof Junction ) { + final Junction original = (Junction) predicate; + if ( original.getPredicates().size() > 1 ) { + final Junction junction = new Junction( + original.getNature(), + original.getExpressionType() + ); + junction.getPredicates().addAll( original.getPredicates() ); + final Predicate secondToLastPredicate = junction.getPredicates().get( junction.getPredicates().size() - 2 ); + final Predicate lastPredicate = junction.getPredicates().get( junction.getPredicates().size() - 1 ); + int filterPredicateIndex = -1; + int fragmentPredicateIndex = -1; + if ( lastPredicate instanceof Junction ) { + // If the mutation query specified an explicit where condition and there are multiple base + // restrictions they will be in a nested Junction predicate, so we need to process that one + final Predicate baseRestrictions = getIdSubqueryPredicate( + lastPredicate, + entityDescriptor, + tableGroup, + session + ); + junction.getPredicates().set( junction.getPredicates().size() - 1, baseRestrictions ); + predicate = junction; + } + else if ( secondToLastPredicate instanceof FilterPredicate ) { + filterPredicateIndex = junction.getPredicates().size() - 2; + fragmentPredicateIndex = filterPredicateIndex + 1; + } + else if ( lastPredicate instanceof FilterPredicate ) { + filterPredicateIndex = junction.getPredicates().size() - 1; + } + else if ( lastPredicate instanceof SqlFragmentPredicate ) { + fragmentPredicateIndex = junction.getPredicates().size() - 1; + } + if ( filterPredicateIndex != -1 || fragmentPredicateIndex != -1 ) { + final List baseRestrictions = getBaseRestrictions( + entityDescriptor, + tableGroup, + session + ); + int index = 0; + if ( filterPredicateIndex != -1 ) { + junction.getPredicates().set( filterPredicateIndex, baseRestrictions.get( index++ ) ); + } + if ( fragmentPredicateIndex != -1 ) { + junction.getPredicates().set( fragmentPredicateIndex, baseRestrictions.get( index ) ); + } + predicate = junction; + } + } + } + return predicate; + } + + private static List getBaseRestrictions( + EntityMappingType entityDescriptor, + TableGroup tableGroup, + SharedSessionContractImplementor session) { + final List baseRestrictions = new ArrayList<>( 2 ); + entityDescriptor.applyBaseRestrictions( + baseRestrictions::add, + tableGroup, + true, + session.getLoadQueryInfluencers().getEnabledFilters(), + null, + null + ); + return baseRestrictions; + } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/mutationquery/MutationQueriesWhereAndFilterTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/mutationquery/MutationQueriesWhereAndFilterTest.java index cefc729553..064f3eedfa 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/mutationquery/MutationQueriesWhereAndFilterTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/mutationquery/MutationQueriesWhereAndFilterTest.java @@ -10,7 +10,7 @@ import java.util.List; import org.hibernate.annotations.Filter; import org.hibernate.annotations.FilterDef; -import org.hibernate.annotations.SQLRestriction; +import org.hibernate.annotations.Where; import org.hibernate.query.MutationQuery; import org.hibernate.testing.jdbc.SQLStatementInspector; @@ -36,7 +36,7 @@ import static java.util.Arrays.stream; /** * Same as {@link MutationQueriesWhereTest} and {@link MutationQueriesFilterTest}, - * but using both {@link SQLRestriction @SQLRestriction} and {@link Filter @Filter} + * but using both {@link Where @Where} and {@link Filter @Filter} * * @author Marco Belladelli */ @@ -176,7 +176,7 @@ public class MutationQueriesWhereAndFilterTest { } @Entity( name = "UserEntity" ) - @SQLRestriction( "where_deleted = false" ) + @Where( clause = "where_deleted = false" ) @FilterDef( name = "deleted_filter", defaultCondition = "filter_deleted = false" ) @Filter( name = "deleted_filter" ) public static class UserEntity { @@ -199,7 +199,7 @@ public class MutationQueriesWhereAndFilterTest { @Entity( name = "DiscriminatorBase" ) @Inheritance( strategy = InheritanceType.SINGLE_TABLE ) @DiscriminatorColumn( name = "disc_col" ) - @SQLRestriction( "where_deleted = false" ) + @Where( clause = "where_deleted = false" ) @Filter( name = "deleted_filter" ) public static class DiscriminatorBase { @Id @@ -225,7 +225,7 @@ public class MutationQueriesWhereAndFilterTest { @Entity( name = "TablePerClassBase" ) @Inheritance( strategy = InheritanceType.TABLE_PER_CLASS ) - @SQLRestriction( "where_deleted = false" ) + @Where( clause = "where_deleted = false" ) @Filter( name = "deleted_filter" ) public static abstract class TablePerClassBase { @Id diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/mutationquery/MutationQueriesWhereTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/mutationquery/MutationQueriesWhereTest.java index 20f758dde4..7cbc63b2c2 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/mutationquery/MutationQueriesWhereTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/mutationquery/MutationQueriesWhereTest.java @@ -8,7 +8,7 @@ package org.hibernate.orm.test.query.mutationquery; import java.util.List; -import org.hibernate.annotations.SQLRestriction; +import org.hibernate.annotations.Where; import org.hibernate.query.MutationQuery; import org.hibernate.testing.jdbc.SQLStatementInspector; @@ -34,7 +34,7 @@ import static java.util.Arrays.stream; /** * Same as {@link MutationQueriesFilterTest} and {@link MutationQueriesWhereAndFilterTest}, - * but using only {@link SQLRestriction @SQLRestriction} + * but using only {@link Where @Where} * * @author Marco Belladelli */ @@ -164,7 +164,7 @@ public class MutationQueriesWhereTest { } @Entity( name = "UserEntity" ) - @SQLRestriction( "deleted = false" ) + @Where( clause = "deleted = false" ) public static class UserEntity { @Id private Long id; @@ -183,7 +183,7 @@ public class MutationQueriesWhereTest { @Entity( name = "DiscriminatorBase" ) @Inheritance( strategy = InheritanceType.SINGLE_TABLE ) @DiscriminatorColumn( name = "disc_col" ) - @SQLRestriction( "deleted = false" ) + @Where( clause = "deleted = false" ) public static class DiscriminatorBase { @Id private Long id; @@ -206,7 +206,7 @@ public class MutationQueriesWhereTest { @Entity( name = "TablePerClassBase" ) @Inheritance( strategy = InheritanceType.TABLE_PER_CLASS ) - @SQLRestriction( "deleted = false" ) + @Where( clause = "deleted = false" ) public static abstract class TablePerClassBase { @Id private Long id;