HHH-14153 refactoring after HHH-14153

- avoid use package-scoped instance felds
- remove copy/pasted code

Note: I don't hate package-scoped final fields, but I
know other folks tend to. So let's go with template
methods instead.

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/BasicExecutor.java

@@ -26,24 +26,20 @@ import org.hibernate.persister.entity.Queryable;
  */
 public abstract class BasicExecutor implements StatementExecutor {
 
-	final Queryable persister;
+	abstract Queryable getPersister();
-	String sql;
+	abstract String getSql();
-	List<ParameterSpecification> parameterSpecifications;
+	abstract List<ParameterSpecification> getParameterSpecifications();
-
-	public BasicExecutor(Queryable persister) {
-		this.persister = persister;
-	}
 
 	@Override
 	public String[] getSqlStatements() {
-		return new String[] { sql };
+		return new String[] { getSql() };
 	}
 
 	@Override
 	public int execute(QueryParameters parameters, SharedSessionContractImplementor session)
 			throws HibernateException {
 
-		BulkOperationCleanupAction action = new BulkOperationCleanupAction( session, persister );
+		BulkOperationCleanupAction action = new BulkOperationCleanupAction( session, getPersister() );
 		if ( session.isEventSource() ) {
 			( (EventSource) session).getActionQueue().addAction( action );
 		}
@@ -54,12 +50,12 @@ public abstract class BasicExecutor implements StatementExecutor {
 		return doExecute(
 				session.getJdbcServices().getDialect()
 						.addSqlHintOrComment(
-								sql,
+								getSql(),
 								parameters,
 								session.getFactory().getSessionFactoryOptions().isCommentsEnabled()
 						),
 				parameters,
-				parameterSpecifications,
+				getParameterSpecifications(),
 				session
 		);
 	}

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/DeleteExecutor.java

@@ -21,8 +21,10 @@ import org.hibernate.hql.internal.ast.QuerySyntaxException;
 import org.hibernate.hql.internal.ast.SqlGenerator;
 import org.hibernate.hql.internal.ast.tree.DeleteStatement;
 import org.hibernate.metamodel.spi.MetamodelImplementor;
+import org.hibernate.param.ParameterSpecification;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.persister.entity.Joinable;
+import org.hibernate.persister.entity.Queryable;
 import org.hibernate.sql.Delete;
 import org.hibernate.type.CollectionType;
 import org.hibernate.type.ComponentType;
@@ -45,10 +47,29 @@ import antlr.collections.AST;
 public class DeleteExecutor extends BasicExecutor {
 	private static final Logger LOG = Logger.getLogger( DeleteExecutor.class );
 
+	private final String sql;
+	private final List<ParameterSpecification> parameterSpecifications;
+	private final Queryable persister;
+
 	private final List<String> deletes = new ArrayList<>();
 
+	@Override
+	Queryable getPersister() {
+		return persister;
+	}
+
+	@Override
+	public String getSql() {
+		return sql;
+	}
+
+	@Override
+	public List<ParameterSpecification> getParameterSpecifications() {
+		return parameterSpecifications;
+	}
+
 	public DeleteExecutor(HqlSqlWalker walker) {
-		super( walker.getFinalFromClause().getFromElement().getQueryable() );
+		persister = walker.getFinalFromClause().getFromElement().getQueryable();
 
 		final SessionFactoryImplementor factory = walker.getSessionFactoryHelper().getFactory();
 
@@ -81,8 +102,7 @@ public class DeleteExecutor extends BasicExecutor {
 
 			final boolean commentsEnabled = factory.getSessionFactoryOptions().isCommentsEnabled();
 			final MetamodelImplementor metamodel = factory.getMetamodel();
-			final Dialect dialect = factory.getJdbcServices().getJdbcEnvironment().getDialect();
+			final boolean notSupportingTuplesInSubqueries = !walker.getDialect().supportsTuplesInSubqueries();
-			final boolean notSupportingTuplesInSubqueries = !dialect.supportsTuplesInSubqueries();
 			// If many-to-many, delete the FK row in the collection table.
 			for ( Type type : persister.getPropertyTypes() ) {
 				if ( type.isCollectionType() ) {

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/IdSubselectUpdateExecutor.java

@@ -16,14 +16,15 @@ import org.hibernate.hql.internal.ast.QuerySyntaxException;
 import org.hibernate.hql.internal.ast.SqlGenerator;
 import org.hibernate.hql.internal.ast.tree.AssignmentSpecification;
 import org.hibernate.hql.internal.ast.tree.UpdateStatement;
+import org.hibernate.param.ParameterSpecification;
 import org.hibernate.persister.entity.Queryable;
-import org.hibernate.sql.Select;
-import org.hibernate.sql.SelectValues;
 import org.hibernate.sql.Update;
 
 import java.util.List;
 import java.util.stream.IntStream;
 
+import static org.hibernate.hql.spi.id.AbstractTableBasedBulkIdHandler.generateIdSelect;
+
 /**
  * Executes HQL bulk updates against a single table, using a subselect
  * against multiple tables to collect ids, which is needed when the
@@ -33,8 +34,25 @@ import java.util.stream.IntStream;
  */
 public class IdSubselectUpdateExecutor extends BasicExecutor {
 
+	private final Queryable persister;
+	private final String sql;
+	private final List<ParameterSpecification> parameterSpecifications;
+
+	public Queryable getPersister() {
+		return persister;
+	}
+	@Override
+	public String getSql() {
+		return sql;
+	}
+
+	@Override
+	public List<ParameterSpecification> getParameterSpecifications() {
+		return parameterSpecifications;
+	}
+
 	public IdSubselectUpdateExecutor(HqlSqlWalker walker) {
-		super( walker.getFinalFromClause().getFromElement().getQueryable() );
+		persister = walker.getFinalFromClause().getFromElement().getQueryable();
 
 		Dialect dialect = walker.getDialect();
 		UpdateStatement updateStatement = (UpdateStatement) walker.getAST();
@@ -98,41 +116,5 @@ public class IdSubselectUpdateExecutor extends BasicExecutor {
 		catch ( RecognitionException e ) {
 			throw QuerySyntaxException.convert( e );
 		}
-
-	}
-
-	//TODO: this is a copy/paste of a method from AbstractTableBasedBulkIdHandler
-	private String generateIdSelect(String tableAlias, String whereClause, Dialect dialect, Queryable queryable) {
-		Select select = new Select( dialect );
-		SelectValues selectClause = new SelectValues( dialect ).addColumns(
-				tableAlias,
-				queryable.getIdentifierColumnNames(),
-				queryable.getIdentifierColumnNames()
-		);
-		select.setSelectClause( selectClause.render() );
-
-		String rootTableName = queryable.getTableName();
-		String fromJoinFragment = queryable.fromJoinFragment( tableAlias, true, false );
-		String whereJoinFragment = queryable.whereJoinFragment( tableAlias, true, false );
-
-		select.setFromClause( rootTableName + ' ' + tableAlias + fromJoinFragment );
-
-		if ( whereJoinFragment == null ) {
-			whereJoinFragment = "";
-		}
-		else {
-			whereJoinFragment = whereJoinFragment.trim();
-			if ( whereJoinFragment.startsWith( "and" ) ) {
-				whereJoinFragment = whereJoinFragment.substring( 4 );
-			}
-		}
-
-		if ( !whereClause.isEmpty() ) {
-			if ( whereJoinFragment.length() > 0 ) {
-				whereJoinFragment += " and ";
-			}
-		}
-		select.setWhereClause( whereJoinFragment + whereClause );
-		return select.toStatementString();
 	}
 }

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/InsertExecutor.java

@@ -24,8 +24,27 @@ import java.util.List;
  * @author Gavin King
  */
 public class InsertExecutor extends BasicExecutor {
+	private final Queryable persister;
+	private final String sql;
+	private final List<ParameterSpecification> parameterSpecifications;
+
+	@Override
+	public Queryable getPersister() {
+		return persister;
+	}
+
+	@Override
+	public String getSql() {
+		return sql;
+	}
+
+	@Override
+	public List<ParameterSpecification> getParameterSpecifications() {
+		return parameterSpecifications;
+	}
+
 	public InsertExecutor(HqlSqlWalker walker) {
-		super( ( (InsertStatement) walker.getAST() ).getIntoClause().getQueryable() );
+		persister = ( (InsertStatement) walker.getAST() ).getIntoClause().getQueryable();
 		try {
 			SqlGenerator gen = new SqlGenerator( walker.getSessionFactoryHelper().getFactory() );
 			gen.statement( walker.getAST() );

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/exec/SimpleUpdateExecutor.java

@@ -11,6 +11,10 @@ import org.hibernate.AssertionFailure;
 import org.hibernate.hql.internal.ast.HqlSqlWalker;
 import org.hibernate.hql.internal.ast.QuerySyntaxException;
 import org.hibernate.hql.internal.ast.SqlGenerator;
+import org.hibernate.param.ParameterSpecification;
+import org.hibernate.persister.entity.Queryable;
+
+import java.util.List;
 
 /**
  * Executes HQL bulk updates against a single table, where the
@@ -20,8 +24,28 @@ import org.hibernate.hql.internal.ast.SqlGenerator;
  * @author Gavin King
  */
 public class SimpleUpdateExecutor extends BasicExecutor {
+
+	private final Queryable persister;
+	private final String sql;
+	private final List<ParameterSpecification> parameterSpecifications;
+
+	@Override
+	public Queryable getPersister() {
+		return persister;
+	}
+
+	@Override
+	public String getSql() {
+		return sql;
+	}
+
+	@Override
+	public List<ParameterSpecification> getParameterSpecifications() {
+		return parameterSpecifications;
+	}
+
 	public SimpleUpdateExecutor(HqlSqlWalker walker) {
-		super( walker.getFinalFromClause().getFromElement().getQueryable() );
+		persister = walker.getFinalFromClause().getFromElement().getQueryable();
 
 		if ( persister.isMultiTable() && walker.getQuerySpaces().size() > 1 ) {
 			throw new AssertionFailure("not a simple update");

hibernate-core/src/main/java/org/hibernate/hql/spi/id/AbstractTableBasedBulkIdHandler.java

@@ -9,6 +9,7 @@ package org.hibernate.hql.spi.id;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.function.Consumer;
 
 import org.hibernate.HibernateException;
 import org.hibernate.dialect.Dialect;
@@ -16,6 +17,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.hql.internal.ast.HqlSqlWalker;
 import org.hibernate.hql.internal.ast.SqlGenerator;
+import org.hibernate.internal.util.StringHelper;
 import org.hibernate.param.ParameterSpecification;
 import org.hibernate.persister.collection.AbstractCollectionPersister;
 import org.hibernate.persister.entity.Queryable;
@@ -93,7 +95,6 @@ public abstract class AbstractTableBasedBulkIdHandler {
 	 *
 	 * @return The bulk-id-ready {@code WHERE} clause representation
 	 */
-	@SuppressWarnings("unchecked")
 	protected ProcessedWhereClause processWhereClause(AST whereClause) {
 		if ( whereClause.getNumberOfChildren() != 0 ) {
 			// If a where clause was specified in the update/delete query, use it to limit the
@@ -130,10 +131,9 @@ public abstract class AbstractTableBasedBulkIdHandler {
 			IdTableInfo idTableInfo,
 			ProcessedWhereClause whereClause) {
 
-		final Dialect dialect = sessionFactory.getJdbcServices().getJdbcEnvironment().getDialect();
 		final Select select = generateIdSelect( tableAlias, whereClause );
 
-		InsertSelect insert = new InsertSelect( dialect );
+		InsertSelect insert = new InsertSelect( walker.getDialect() );
 		if ( sessionFactory.getSessionFactoryOptions().isCommentsEnabled() ) {
 			insert.setComment( "insert-select for " + getTargetedQueryable().getEntityName() + " ids" );
 		}
@@ -142,24 +142,50 @@ public abstract class AbstractTableBasedBulkIdHandler {
 		return insert.toStatementString();
 	}
 
-	protected Select generateIdSelect(
+	Select generateIdSelect(String tableAlias, ProcessedWhereClause whereClause) {
-			String tableAlias,
+		return generateIdSelect(
-			ProcessedWhereClause whereClause) {
+				tableAlias,
+				whereClause.getUserWhereClauseFragment(),
+				walker.getDialect(),
+				getTargetedQueryable(),
+				this::addAnyExtraIdSelectValues
+		);
+	}
 
-		final Dialect dialect = sessionFactory.getJdbcServices().getJdbcEnvironment().getDialect();
+	public static String generateIdSelect(
+			String tableAlias,
+			String whereClause,
+			Dialect dialect,
+			Queryable queryable) {
+		return generateIdSelect(
+				tableAlias,
+				whereClause,
+				dialect,
+				queryable,
+				selectValues -> {}
+		)
+				.toStatementString();
+	}
+
+	private static Select generateIdSelect(
+			String tableAlias,
+			String whereClause,
+			Dialect dialect,
+			Queryable queryable,
+			Consumer<SelectValues> addAnyExtraIdSelectValues) {
 
 		final Select select = new Select(dialect);
 		final SelectValues selectClause = new SelectValues(dialect).addColumns(
 				tableAlias,
-				getTargetedQueryable().getIdentifierColumnNames(),
+				queryable.getIdentifierColumnNames(),
-				getTargetedQueryable().getIdentifierColumnNames()
+				queryable.getIdentifierColumnNames()
 		);
-		addAnyExtraIdSelectValues( selectClause );
+		addAnyExtraIdSelectValues.accept( selectClause );
 		select.setSelectClause( selectClause.render() );
 
-		String rootTableName = getTargetedQueryable().getTableName();
+		String rootTableName = queryable.getTableName();
-		String fromJoinFragment = getTargetedQueryable().fromJoinFragment( tableAlias, true, false );
+		String fromJoinFragment = queryable.fromJoinFragment(tableAlias, true, false );
-		String whereJoinFragment = getTargetedQueryable().whereJoinFragment( tableAlias, true, false );
+		String whereJoinFragment = queryable.whereJoinFragment(tableAlias, true, false );
 
 		select.setFromClause( rootTableName + ' ' + tableAlias + fromJoinFragment );
 
@@ -173,12 +199,10 @@ public abstract class AbstractTableBasedBulkIdHandler {
 			}
 		}
 
-		if ( whereClause.getUserWhereClauseFragment().length() > 0 ) {
+		if ( !whereClause.isEmpty() && !whereJoinFragment.isEmpty() ) {
-			if ( whereJoinFragment.length() > 0 ) {
 			whereJoinFragment += " and ";
 		}
-		}
+		select.setWhereClause( whereJoinFragment + whereClause);
-		select.setWhereClause( whereJoinFragment + whereClause.getUserWhereClauseFragment() );
 		return select;
 	}
 
@@ -215,7 +239,7 @@ public abstract class AbstractTableBasedBulkIdHandler {
 			for ( String propertyName : componentType.getPropertyNames() ) {
 				Collections.addAll( columns, persister.toColumns( propertyName ) );
 			}
-			columnNames = columns.toArray( new String[columns.size()] );
+			columnNames = columns.toArray( StringHelper.EMPTY_STRINGS );
 		}
 		else {
 			columnNames = persister.getIdentifierColumnNames();