HHH-9965 : Pagination ignored on collection fetch join: Add the abili…

…ty to throw an exception instead of logging a warn (cherry picked from commit b711e14)
hibernate · Jan 30, 2018 · dbd7135 · dbd7135
1 parent d34f026
commit dbd7135
Show file tree

Hide file tree

Showing 10 changed files with 193 additions and 5 deletions.
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java
@@ -577,6 +577,8 @@ public static class SessionFactoryOptionsStateStandardImpl implements SessionFac
 
 		private Map<String, SQLFunction> sqlFunctions;
 
+		private boolean failOnPaginationOverCollectionFetchEnabled;
+
 		public SessionFactoryOptionsStateStandardImpl(StandardServiceRegistry serviceRegistry) {
 			this.serviceRegistry = serviceRegistry;
 
@@ -790,6 +792,12 @@ else if ( jdbcTimeZoneValue != null ) {
 			this.criteriaLiteralHandlingMode = LiteralHandlingMode.interpret(
 				configurationSettings.get( CRITERIA_LITERAL_HANDLING_MODE )
 			);
+
+			this.failOnPaginationOverCollectionFetchEnabled = ConfigurationHelper.getBoolean(
+					FAIL_ON_PAGINATION_OVER_COLLECTION_FETCH,
+					configurationSettings,
+					false
+			);
 		}
 
 		private static Interceptor determineInterceptor(Map configurationSettings, StrategySelector strategySelector) {
@@ -1243,6 +1251,11 @@ public boolean isQueryParametersValidationEnabled() {
 		public LiteralHandlingMode getCriteriaLiteralHandlingMode() {
 			return this.criteriaLiteralHandlingMode;
 		}
+
+		@Override
+		public boolean isFailOnPaginationOverCollectionFetchEnabled() {
+			return this.failOnPaginationOverCollectionFetchEnabled;
+		}
 	}
 
 	private static Supplier<? extends Interceptor> interceptorSupplier(Class<? extends Interceptor> clazz) {
@@ -1592,4 +1605,8 @@ public boolean isQueryParametersValidationEnabled() {
 	public LiteralHandlingMode getCriteriaLiteralHandlingMode() {
 		return options.getCriteriaLiteralHandlingMode();
 	}
+	@Override
+	public boolean isFailOnPaginationOverCollectionFetchEnabled() {
+		return options.isFailOnPaginationOverCollectionFetchEnabled();
+	}
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsImpl.java
@@ -131,6 +131,7 @@ public class SessionFactoryOptionsImpl implements SessionFactoryOptions {
 	private final Map<String, SQLFunction> sqlFunctions;
 	private boolean queryParametersValidationEnabled;
 	private LiteralHandlingMode criteriaLiteralHandlingMode;
+	private final boolean failOnPaginationOverCollectionFetchEnabled;
 
 	public SessionFactoryOptionsImpl(SessionFactoryOptionsState state) {
 		this.serviceRegistry = state.getServiceRegistry();
@@ -213,6 +214,8 @@ public SessionFactoryOptionsImpl(SessionFactoryOptionsState state) {
 
 		this.queryParametersValidationEnabled = state.isQueryParametersValidationEnabled();
 		this.criteriaLiteralHandlingMode = state.getCriteriaLiteralHandlingMode();
+
+		this.failOnPaginationOverCollectionFetchEnabled = state.isFailOnPaginationOverCollectionFetchEnabled();
 	}
 
 	@Override
@@ -558,4 +561,9 @@ public boolean isQueryParametersValidationEnabled() {
 	public LiteralHandlingMode getCriteriaLiteralHandlingMode() {
 		return criteriaLiteralHandlingMode;
 	}
+
+	@Override
+	public boolean isFailOnPaginationOverCollectionFetchEnabled() {
+		return failOnPaginationOverCollectionFetchEnabled;
+	}
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java
@@ -203,4 +203,8 @@ default Supplier<? extends Interceptor> getStatelessInterceptorImplementorSuppli
 	default LiteralHandlingMode getCriteriaLiteralHandlingMode() {
 		return LiteralHandlingMode.AUTO;
 	}
+
+	default boolean isFailOnPaginationOverCollectionFetchEnabled() {
+		return false;
+	}
 }
diff --git a/...te-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java b/...te-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java
@@ -395,4 +395,9 @@ public boolean isQueryParametersValidationEnabled() {
 	public LiteralHandlingMode getCriteriaLiteralHandlingMode() {
 		return delegate.getCriteriaLiteralHandlingMode();
 	}
+
+	@Override
+	public boolean isFailOnPaginationOverCollectionFetchEnabled() {
+		return delegate.isFailOnPaginationOverCollectionFetchEnabled();
+	}
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java b/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java
@@ -248,4 +248,8 @@ default boolean isQueryParametersValidationEnabled(){
 	default LiteralHandlingMode getCriteriaLiteralHandlingMode() {
 		return LiteralHandlingMode.AUTO;
 	}
+
+	default boolean isFailOnPaginationOverCollectionFetchEnabled() {
+		return false;
+	}
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java
@@ -1121,7 +1121,7 @@ public interface AvailableSettings {
 	 *     <li>{@link org.hibernate.engine.query.spi.FilterQueryPlan}</li>
 	 *     <li>{@link org.hibernate.engine.query.spi.NativeSQLQueryPlan}</li>
 	 * </ul>
-	 * 
+	 *
 	 * maintained by {@link org.hibernate.engine.query.spi.QueryPlanCache}. Default is 2048.
 	 */
 	String QUERY_PLAN_CACHE_MAX_SIZE = "hibernate.query.plan_cache_max_size";
@@ -1501,14 +1501,14 @@ public interface AvailableSettings {
 	 * {@code LEGACY} is the default value.
 	 */
 	String BATCH_FETCH_STYLE = "hibernate.batch_fetch_style";
-	
+
 	/**
 	 * A transaction can be rolled back by another thread ("tracking by thread")
 	 * -- not the original application. Examples of this include a JTA
 	 * transaction timeout handled by a background reaper thread.  The ability
 	 * to handle this situation requires checking the Thread ID every time
 	 * Session is called.  This can certainly have performance considerations.
-	 * 
+	 *
 	 * Default is <code>true</code> (enabled).
 	 */
 	String JTA_TRACK_BY_THREAD = "hibernate.jta.track_by_thread";
@@ -1538,7 +1538,7 @@ public interface AvailableSettings {
 	 * SchemaUpdate needs to create these constraints, but DB's
 	 * support for finding existing constraints is extremely inconsistent. Further,
 	 * non-explicitly-named unique constraints use randomly generated characters.
-	 * 
+	 *
 	 * Therefore, select from these strategies.
 	 * {@link org.hibernate.tool.hbm2ddl.UniqueConstraintSchemaUpdateStrategy#DROP_RECREATE_QUIETLY} (DEFAULT):
 	 * 			Attempt to drop, then (re-)create each unique constraint.
@@ -1713,4 +1713,10 @@ public interface AvailableSettings {
 	 * @see org.hibernate.query.criteria.LiteralHandlingMode
 	 */
 	String CRITERIA_LITERAL_HANDLING_MODE = "hibernate.criteria.literal_handling_mode";
+
+	/**
+	 * Raises an exception when in-memory pagination over collection fetch is about to be performed.
+	 * Disabled by default. Set to true to enable.
+	 */
+	String FAIL_ON_PAGINATION_OVER_COLLECTION_FETCH = "hibernate.query.fail_on_pagination_over_collection_fetch";
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QueryTranslatorImpl.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/QueryTranslatorImpl.java
@@ -371,7 +371,15 @@ public List list(SharedSessionContractImplementor session, QueryParameters query
 
 		QueryParameters queryParametersToUse;
 		if ( hasLimit && containsCollectionFetches() ) {
-			LOG.firstOrMaxResultsSpecifiedWithCollectionFetch();
+			boolean fail = session.getFactory().getSessionFactoryOptions().isFailOnPaginationOverCollectionFetchEnabled();
+			if (fail) {
+				throw new HibernateException("firstResult/maxResults specified with collection fetch. " +
+						"In memory pagination was about to be applied. " +
+						"Failing because 'Fail on pagination over collection fetch' is enabled.");
+			}
+			else {
+				LOG.firstOrMaxResultsSpecifiedWithCollectionFetch();
+			}
 			RowSelection selection = new RowSelection();
 			selection.setFetchSize( queryParameters.getRowSelection().getFetchSize() );
 			selection.setTimeout( queryParameters.getRowSelection().getTimeout() );

diff --git a/hibernate-core/src/test/java/org/hibernate/test/pagination/hhh9965/HHH9965Test.java b/hibernate-core/src/test/java/org/hibernate/test/pagination/hhh9965/HHH9965Test.java
@@ -0,0 +1,60 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.test.pagination.hhh9965;
+
+import org.hibernate.Session;
+import org.hibernate.cfg.Configuration;
+import org.hibernate.cfg.Environment;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.junit.Test;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Created on 17/12/17.
+ *
+ * @author Reda.Housni-Alaoui
+ */
+@TestForIssue(jiraKey = "HHH-9965")
+public class HHH9965Test extends BaseCoreFunctionalTestCase {
+
+    @Test
+    public void testHHH9965() {
+        Session session = openSession();
+        session.beginTransaction();
+
+        String hql = "SELECT s FROM Shop s join fetch s.products";
+
+        try{
+            session.createQuery(hql)
+                    .setMaxResults(3)
+                    .list();
+            fail("Pagination over collection fetch failure was expected");
+        } catch (Exception e){
+            log.info(e.getMessage());
+        }
+        finally {
+            session.getTransaction().rollback();
+            session.close();
+        }
+    }
+
+    @Override
+    protected void configure(Configuration cfg) {
+        super.configure(cfg);
+        cfg.setProperty( Environment.FAIL_ON_PAGINATION_OVER_COLLECTION_FETCH, "true");
+    }
+
+    @Override
+    protected Class<?>[] getAnnotatedClasses() {
+        return new Class<?>[]{
+                Shop.class,
+                Product.class
+        };
+    }
+}
diff --git a/hibernate-core/src/test/java/org/hibernate/test/pagination/hhh9965/Product.java b/hibernate-core/src/test/java/org/hibernate/test/pagination/hhh9965/Product.java
@@ -0,0 +1,33 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.test.pagination.hhh9965;
+
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+/**
+ * Created on 17/12/17.
+ *
+ * @author Reda.Housni-Alaoui
+ */
+@Entity
+public class Product {
+
+    @Id
+    @GeneratedValue
+    private Long id;
+
+
+    public Long getId() {
+        return id;
+    }
+
+    public void setId(Long id) {
+        this.id = id;
+    }
+}
diff --git a/hibernate-core/src/test/java/org/hibernate/test/pagination/hhh9965/Shop.java b/hibernate-core/src/test/java/org/hibernate/test/pagination/hhh9965/Shop.java
@@ -0,0 +1,43 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.test.pagination.hhh9965;
+
+import javax.persistence.*;
+import java.util.List;
+
+/**
+ * Created on 17/12/17.
+ *
+ * @author Reda.Housni-Alaoui
+ */
+@Entity
+public class Shop {
+
+    @Id
+    @GeneratedValue
+    private Long id;
+
+    @OneToMany(fetch = FetchType.LAZY)
+    private List<Product> products;
+
+    public Long getId() {
+        return id;
+    }
+
+    public void setId(Long id) {
+        this.id = id;
+    }
+
+    public List<Product> getProducts() {
+        return products;
+    }
+
+    public void setProducts(List<Product> products) {
+        this.products = products;
+    }
+
+}