From 20ce1ef516a3d5e0852ef97b1ec79a0099d99522 Mon Sep 17 00:00:00 2001 From: Vlad Mihalcea Date: Thu, 15 Mar 2018 10:51:12 +0200 Subject: [PATCH] HHH-12362 - Allow both SQL query hints and comments --- .../java/org/hibernate/dialect/Dialect.java | 2 +- .../test/queryhint/QueryHintTest.java | 232 ++++++++++-------- 2 files changed, 136 insertions(+), 98 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index cf33c6f745..bcbc18b5ad 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -2985,7 +2985,7 @@ public String addSqlHintOrComment( if ( parameters.getQueryHints() != null && parameters.getQueryHints().size() > 0 ) { sql = getQueryHintString( sql, parameters.getQueryHints() ); } - else if ( commentsEnabled && parameters.getComment() != null ){ + if ( commentsEnabled && parameters.getComment() != null ){ sql = prependComment( sql, parameters.getComment() ); } diff --git a/hibernate-core/src/test/java/org/hibernate/test/queryhint/QueryHintTest.java b/hibernate-core/src/test/java/org/hibernate/test/queryhint/QueryHintTest.java index 8a2c040262..400886098e 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/queryhint/QueryHintTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/queryhint/QueryHintTest.java @@ -7,139 +7,177 @@ package org.hibernate.test.queryhint; import java.util.List; +import java.util.Map; import javax.persistence.Entity; +import javax.persistence.FetchType; import javax.persistence.GeneratedValue; import javax.persistence.Id; import javax.persistence.ManyToOne; import org.hibernate.Criteria; -import org.hibernate.Query; -import org.hibernate.Session; import org.hibernate.cfg.AvailableSettings; -import org.hibernate.cfg.Configuration; import org.hibernate.criterion.Restrictions; import org.hibernate.dialect.Oracle8iDialect; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.query.Query; import org.hibernate.testing.RequiresDialect; -import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase; +import org.hibernate.test.util.jdbc.PreparedStatementSpyConnectionProvider; import org.junit.Test; +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNotNull; /** * @author Brett Meyer */ @RequiresDialect( Oracle8iDialect.class ) -public class QueryHintTest extends BaseCoreFunctionalTestCase { - +public class QueryHintTest extends BaseNonConfigCoreFunctionalTestCase { + + private PreparedStatementSpyConnectionProvider connectionProvider; + + @Override + protected void addSettings(Map settings) { + settings.put( AvailableSettings.USE_SQL_COMMENTS, "true" ); + settings.put( + org.hibernate.cfg.AvailableSettings.CONNECTION_PROVIDER, + connectionProvider + ); + } + + @Override + protected void buildResources() { + connectionProvider = new PreparedStatementSpyConnectionProvider(); + super.buildResources(); + } + + @Override + public void releaseResources() { + super.releaseResources(); + connectionProvider.stop(); + } + @Override protected Class[] getAnnotatedClasses() { return new Class[] { Employee.class, Department.class }; } - + @Override - protected void configure(Configuration configuration) { - configuration.setProperty( AvailableSettings.DIALECT, QueryHintTestDialect.class.getName() ); - configuration.setProperty( AvailableSettings.USE_SQL_COMMENTS, "true" ); - } - - @Test - public void testQueryHint() { + protected void afterSessionFactoryBuilt(SessionFactoryImplementor sessionFactory) { Department department = new Department(); department.name = "Sales"; Employee employee1 = new Employee(); employee1.department = department; Employee employee2 = new Employee(); employee2.department = department; - - Session s = openSession(); - s.getTransaction().begin(); - s.persist( department ); s.persist( employee1 ); - s.persist( employee2 ); - s.getTransaction().commit(); - s.clear(); + + doInHibernate( this::sessionFactory, s -> { + s.persist( department ); + s.persist( employee1 ); + s.persist( employee2 ); + } ); + } + + @Test + public void testQueryHint() { + + connectionProvider.clear(); // test Query w/ a simple Oracle optimizer hint - s.getTransaction().begin(); - Query query = s.createQuery( "FROM QueryHintTest$Employee e WHERE e.department.name = :departmentName" ) - .addQueryHint( "ALL_ROWS" ) - .setParameter( "departmentName", "Sales" ); - List results = query.list(); - s.getTransaction().commit(); - s.clear(); - - assertEquals(results.size(), 2); - assertTrue(QueryHintTestDialect.getProcessedSql().contains( "select /*+ ALL_ROWS */")); - - QueryHintTestDialect.resetProcessedSql(); - + doInHibernate( this::sessionFactory, s -> { + Query query = s.createQuery( "FROM QueryHintTest$Employee e WHERE e.department.name = :departmentName" ) + .addQueryHint( "ALL_ROWS" ) + .setParameter( "departmentName", "Sales" ); + List results = query.list(); + + assertEquals(results.size(), 2); + } ); + + assertEquals( + 1, + connectionProvider.getPreparedStatements().size() + ); + assertNotNull( connectionProvider.getPreparedSQLStatements().get( 0 ).contains( "select /*+ ALL_ROWS */" ) ); + connectionProvider.clear(); + // test multiple hints - s.getTransaction().begin(); - query = s.createQuery( "FROM QueryHintTest$Employee e WHERE e.department.name = :departmentName" ) - .addQueryHint( "ALL_ROWS" ) - .addQueryHint( "USE_CONCAT" ) - .setParameter( "departmentName", "Sales" ); - results = query.list(); - s.getTransaction().commit(); - s.clear(); - - assertEquals(results.size(), 2); - assertTrue(QueryHintTestDialect.getProcessedSql().contains( "select /*+ ALL_ROWS, USE_CONCAT */")); - - QueryHintTestDialect.resetProcessedSql(); + doInHibernate( this::sessionFactory, s -> { + Query query = s.createQuery( "FROM QueryHintTest$Employee e WHERE e.department.name = :departmentName" ) + .addQueryHint( "ALL_ROWS" ) + .addQueryHint( "USE_CONCAT" ) + .setParameter( "departmentName", "Sales" ); + List results = query.list(); + + assertEquals(results.size(), 2); + } ); + + assertEquals( + 1, + connectionProvider.getPreparedStatements().size() + ); + assertNotNull( connectionProvider.getPreparedSQLStatements().get( 0 ).contains( "select /*+ ALL_ROWS, USE_CONCAT */" ) ); + connectionProvider.clear(); // ensure the insertion logic can handle a comment appended to the front - s.getTransaction().begin(); - query = s.createQuery( "FROM QueryHintTest$Employee e WHERE e.department.name = :departmentName" ) - .setComment( "this is a test" ) - .addQueryHint( "ALL_ROWS" ) - .setParameter( "departmentName", "Sales" ); - results = query.list(); - s.getTransaction().commit(); - s.clear(); - - assertEquals(results.size(), 2); - assertTrue(QueryHintTestDialect.getProcessedSql().contains( "select /*+ ALL_ROWS */")); - - QueryHintTestDialect.resetProcessedSql(); - + doInHibernate( this::sessionFactory, s -> { + Query query = s.createQuery( "FROM QueryHintTest$Employee e WHERE e.department.name = :departmentName" ) + .setComment( "this is a test" ) + .addQueryHint( "ALL_ROWS" ) + .setParameter( "departmentName", "Sales" ); + List results = query.list(); + + assertEquals(results.size(), 2); + } ); + + assertEquals( + 1, + connectionProvider.getPreparedStatements().size() + ); + assertNotNull( connectionProvider.getPreparedSQLStatements().get( 0 ).contains( "select /*+ ALL_ROWS */" ) ); + connectionProvider.clear(); + // test Criteria - s.getTransaction().begin(); - Criteria criteria = s.createCriteria( Employee.class ) - .addQueryHint( "ALL_ROWS" ) - .createCriteria( "department" ).add( Restrictions.eq( "name", "Sales" ) ); - results = criteria.list(); - s.getTransaction().commit(); - s.close(); - - assertEquals(results.size(), 2); - assertTrue(QueryHintTestDialect.getProcessedSql().contains( "select /*+ ALL_ROWS */")); + doInHibernate( this::sessionFactory, s -> { + Criteria criteria = s.createCriteria( Employee.class ) + .addQueryHint( "ALL_ROWS" ) + .createCriteria( "department" ).add( Restrictions.eq( "name", "Sales" ) ); + List results = criteria.list(); + + assertEquals(results.size(), 2); + } ); + + assertEquals( + 1, + connectionProvider.getPreparedStatements().size() + ); + assertNotNull( connectionProvider.getPreparedSQLStatements().get( 0 ).contains( "select /*+ ALL_ROWS */" ) ); + connectionProvider.clear(); } - - /** - * Since the query hint is added to the SQL during Loader's executeQueryStatement -> preprocessSQL, rather than - * early on during the QueryTranslator or QueryLoader initialization, there's not an easy way to check the full SQL - * after completely processing it. Instead, use this ridiculous hack to ensure Loader actually calls Dialect. - * - * TODO: This is terrible. Better ideas? - */ - public static class QueryHintTestDialect extends Oracle8iDialect { - private static String processedSql; - - @Override - public String getQueryHintString(String sql, List hints) { - processedSql = super.getQueryHintString( sql, hints ); - return processedSql; - } - - public static String getProcessedSql() { - return processedSql; - } - - public static void resetProcessedSql() { - processedSql = ""; - } + + @Test + @TestForIssue( jiraKey = "HHH-12362" ) + public void testQueryHintAndComment() { + connectionProvider.clear(); + + doInHibernate( this::sessionFactory, s -> { + Query query = s.createQuery( "FROM QueryHintTest$Employee e WHERE e.department.name = :departmentName" ) + .addQueryHint( "ALL_ROWS" ) + .setComment( "My_Query" ) + .setParameter( "departmentName", "Sales" ); + List results = query.list(); + + assertEquals(results.size(), 2); + } ); + + assertEquals( + 1, + connectionProvider.getPreparedStatements().size() + ); + assertNotNull( connectionProvider.getPreparedSQLStatements().get( 0 ).contains( "/* My_Query */ select /*+ ALL_ROWS */" ) ); + connectionProvider.clear(); } @Entity @@ -148,7 +186,7 @@ public static class Employee { @GeneratedValue public long id; - @ManyToOne + @ManyToOne(fetch = FetchType.LAZY) public Department department; }