Permalink
Browse files

HHH-4959 - Concurrent HQL parsing blocks on ReflectHelper.classForName()

Exclude JPQL and Criteria API aliases when searching for a Java constant value
  • Loading branch information...
1 parent 159bc99 commit 0bd7b8eac16e6966dea81b8a5991e052192a6af2 @vladmihalcea vladmihalcea committed Dec 14, 2016
@@ -349,6 +349,15 @@ implementations that sacrifices performance optimizations to allow legacy 4.x li
Legacy 4.x behavior favored performing pagination in-memory by avoiding the use of the offset value, which is overall poor performance.
In 5.x, the limit handler behavior favors performance, thus, if the dialect doesn't support offsets, an exception is thrown instead.
+|`hibernate.query.conventional_java_constants` | `true` (default value) or `false` |
+Setting which indicates whether or not Java constant follow the https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html[Java Naming conventions].
+
+Default is `true`.
+Existing applications may want to disable this (set it `false`) if non-conventional Java constants are used.
+However, there is a significant performance overhead for using non-conventional Java constants
+since Hibernate cannot determine if aliases should be treated as Java constants or not.
+
+Check out https://hibernate.atlassian.net/browse/HHH-4959[HHH-4959] for more details.
|===================================================================================================================================================================================================================================
[[configurations-batch]]
@@ -528,6 +528,7 @@ public SessionFactoryOptions buildSessionFactoryOptions() {
private Map querySubstitutions;
private boolean strictJpaQueryLanguageCompliance;
private boolean namedQueryStartupCheckingEnabled;
+ private boolean conventionalJavaConstants;
private final boolean procedureParameterNullPassingEnabled;
private final boolean collectionJoinSubqueryRewriteEnabled;
@@ -654,6 +655,8 @@ public SessionFactoryOptionsStateStandardImpl(StandardServiceRegistry serviceReg
this.querySubstitutions = ConfigurationHelper.toMap( QUERY_SUBSTITUTIONS, " ,=;:\n\t\r\f", configurationSettings );
this.strictJpaQueryLanguageCompliance = cfgService.getSetting( JPAQL_STRICT_COMPLIANCE, BOOLEAN, false );
this.namedQueryStartupCheckingEnabled = cfgService.getSetting( QUERY_STARTUP_CHECKING, BOOLEAN, true );
+ this.conventionalJavaConstants = cfgService.getSetting(
+ CONVENTIONAL_JAVA_CONSTANTS, BOOLEAN, true );
this.procedureParameterNullPassingEnabled = cfgService.getSetting( PROCEDURE_NULL_PARAM_PASSING, BOOLEAN, false );
this.collectionJoinSubqueryRewriteEnabled = cfgService.getSetting( COLLECTION_JOIN_SUBQUERY, BOOLEAN, true );
@@ -1051,6 +1054,10 @@ public boolean isNamedQueryStartupCheckingEnabled() {
return namedQueryStartupCheckingEnabled;
}
+ public boolean isConventionalJavaConstants() {
+ return conventionalJavaConstants;
+ }
+
@Override
public boolean isProcedureParameterNullPassingEnabled() {
return procedureParameterNullPassingEnabled;
@@ -1365,6 +1372,10 @@ public boolean isNamedQueryStartupCheckingEnabled() {
return options.isNamedQueryStartupCheckingEnabled();
}
+ public boolean isConventionalJavaConstants() {
+ return options.isConventionalJavaConstants();
+ }
+
@Override
public boolean isProcedureParameterNullPassingEnabled() {
return options.isProcedureParameterNullPassingEnabled();
@@ -96,6 +96,7 @@
private final Map querySubstitutions;
private final boolean strictJpaQueryLanguageCompliance;
private final boolean namedQueryStartupCheckingEnabled;
+ private final boolean conventionalJavaConstants;
private final boolean procedureParameterNullPassingEnabled;
private final boolean collectionJoinSubqueryRewriteEnabled;
@@ -175,6 +176,7 @@ public SessionFactoryOptionsImpl(SessionFactoryOptionsState state) {
this.querySubstitutions = state.getQuerySubstitutions();
this.strictJpaQueryLanguageCompliance = state.isStrictJpaQueryLanguageCompliance();
this.namedQueryStartupCheckingEnabled = state.isNamedQueryStartupCheckingEnabled();
+ this.conventionalJavaConstants = state.isConventionalJavaConstants();
this.procedureParameterNullPassingEnabled = state.isProcedureParameterNullPassingEnabled();
this.collectionJoinSubqueryRewriteEnabled = state.isCollectionJoinSubqueryRewriteEnabled();
@@ -377,6 +379,11 @@ public boolean isNamedQueryStartupCheckingEnabled() {
}
@Override
+ public boolean isConventionalJavaConstants() {
+ return conventionalJavaConstants;
+ }
+
+ @Override
public boolean isProcedureParameterNullPassingEnabled() {
return procedureParameterNullPassingEnabled;
}
@@ -117,6 +117,8 @@
boolean isNamedQueryStartupCheckingEnabled();
+ boolean isConventionalJavaConstants();
+
boolean isProcedureParameterNullPassingEnabled();
boolean isCollectionJoinSubqueryRewriteEnabled();
@@ -215,6 +215,11 @@ public boolean isNamedQueryStartupCheckingEnabled() {
}
@Override
+ public boolean isConventionalJavaConstants() {
+ return delegate.isConventionalJavaConstants();
+ }
+
+ @Override
public boolean isProcedureParameterNullPassingEnabled() {
return delegate.isProcedureParameterNullPassingEnabled();
}
@@ -147,6 +147,8 @@ default boolean isAllowRefreshDetachedEntity() {
boolean isNamedQueryStartupCheckingEnabled();
+ boolean isConventionalJavaConstants();
+
boolean isSecondLevelCacheEnabled();
boolean isQueryCacheEnabled();
@@ -802,6 +802,17 @@
String QUERY_STARTUP_CHECKING = "hibernate.query.startup_check";
/**
+ * Setting which indicates whether or not Java constant follow the Java Naming conventions.
+ * <p/>
+ * Default is {@code true}. Existing applications may want to disable this (set it {@code false}) if non-conventional Java constants are used.
+ * However, there is a significant performance overhead for using non-conventional Java constants since Hibernate cannot determine if aliases
+ * should be treated as Java constants or not.
+ *
+ * @since 5.2
+ */
+ String CONVENTIONAL_JAVA_CONSTANTS = "hibernate.query.conventional_java_constants";
+
+ /**
* The {@link org.hibernate.exception.spi.SQLExceptionConverter} to use for converting SQLExceptions
* to Hibernate's JDBCException hierarchy. The default is to use the configured
* {@link org.hibernate.dialect.Dialect}'s preferred SQLExceptionConverter.
@@ -18,7 +18,6 @@
import org.hibernate.HibernateException;
import org.hibernate.MappingException;
import org.hibernate.QueryException;
-import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
import org.hibernate.engine.query.spi.EntityGraphQueryHint;
import org.hibernate.engine.spi.QueryParameters;
import org.hibernate.engine.spi.RowSelection;
@@ -590,7 +589,6 @@ public Class getDynamicInstantiationResultType() {
private AST dotRoot;
public JavaConstantConverter(SessionFactoryImplementor factory) {
-
this.factory = factory;
}
@@ -612,7 +610,7 @@ public void visit(AST node) {
}
private void handleDotStructure(AST dotStructureRoot) {
final String expression = ASTUtil.getPathText( dotStructureRoot );
- final Object constant = ReflectHelper.getConstantValue( expression, factory.getServiceRegistry().getService( ClassLoaderService.class ) );
+ final Object constant = ReflectHelper.getConstantValue( expression, factory );
if ( constant != null ) {
dotStructureRoot.setFirstChild( null );
dotStructureRoot.setType( HqlTokenTypes.JAVA_CONSTANT );
@@ -9,7 +9,6 @@
import java.util.Locale;
import org.hibernate.QueryException;
-import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
import org.hibernate.dialect.Dialect;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.hql.spi.QueryTranslator;
@@ -39,7 +38,7 @@ public void setText(String s) {
// this method to get called twice. The first time with an empty string
if ( StringHelper.isNotEmpty( s ) ) {
constantExpression = s;
- constantValue = ReflectHelper.getConstantValue( s, factory.getServiceRegistry().getService( ClassLoaderService.class ) );
+ constantValue = ReflectHelper.getConstantValue( s, factory );
heuristicType = factory.getTypeResolver().heuristicType( constantValue.getClass().getName() );
super.setText( s );
}
@@ -13,13 +13,11 @@
import org.hibernate.HibernateException;
import org.hibernate.MappingException;
import org.hibernate.QueryException;
-import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
import org.hibernate.dialect.Dialect;
import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes;
import org.hibernate.hql.internal.antlr.SqlTokenTypes;
import org.hibernate.hql.internal.ast.HqlSqlWalker;
import org.hibernate.hql.internal.ast.InvalidPathException;
-import org.hibernate.hql.internal.ast.tree.BooleanLiteralNode;
import org.hibernate.hql.internal.ast.tree.DotNode;
import org.hibernate.hql.internal.ast.tree.FromClause;
import org.hibernate.hql.internal.ast.tree.IdentNode;
@@ -35,7 +33,6 @@
import antlr.SemanticException;
import antlr.collections.AST;
-import java.util.Locale;
/**
* A delegate that handles literals and constants for HqlSqlWalker, performing the token replacement functions and
@@ -109,7 +106,7 @@ public void lookupConstant(DotNode node) throws SemanticException {
setSQLValue( node, text, discrim );
}
else {
- Object value = ReflectHelper.getConstantValue( text, walker.getSessionFactoryHelper().getFactory().getServiceRegistry().getService( ClassLoaderService.class ) );
+ Object value = ReflectHelper.getConstantValue( text, walker.getSessionFactoryHelper().getFactory() );
if ( value == null ) {
throw new InvalidPathException( "Invalid path: '" + text + "'" );
}
@@ -16,7 +16,6 @@
import org.hibernate.MappingException;
import org.hibernate.QueryException;
-import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
import org.hibernate.engine.internal.JoinSequence;
import org.hibernate.hql.spi.QueryTranslator;
import org.hibernate.internal.util.ReflectHelper;
@@ -419,7 +418,7 @@ else if ( token.startsWith( ParserHelper.HQL_VARIABLE_PREFIX ) ) { //named query
Object constant;
if (
token.indexOf( '.' ) > -1 &&
- ( constant = ReflectHelper.getConstantValue( token, q.getFactory().getServiceRegistry().getService( ClassLoaderService.class ) ) ) != null
+ ( constant = ReflectHelper.getConstantValue( token, q.getFactory() ) ) != null
) {
Type type;
try {
@@ -13,12 +13,14 @@
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Locale;
+import java.util.regex.Pattern;
import org.hibernate.AssertionFailure;
import org.hibernate.MappingException;
import org.hibernate.PropertyNotFoundException;
import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
import org.hibernate.boot.registry.classloading.spi.ClassLoadingException;
+import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.property.access.internal.PropertyAccessStrategyMixedImpl;
import org.hibernate.property.access.spi.Getter;
import org.hibernate.type.PrimitiveType;
@@ -32,6 +34,10 @@
*/
@SuppressWarnings("unchecked")
public final class ReflectHelper {
+
+ private static final Pattern JAVA_CONSTANT_PATTERN = Pattern.compile(
+ "[a-z]+\\.([A-Z]{1}[a-z]+)+\\$?([A-Z]{1}[a-z]+)*\\.[A-Z_\\$]+" );
@stonio
stonio Dec 19, 2016 edited

Is there a reason to ignore digit characters (and underscore character)? They may be found in class name or constant name.

Reference : http://www.oracle.com/technetwork/java/codeconventions-135099.html

@vladmihalcea
vladmihalcea via email Dec 19, 2016 Member
+
public static final Class[] NO_PARAM_SIGNATURE = new Class[0];
public static final Object[] NO_PARAMS = new Object[0];
@@ -229,9 +235,15 @@ private static Getter getter(Class clazz, String name) throws MappingException {
return PropertyAccessStrategyMixedImpl.INSTANCE.buildPropertyAccess( clazz, name ).getGetter();
}
- public static Object getConstantValue(String name, ClassLoaderService classLoaderService) {
+ public static Object getConstantValue(String name, SessionFactoryImplementor factory) {
+ boolean conventionalJavaConstants = factory.getSessionFactoryOptions().isConventionalJavaConstants();
Class clazz;
try {
+ if ( conventionalJavaConstants &&
+ !JAVA_CONSTANT_PATTERN.matcher( name ).find() ) {
+ return null;
+ }
+ ClassLoaderService classLoaderService = factory.getServiceRegistry().getService( ClassLoaderService.class );
clazz = classLoaderService.classForName( StringHelper.qualifier( name ) );
}
catch ( Throwable t ) {
@@ -331,7 +343,7 @@ public static Constructor getConstructor(Class clazz, Type[] types) throws Prope
throw new PropertyNotFoundException( "no appropriate constructor in class: " + clazz.getName() );
}
-
+
public static Method getMethod(Class clazz, Method method) {
try {
return clazz.getMethod( method.getName(), method.getParameterTypes() );
Oops, something went wrong.

0 comments on commit 0bd7b8e

Please sign in to comment.