HHH-16260 - JdbcParameterRenderer not called with dynamic filters

HHH-16256 - JdbcParameterRenderer to have an impact on write operations

hibernate-core/src/main/java/org/hibernate/internal/util/collections/CollectionHelper.java

@@ -489,6 +489,14 @@ public final class CollectionHelper {
 		return values == null ? 0 : values.size();
 	}
 
+	public static int size(Collection<?> values) {
+		return values == null ? 0 : values.size();
+	}
+
+	public static int size(Map<?,?> values) {
+		return values == null ? 0 : values.size();
+	}
+
 	public static <X> Set<X> toSet(X... values) {
 		final HashSet<X> result = new HashSet<>();
 		if ( isNotEmpty( values ) ) {

hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java

@@ -8012,7 +8012,39 @@ public abstract class AbstractSqlAstTranslator<T extends JdbcOperation> implemen
 
 	@Override
 	public void visitColumnWriteFragment(ColumnWriteFragment columnWriteFragment) {
-		sqlBuffer.append( columnWriteFragment.getFragment() );
+		// if there are no parameters or if we are using the standard parameter renderer
+		//		- the rendering is pretty simple
+		if ( CollectionHelper.isEmpty( columnWriteFragment.getParameters() )
+				|| JdbcParameterRenderer.isStandardRenderer( jdbcParameterRenderer ) ) {
+			simpleColumnWriteFragmentRendering( columnWriteFragment );
+			return;
+		}
+
+		// otherwise, render the fragment using the custom parameter renderer
+		final String sqlFragment = columnWriteFragment.getFragment();
+		int lastEnd = 0;
+
+		for ( ColumnValueParameter parameter : columnWriteFragment.getParameters() ) {
+			final int markerStart = sqlFragment.indexOf( "?", lastEnd );
+
+			// append the part of the fragment from the last-end position (start of string for first pass)
+			// to the index of the parameter marker
+			appendSql( sqlFragment.substring( lastEnd, markerStart ) );
+
+			// render the parameter marker and register it
+			visitParameterAsParameter( parameter );
+
+			lastEnd = markerStart + 1;
+		}
+
+		if ( lastEnd < sqlFragment.length() ) {
+			appendSql( sqlFragment.substring( lastEnd ) );
+		}
+	}
+
+	protected void simpleColumnWriteFragmentRendering(ColumnWriteFragment columnWriteFragment) {
+		appendSql( columnWriteFragment.getFragment() );
+
 		for ( ColumnValueParameter parameter : columnWriteFragment.getParameters() ) {
 			parameterBinders.add( parameter.getParameterBinder() );
 			jdbcParameters.addParameter( parameter );

hibernate-core/src/main/java/org/hibernate/sql/ast/spi/JdbcParameterRenderer.java

@@ -8,6 +8,7 @@ package org.hibernate.sql.ast.spi;
 
 import org.hibernate.dialect.Dialect;
 import org.hibernate.service.Service;
+import org.hibernate.sql.ast.internal.JdbcParameterRendererStandard;
 import org.hibernate.type.descriptor.jdbc.JdbcType;
 
 /**
@@ -26,4 +27,8 @@ public interface JdbcParameterRenderer extends Service {
 	 * @param dialect The Dialect in use within the SessionFactory
 	 */
 	void renderJdbcParameter(int position, JdbcType jdbcType, SqlAppender appender, Dialect dialect);
+
+	static boolean isStandardRenderer(JdbcParameterRenderer check) {
+		return check == null || JdbcParameterRendererStandard.class.equals( check.getClass() );
+	}
 }

hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueBindingList.java

@@ -10,13 +10,10 @@ import java.util.ArrayList;
 
 import org.hibernate.Internal;
 import org.hibernate.engine.jdbc.mutation.ParameterUsage;
-import org.hibernate.metamodel.mapping.EmbeddableMappingType;
 import org.hibernate.metamodel.mapping.JdbcMapping;
 import org.hibernate.metamodel.mapping.ModelPart;
 import org.hibernate.metamodel.mapping.SelectableMapping;
-import org.hibernate.sql.ast.tree.expression.ColumnReference;
+import org.hibernate.sql.model.ast.builder.ColumnValueBindingBuilder;
-import org.hibernate.type.descriptor.jdbc.AggregateJdbcType;
-import org.hibernate.type.descriptor.jdbc.JdbcType;
 
 @Internal
 public class ColumnValueBindingList extends ArrayList<ColumnValueBinding> implements ModelPart.JdbcValueConsumer {
@@ -53,16 +50,6 @@ public class ColumnValueBindingList extends ArrayList<ColumnValueBinding> implem
 		add( createValueBinding( column.getSelectionExpression(), null, column.getJdbcMapping() ) );
 	}
 
-	public void addRestriction(SelectableMapping column) {
-		add(
-				createValueBinding(
-						column.getSelectionExpression(),
-						column.getWriteExpression(),
-						column.getJdbcMapping()
-				)
-		);
-	}
-
 	public void addRestriction(String columnName, String columnWriteFragment, JdbcMapping jdbcMapping) {
 		add( createValueBinding( columnName, columnWriteFragment, jdbcMapping ) );
 	}
@@ -71,41 +58,14 @@ public class ColumnValueBindingList extends ArrayList<ColumnValueBinding> implem
 			String columnName,
 			String customWriteExpression,
 			JdbcMapping jdbcMapping) {
-		final ColumnReference columnReference = new ColumnReference( mutatingTable, columnName, jdbcMapping );
+		return ColumnValueBindingBuilder.createValueBinding(
-		final ColumnWriteFragment columnWriteFragment;
+				columnName,
-		if ( customWriteExpression == null ) {
+				customWriteExpression,
-			columnWriteFragment = null;
+				jdbcMapping,
-		}
+				mutatingTable,
-		else if ( customWriteExpression.contains( "?" ) ) {
+				parameterUsage,
-			final JdbcType jdbcType = jdbcMapping.getJdbcType();
+				parameters::apply
-			final EmbeddableMappingType aggregateMappingType = jdbcType instanceof AggregateJdbcType
+		);
-					? ( (AggregateJdbcType) jdbcType ).getEmbeddableMappingType()
-					: null;
-			if ( aggregateMappingType != null && !aggregateMappingType.shouldBindAggregateMapping() ) {
-				final ColumnValueParameterList parameters = new ColumnValueParameterList(
-						mutatingTable,
-						parameterUsage,
-						aggregateMappingType.getJdbcTypeCount()
-				);
-				aggregateMappingType.forEachSelectable( parameters );
-				this.parameters.addAll( parameters );
-
-				columnWriteFragment = new ColumnWriteFragment(
-						customWriteExpression,
-						parameters,
-						jdbcMapping
-				);
-			}
-			else {
-				final ColumnValueParameter parameter = new ColumnValueParameter( columnReference, parameterUsage );
-				parameters.add( parameter );
-				columnWriteFragment = new ColumnWriteFragment( customWriteExpression, parameter, jdbcMapping );
-			}
-		}
-		else {
-			columnWriteFragment = new ColumnWriteFragment( customWriteExpression, jdbcMapping );
-		}
-		return new ColumnValueBinding( columnReference, columnWriteFragment ) ;
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueParameterList.java

@@ -44,4 +44,13 @@ public class ColumnValueParameterList extends ArrayList<ColumnValueParameter> im
 				)
 		);
 	}
+
+	public void apply(Object parameterRef) {
+		if ( parameterRef instanceof ColumnValueParameterList ) {
+			addAll( (ColumnValueParameterList) parameterRef );
+		}
+		else {
+			add( (ColumnValueParameter) parameterRef );
+		}
+	}
 }

hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnWriteFragment.java

@@ -17,7 +17,11 @@ import org.hibernate.sql.ast.SqlAstWalker;
 import org.hibernate.sql.ast.tree.expression.Expression;
 
 /**
- * Models a column's "write fragment" within the SQL AST
+ * Models a column's value expression within the SQL AST. Used to model:<ul>
+ *     <li>a column's new value in a SET clause</li>
+ *     <li>a column's new value in a SET clause</li>
+ *     <li>a column's old value in a restriction (optimistic locking)</li>
+ * </ul>
  *
  * @see ColumnTransformer#write()
  *

hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableMutationBuilder.java

@@ -12,20 +12,14 @@ import java.util.List;
 import org.hibernate.engine.jdbc.mutation.ParameterUsage;
 import org.hibernate.engine.jdbc.spi.JdbcServices;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
-import org.hibernate.metamodel.mapping.EmbeddableMappingType;
 import org.hibernate.metamodel.mapping.JdbcMapping;
-import org.hibernate.sql.ast.tree.expression.ColumnReference;
 import org.hibernate.sql.model.MutationTarget;
 import org.hibernate.sql.model.MutationType;
 import org.hibernate.sql.model.TableMapping;
 import org.hibernate.sql.model.ast.ColumnValueBinding;
-import org.hibernate.sql.model.ast.ColumnValueParameter;
 import org.hibernate.sql.model.ast.ColumnValueParameterList;
-import org.hibernate.sql.model.ast.ColumnWriteFragment;
 import org.hibernate.sql.model.ast.MutatingTableReference;
 import org.hibernate.sql.model.ast.TableMutation;
-import org.hibernate.type.descriptor.jdbc.AggregateJdbcType;
-import org.hibernate.type.descriptor.jdbc.JdbcType;
 
 /**
  * Base support for TableMutationBuilder implementations
@@ -108,44 +102,19 @@ public abstract class AbstractTableMutationBuilder<M extends TableMutation<?>> i
 			JdbcMapping jdbcMapping) {
 		return createValueBinding( columnName, columnWriteFragment, jdbcMapping, ParameterUsage.SET );
 	}
-
 	protected ColumnValueBinding createValueBinding(
 			String columnName,
 			String customWriteExpression,
 			JdbcMapping jdbcMapping,
 			ParameterUsage parameterUsage) {
-		final ColumnReference columnReference = new ColumnReference( mutatingTable, columnName, jdbcMapping );
+		return ColumnValueBindingBuilder.createValueBinding(
-		final ColumnWriteFragment columnWriteFragment;
+				columnName,
-		if ( customWriteExpression.contains( "?" ) ) {
+				customWriteExpression,
-			final JdbcType jdbcType = jdbcMapping.getJdbcType();
+				jdbcMapping,
-			final EmbeddableMappingType aggregateMappingType = jdbcType instanceof AggregateJdbcType
+				getMutatingTable(),
-					? ( (AggregateJdbcType) jdbcType ).getEmbeddableMappingType()
+				parameterUsage,
-					: null;
+				parameters::apply
-			if ( aggregateMappingType != null && !aggregateMappingType.shouldBindAggregateMapping() ) {
+		);
-				final ColumnValueParameterList parameters = new ColumnValueParameterList(
-						getMutatingTable(),
-						parameterUsage,
-						aggregateMappingType.getJdbcTypeCount()
-				);
-				aggregateMappingType.forEachSelectable( parameters );
-				this.parameters.addAll( parameters );
-
-				columnWriteFragment = new ColumnWriteFragment(
-						customWriteExpression,
-						parameters,
-						jdbcMapping
-				);
-			}
-			else {
-				final ColumnValueParameter parameter = new ColumnValueParameter( columnReference, parameterUsage );
-				parameters.add( parameter );
-				columnWriteFragment = new ColumnWriteFragment( customWriteExpression, parameter, jdbcMapping );
-			}
-		}
-		else {
-			columnWriteFragment = new ColumnWriteFragment( customWriteExpression, jdbcMapping );
-		}
-		return new ColumnValueBinding( columnReference, columnWriteFragment ) ;
 	}
 
 	@SafeVarargs

hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/ColumnValueBindingBuilder.java

@@ -0,0 +1,129 @@
+/*
+ * 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.sql.model.ast.builder;
+
+import java.util.function.Consumer;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.hibernate.engine.jdbc.mutation.ParameterUsage;
+import org.hibernate.metamodel.mapping.EmbeddableMappingType;
+import org.hibernate.metamodel.mapping.JdbcMapping;
+import org.hibernate.sql.ast.tree.expression.ColumnReference;
+import org.hibernate.sql.model.ast.ColumnValueBinding;
+import org.hibernate.sql.model.ast.ColumnValueParameter;
+import org.hibernate.sql.model.ast.ColumnValueParameterList;
+import org.hibernate.sql.model.ast.ColumnWriteFragment;
+import org.hibernate.sql.model.ast.MutatingTableReference;
+import org.hibernate.type.descriptor.jdbc.AggregateJdbcType;
+import org.hibernate.type.descriptor.jdbc.JdbcType;
+
+/**
+ * Builder for {@link ColumnValueBinding} instances
+ *
+ * @author Steve Ebersole
+ */
+public class ColumnValueBindingBuilder {
+
+	/**
+	 * rexgex used to split the write-expressions into chunks of (1) quoted strings and (2) everything else.
+	 */
+	private static final String SPLIT_REGEX = "[^\\s\"']+|\"([^\"]*)\"|'([^']*)'";
+	private static final Pattern SPLIT_PATTERN = Pattern.compile( SPLIT_REGEX );
+
+
+	public static ColumnValueBinding createValueBinding(
+			String columnName,
+			String writeExpression,
+			JdbcMapping jdbcMapping,
+			MutatingTableReference mutatingTableReference,
+			ParameterUsage parameterUsage,
+			Consumer<Object> parameterConsumer) {
+		final ColumnReference columnReference = new ColumnReference( mutatingTableReference, columnName, jdbcMapping );
+		final ColumnWriteFragment columnWriteFragment = buildWriteFragment(
+				writeExpression,
+				jdbcMapping,
+				mutatingTableReference,
+				columnReference,
+				parameterUsage,
+				parameterConsumer
+		);
+		return new ColumnValueBinding( columnReference, columnWriteFragment ) ;
+	}
+
+	public static ColumnWriteFragment buildWriteFragment(
+			String writeExpression,
+			JdbcMapping jdbcMapping,
+			MutatingTableReference mutatingTableReference,
+			ColumnReference columnReference,
+			ParameterUsage parameterUsage,
+			Consumer<Object> parameterConsumer) {
+		if ( writeExpression == null ) {
+			return null;
+		}
+
+		if ( writeExpression.equals( "?" )
+				|| ( writeExpression.contains( "?" ) && !writeExpression.contains( "'" ) ) ) {
+			return buildParameterizedWriteFragment( writeExpression, jdbcMapping, mutatingTableReference, columnReference, parameterUsage, parameterConsumer );
+		}
+
+		if ( !writeExpression.contains( "?" ) ) {
+			return new ColumnWriteFragment( writeExpression, jdbcMapping );
+		}
+
+		if ( containsParameter( writeExpression ) ) {
+			return buildParameterizedWriteFragment( writeExpression, jdbcMapping, mutatingTableReference, columnReference, parameterUsage, parameterConsumer );
+		}
+
+		return new ColumnWriteFragment( writeExpression, jdbcMapping );
+	}
+
+	private static ColumnWriteFragment buildParameterizedWriteFragment(
+			String writeExpression,
+			JdbcMapping jdbcMapping,
+			MutatingTableReference mutatingTableReference,
+			ColumnReference columnReference,
+			ParameterUsage parameterUsage,
+			Consumer<Object> parameterConsumer) {
+		final JdbcType jdbcType = jdbcMapping.getJdbcType();
+		final EmbeddableMappingType aggregateMappingType = jdbcType instanceof AggregateJdbcType
+				? ( (AggregateJdbcType) jdbcType ).getEmbeddableMappingType()
+				: null;
+		if ( aggregateMappingType != null && !aggregateMappingType.shouldBindAggregateMapping() ) {
+			final ColumnValueParameterList parameters = new ColumnValueParameterList(
+					mutatingTableReference,
+					parameterUsage,
+					aggregateMappingType.getJdbcTypeCount()
+			);
+			aggregateMappingType.forEachSelectable( parameters );
+			parameterConsumer.accept( parameters );
+
+			return new ColumnWriteFragment( writeExpression, parameters, jdbcMapping );
+		}
+		else {
+			final ColumnValueParameter parameter = new ColumnValueParameter( columnReference, parameterUsage );
+			parameterConsumer.accept( parameter );
+			return new ColumnWriteFragment( writeExpression, parameter, jdbcMapping );
+		}
+	}
+
+	private static boolean containsParameter(String writeExpression) {
+		final Matcher matcher = SPLIT_PATTERN.matcher( writeExpression );
+		while ( matcher.find() ) {
+			final String group = matcher.group();
+			if ( group.startsWith( "'" ) && group.endsWith( "'" ) ) {
+				continue;
+			}
+
+			if ( group.contains( "?" ) ) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/JdbcParameterRendererTests.java

@@ -11,6 +11,7 @@ import org.hibernate.annotations.FilterDef;
 import org.hibernate.annotations.ParamDef;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.dialect.H2Dialect;
+import org.hibernate.internal.util.StringHelper;
 import org.hibernate.sql.ast.spi.JdbcParameterRenderer;
 import org.hibernate.sql.ast.spi.SqlAppender;
 import org.hibernate.type.descriptor.jdbc.JdbcType;
@@ -27,7 +28,6 @@ import org.junit.jupiter.api.Test;
 import jakarta.persistence.Basic;
 import jakarta.persistence.Entity;
 import jakarta.persistence.Id;
-import jakarta.persistence.OneToMany;
 import jakarta.persistence.Table;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -61,6 +61,7 @@ public class JdbcParameterRendererTests {
 		final String sql = statementInspector.getSqlQueries().get( 0 );
 		assertThat( sql ).contains( "?1" );
 	}
+
 	@Test
 	public void testFilters(SessionFactoryScope scope) {
 		final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
@@ -73,6 +74,37 @@ public class JdbcParameterRendererTests {
 			final String sql = statementInspector.getSqlQueries().get( 0 );
 			assertThat( sql ).contains( "?1" );
 		} );
+
+		statementInspector.clear();
+		scope.inTransaction( (session) -> {
+			final EntityWithFilters it = new EntityWithFilters( 1, "It", "EMEA" );
+			session.persist( it );
+		} );
+		assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
+		assertThat( StringHelper.count( statementInspector.getSqlQueries().get( 0 ), "?" ) ).isEqualTo( 3 );
+		assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1" );
+		assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?2" );
+		assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?3" );
+
+		scope.inTransaction( (session) -> {
+			final EntityWithFilters it = session.find( EntityWithFilters.class, 1 );
+			statementInspector.clear();
+			it.setName( "It 2" );
+		} );
+		assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
+		assertThat( StringHelper.count( statementInspector.getSqlQueries().get( 0 ), "?" ) ).isEqualTo( 3 );
+		assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1" );
+		assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?2" );
+		assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?3" );
+
+		scope.inTransaction( (session) -> {
+			final EntityWithFilters it = session.find( EntityWithFilters.class, 1 );
+			statementInspector.clear();
+			session.remove( it );
+		} );
+		assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
+		assertThat( StringHelper.count( statementInspector.getSqlQueries().get( 0 ), "?" ) ).isEqualTo( 1 );
+		assertThat( statementInspector.getSqlQueries().get( 0 ) ).contains( "?1" );
 	}
 
 	public static class JdbcParameterRendererImpl implements JdbcParameterRenderer {