From 556aa265c07b99b3d83d227ffe144567a8c00819 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Fri, 21 Oct 2016 18:14:42 +0200 Subject: [PATCH] Add property for disabling subquery join rewrites and handle mysql quoted identifiers --- .../internal/SessionFactoryBuilderImpl.java | 62 ++++--------------- .../internal/SessionFactoryOptionsImpl.java | 7 +++ .../internal/SessionFactoryOptionsState.java | 2 + ...stractDelegatingSessionFactoryOptions.java | 5 ++ .../boot/spi/SessionFactoryOptions.java | 2 + .../org/hibernate/cfg/AvailableSettings.java | 10 +++ .../engine/internal/JoinSequence.java | 6 +- .../hibernate/test/hql/WithClauseTest.java | 25 ++++++++ .../junit4/BaseCoreFunctionalTestCase.java | 20 ++++-- 9 files changed, 84 insertions(+), 55 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 index bad3e7de5c..5290597373 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryBuilderImpl.java @@ -61,55 +61,7 @@ import org.hibernate.tuple.entity.EntityTuplizerFactory; import org.jboss.logging.Logger; -import static org.hibernate.cfg.AvailableSettings.ACQUIRE_CONNECTIONS; -import static org.hibernate.cfg.AvailableSettings.ALLOW_JTA_TRANSACTION_ACCESS; -import static org.hibernate.cfg.AvailableSettings.AUTO_CLOSE_SESSION; -import static org.hibernate.cfg.AvailableSettings.AUTO_EVICT_COLLECTION_CACHE; -import static org.hibernate.cfg.AvailableSettings.AUTO_SESSION_EVENTS_LISTENER; -import static org.hibernate.cfg.AvailableSettings.BATCH_FETCH_STYLE; -import static org.hibernate.cfg.AvailableSettings.BATCH_VERSIONED_DATA; -import static org.hibernate.cfg.AvailableSettings.CACHE_REGION_PREFIX; -import static org.hibernate.cfg.AvailableSettings.CHECK_NULLABILITY; -import static org.hibernate.cfg.AvailableSettings.CONNECTION_HANDLING; -import static org.hibernate.cfg.AvailableSettings.CUSTOM_ENTITY_DIRTINESS_STRATEGY; -import static org.hibernate.cfg.AvailableSettings.DEFAULT_BATCH_FETCH_SIZE; -import static org.hibernate.cfg.AvailableSettings.DEFAULT_ENTITY_MODE; -import static org.hibernate.cfg.AvailableSettings.ENABLE_LAZY_LOAD_NO_TRANS; -import static org.hibernate.cfg.AvailableSettings.FLUSH_BEFORE_COMPLETION; -import static org.hibernate.cfg.AvailableSettings.GENERATE_STATISTICS; -import static org.hibernate.cfg.AvailableSettings.HQL_BULK_ID_STRATEGY; -import static org.hibernate.cfg.AvailableSettings.INTERCEPTOR; -import static org.hibernate.cfg.AvailableSettings.JDBC_TIME_ZONE; -import static org.hibernate.cfg.AvailableSettings.JPAQL_STRICT_COMPLIANCE; -import static org.hibernate.cfg.AvailableSettings.JTA_TRACK_BY_THREAD; -import static org.hibernate.cfg.AvailableSettings.LOG_SESSION_METRICS; -import static org.hibernate.cfg.AvailableSettings.MAX_FETCH_DEPTH; -import static org.hibernate.cfg.AvailableSettings.MULTI_TENANT_IDENTIFIER_RESOLVER; -import static org.hibernate.cfg.AvailableSettings.ORDER_INSERTS; -import static org.hibernate.cfg.AvailableSettings.ORDER_UPDATES; -import static org.hibernate.cfg.AvailableSettings.PREFER_USER_TRANSACTION; -import static org.hibernate.cfg.AvailableSettings.PROCEDURE_NULL_PARAM_PASSING; -import static org.hibernate.cfg.AvailableSettings.QUERY_CACHE_FACTORY; -import static org.hibernate.cfg.AvailableSettings.QUERY_STARTUP_CHECKING; -import static org.hibernate.cfg.AvailableSettings.QUERY_SUBSTITUTIONS; -import static org.hibernate.cfg.AvailableSettings.RELEASE_CONNECTIONS; -import static org.hibernate.cfg.AvailableSettings.SESSION_FACTORY_NAME; -import static org.hibernate.cfg.AvailableSettings.SESSION_FACTORY_NAME_IS_JNDI; -import static org.hibernate.cfg.AvailableSettings.SESSION_SCOPED_INTERCEPTOR; -import static org.hibernate.cfg.AvailableSettings.STATEMENT_BATCH_SIZE; -import static org.hibernate.cfg.AvailableSettings.STATEMENT_FETCH_SIZE; -import static org.hibernate.cfg.AvailableSettings.STATEMENT_INSPECTOR; -import static org.hibernate.cfg.AvailableSettings.USE_DIRECT_REFERENCE_CACHE_ENTRIES; -import static org.hibernate.cfg.AvailableSettings.USE_GET_GENERATED_KEYS; -import static org.hibernate.cfg.AvailableSettings.USE_IDENTIFIER_ROLLBACK; -import static org.hibernate.cfg.AvailableSettings.USE_MINIMAL_PUTS; -import static org.hibernate.cfg.AvailableSettings.USE_QUERY_CACHE; -import static org.hibernate.cfg.AvailableSettings.USE_SCROLLABLE_RESULTSET; -import static org.hibernate.cfg.AvailableSettings.USE_SECOND_LEVEL_CACHE; -import static org.hibernate.cfg.AvailableSettings.USE_SQL_COMMENTS; -import static org.hibernate.cfg.AvailableSettings.USE_STRUCTURED_CACHE; -import static org.hibernate.cfg.AvailableSettings.WRAP_RESULT_SETS; -import static org.hibernate.cfg.AvailableSettings.ALLOW_UPDATE_OUTSIDE_TRANSACTION; +import static org.hibernate.cfg.AvailableSettings.*; import static org.hibernate.engine.config.spi.StandardConverters.BOOLEAN; import static org.hibernate.jpa.AvailableSettings.DISCARD_PC_ON_CLOSE; @@ -571,6 +523,7 @@ public class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement private boolean strictJpaQueryLanguageCompliance; private boolean namedQueryStartupCheckingEnabled; private final boolean procedureParameterNullPassingEnabled; + private final boolean collectionJoinSubqueryRewriteEnabled; // Caching private boolean secondLevelCacheEnabled; @@ -690,6 +643,7 @@ public class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement this.strictJpaQueryLanguageCompliance = cfgService.getSetting( JPAQL_STRICT_COMPLIANCE, BOOLEAN, false ); this.namedQueryStartupCheckingEnabled = cfgService.getSetting( QUERY_STARTUP_CHECKING, BOOLEAN, true ); this.procedureParameterNullPassingEnabled = cfgService.getSetting( PROCEDURE_NULL_PARAM_PASSING, BOOLEAN, false ); + this.collectionJoinSubqueryRewriteEnabled = cfgService.getSetting( COLLECTION_JOIN_SUBQUERY, BOOLEAN, true ); this.secondLevelCacheEnabled = cfgService.getSetting( USE_SECOND_LEVEL_CACHE, BOOLEAN, true ); this.queryCacheEnabled = cfgService.getSetting( USE_QUERY_CACHE, BOOLEAN, false ); @@ -1086,6 +1040,11 @@ public class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement return procedureParameterNullPassingEnabled; } + @Override + public boolean isCollectionJoinSubqueryRewriteEnabled() { + return collectionJoinSubqueryRewriteEnabled; + } + @Override public boolean isSecondLevelCacheEnabled() { return secondLevelCacheEnabled; @@ -1391,6 +1350,11 @@ public class SessionFactoryBuilderImpl implements SessionFactoryBuilderImplement return options.isProcedureParameterNullPassingEnabled(); } + @Override + public boolean isCollectionJoinSubqueryRewriteEnabled() { + return options.isCollectionJoinSubqueryRewriteEnabled(); + } + @Override public boolean isSecondLevelCacheEnabled() { return options.isSecondLevelCacheEnabled(); 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 index a177d29fe3..40090b6ecf 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsImpl.java @@ -96,6 +96,7 @@ public class SessionFactoryOptionsImpl implements SessionFactoryOptions { private final boolean strictJpaQueryLanguageCompliance; private final boolean namedQueryStartupCheckingEnabled; private final boolean procedureParameterNullPassingEnabled; + private final boolean collectionJoinSubqueryRewriteEnabled; // Caching private final boolean secondLevelCacheEnabled; @@ -173,6 +174,7 @@ public class SessionFactoryOptionsImpl implements SessionFactoryOptions { this.strictJpaQueryLanguageCompliance = state.isStrictJpaQueryLanguageCompliance(); this.namedQueryStartupCheckingEnabled = state.isNamedQueryStartupCheckingEnabled(); this.procedureParameterNullPassingEnabled = state.isProcedureParameterNullPassingEnabled(); + this.collectionJoinSubqueryRewriteEnabled = state.isCollectionJoinSubqueryRewriteEnabled(); this.secondLevelCacheEnabled = state.isSecondLevelCacheEnabled(); this.queryCacheEnabled = state.isQueryCacheEnabled(); @@ -372,6 +374,11 @@ public class SessionFactoryOptionsImpl implements SessionFactoryOptions { return procedureParameterNullPassingEnabled; } + @Override + public boolean isCollectionJoinSubqueryRewriteEnabled() { + return collectionJoinSubqueryRewriteEnabled; + } + @Override public boolean isAllowOutOfTransactionUpdateOperations() { return allowOutOfTransactionUpdateOperations; 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 index c6737fcf97..5a935d3457 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsState.java @@ -117,6 +117,8 @@ public interface SessionFactoryOptionsState { boolean isProcedureParameterNullPassingEnabled(); + boolean isCollectionJoinSubqueryRewriteEnabled(); + boolean isSecondLevelCacheEnabled(); boolean isQueryCacheEnabled(); diff --git a/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java b/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java index 10681fb5ac..b11972146f 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/spi/AbstractDelegatingSessionFactoryOptions.java @@ -214,6 +214,11 @@ public class AbstractDelegatingSessionFactoryOptions implements SessionFactoryOp return delegate.isProcedureParameterNullPassingEnabled(); } + @Override + public boolean isCollectionJoinSubqueryRewriteEnabled() { + return delegate.isCollectionJoinSubqueryRewriteEnabled(); + } + @Override public boolean isAllowOutOfTransactionUpdateOperations() { return delegate.isAllowOutOfTransactionUpdateOperations(); 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 index d3edf91458..d5d648fe7d 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java @@ -202,6 +202,8 @@ public interface SessionFactoryOptions { boolean isProcedureParameterNullPassingEnabled(); + boolean isCollectionJoinSubqueryRewriteEnabled(); + boolean isAllowOutOfTransactionUpdateOperations(); boolean isReleaseResourcesOnCloseEnabled(); diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java index f4b3b7d175..fcb87fa2e0 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java @@ -1548,4 +1548,14 @@ public interface AvailableSettings { * @since 5.2 */ String ALLOW_UPDATE_OUTSIDE_TRANSACTION = "hibernate.allow_update_outside_transaction"; + + /** + * Setting which indicates whether or not the new JOINS over collection tables should be rewritten to subqueries. + *

+ * Default is {@code true}. Existing applications may want to disable this (set it {@code false}) for + * upgrade compatibility. + * + * @since 5.2 + */ + String COLLECTION_JOIN_SUBQUERY = "hibernate.collection_join_subquery"; } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java index a81a89bcdf..acb93361ad 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java @@ -34,6 +34,7 @@ import org.hibernate.type.AssociationType; */ public class JoinSequence { private final SessionFactoryImplementor factory; + private final boolean collectionJoinSubquery; private final StringBuilder conditions = new StringBuilder(); private final List joins = new ArrayList(); @@ -52,6 +53,7 @@ public class JoinSequence { */ public JoinSequence(SessionFactoryImplementor factory) { this.factory = factory; + this.collectionJoinSubquery = factory.getSessionFactoryOptions().isCollectionJoinSubqueryRewriteEnabled(); } /** @@ -192,7 +194,8 @@ public class JoinSequence { last = rootJoinable; } else if ( - withClauseFragment != null + collectionJoinSubquery + && withClauseFragment != null && joins.size() > 1 && withClauseFragment.contains( withClauseJoinAlias ) && ( first = ( iter = joins.iterator() ).next() ).joinType == JoinType.LEFT_OUTER_JOIN @@ -208,6 +211,7 @@ public class JoinSequence { "([a-zA-Z0-9_]+) | " + // Normal identifiers "('[a-zA-Z0-9_]+' ((''[a-zA-Z0-9_]+)+')?) | " + // Single quoted identifiers "(\"[a-zA-Z0-9_]+\" ((\"\"[a-zA-Z0-9_]+)+\")?) | " + // Double quoted identifiers + "(`[a-zA-Z0-9_]+` ((``[a-zA-Z0-9_]+)+`)?) | " + // MySQL quoted identifiers "(\\[[a-zA-Z0-9_\\s]+\\])" + // MSSQL quoted identifiers ")" ); diff --git a/hibernate-core/src/test/java/org/hibernate/test/hql/WithClauseTest.java b/hibernate-core/src/test/java/org/hibernate/test/hql/WithClauseTest.java index 3e93b32667..c4942ba234 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/hql/WithClauseTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/hql/WithClauseTest.java @@ -7,6 +7,7 @@ package org.hibernate.test.hql; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -15,6 +16,7 @@ import org.hibernate.Query; import org.hibernate.QueryException; import org.hibernate.Session; import org.hibernate.Transaction; +import org.hibernate.cfg.AvailableSettings; import org.hibernate.hql.internal.ast.InvalidWithClauseException; import org.hibernate.testing.TestForIssue; @@ -220,6 +222,29 @@ public class WithClauseTest extends BaseCoreFunctionalTestCase { data.cleanup(); } + @Test + @TestForIssue(jiraKey = "HHH-11157") + public void testWithClauseAsNonSubqueryWithKey() { + rebuildSessionFactory( Collections.singletonMap( AvailableSettings.COLLECTION_JOIN_SUBQUERY, "false" ) ); + + TestData data = new TestData(); + data.prepare(); + + Session s = openSession(); + Transaction txn = s.beginTransaction(); + + // Since family has a join table, we will first left join all family members and then do the WITH clause on the target entity table join + // Normally this produces 2 results which is wrong and can only be circumvented by converting the join table and target entity table join to a subquery + List list = s.createQuery("from Human h left join h.family as f with key(f) like 'son1' where h.description = 'father'") + .list(); + assertEquals("subquery rewriting of join table was not disabled", 2, list.size()); + + txn.commit(); + s.close(); + + data.cleanup(); + } + private class TestData { public void prepare() { Session session = openSession(); diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/junit4/BaseCoreFunctionalTestCase.java b/hibernate-testing/src/main/java/org/hibernate/testing/junit4/BaseCoreFunctionalTestCase.java index c3a40997b0..64ec85f26c 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/junit4/BaseCoreFunctionalTestCase.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/junit4/BaseCoreFunctionalTestCase.java @@ -9,10 +9,7 @@ package org.hibernate.testing.junit4; import java.io.InputStream; import java.sql.Connection; import java.sql.SQLException; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Properties; +import java.util.*; import javax.persistence.SharedCacheMode; import org.hibernate.HibernateException; @@ -98,8 +95,17 @@ public abstract class BaseCoreFunctionalTestCase extends BaseUnitTestCase { @BeforeClassOnce @SuppressWarnings( {"UnusedDeclaration"}) protected void buildSessionFactory() { + buildSessionFactory( Collections.emptyMap() ); + } + + protected void buildSessionFactory(Map properties) { // for now, build the configuration to get all the property settings configuration = constructAndConfigureConfiguration(); + if ( properties != null && !properties.isEmpty() ) { + for ( Map.Entry entry : properties.entrySet() ) { + configuration.setProperty( entry.getKey(), entry.getValue() ); + } + } BootstrapServiceRegistry bootRegistry = buildBootstrapServiceRegistry(); serviceRegistry = buildServiceRegistry( bootRegistry, configuration ); // this is done here because Configuration does not currently support 4.0 xsd @@ -109,6 +115,10 @@ public abstract class BaseCoreFunctionalTestCase extends BaseUnitTestCase { } protected void rebuildSessionFactory() { + rebuildSessionFactory( Collections.emptyMap() ); + } + + protected void rebuildSessionFactory(Map properties) { if ( sessionFactory == null ) { return; } @@ -122,7 +132,7 @@ public abstract class BaseCoreFunctionalTestCase extends BaseUnitTestCase { catch (Exception ignore) { } - buildSessionFactory(); + buildSessionFactory( properties ); } protected Configuration buildConfiguration() {