From 9b83362b88aa3ed071c4e835c8ded11552a82c2c Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Wed, 8 Mar 2023 18:06:19 -0600 Subject: [PATCH] HHH-16260 - JdbcParameterRenderer not called with dynamic filters HHH-16256 - JdbcParameterRenderer to have an impact on write operations --- .../util/collections/CollectionHelper.java | 8 ++ .../sql/ast/spi/AbstractSqlAstTranslator.java | 34 ++++- .../sql/ast/spi/JdbcParameterRenderer.java | 5 + .../sql/model/ast/ColumnValueBindingList.java | 58 ++------ .../model/ast/ColumnValueParameterList.java | 9 ++ .../sql/model/ast/ColumnWriteFragment.java | 6 +- .../builder/AbstractTableMutationBuilder.java | 47 ++----- .../builder/ColumnValueBindingBuilder.java | 129 ++++++++++++++++++ .../sql/ast/JdbcParameterRendererTests.java | 34 ++++- 9 files changed, 239 insertions(+), 91 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/ColumnValueBindingBuilder.java diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/collections/CollectionHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/collections/CollectionHelper.java index 2fc6faca75..6171fe023b 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/collections/CollectionHelper.java +++ b/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 Set toSet(X... values) { final HashSet result = new HashSet<>(); if ( isNotEmpty( values ) ) { diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java index 01cf480e20..275223fff2 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java @@ -8012,7 +8012,39 @@ public abstract class AbstractSqlAstTranslator 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 ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/JdbcParameterRenderer.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/JdbcParameterRenderer.java index 68bc109a62..f9c8be0ceb 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/JdbcParameterRenderer.java +++ b/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() ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueBindingList.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueBindingList.java index 3c930fb053..41a92cad9d 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueBindingList.java +++ b/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.type.descriptor.jdbc.AggregateJdbcType; -import org.hibernate.type.descriptor.jdbc.JdbcType; +import org.hibernate.sql.model.ast.builder.ColumnValueBindingBuilder; @Internal public class ColumnValueBindingList extends ArrayList implements ModelPart.JdbcValueConsumer { @@ -53,16 +50,6 @@ public class ColumnValueBindingList extends ArrayList 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 implem String columnName, String customWriteExpression, JdbcMapping jdbcMapping) { - final ColumnReference columnReference = new ColumnReference( mutatingTable, columnName, jdbcMapping ); - final ColumnWriteFragment columnWriteFragment; - if ( customWriteExpression == null ) { - columnWriteFragment = null; - } - else if ( customWriteExpression.contains( "?" ) ) { - 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( - 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 ) ; + return ColumnValueBindingBuilder.createValueBinding( + columnName, + customWriteExpression, + jdbcMapping, + mutatingTable, + parameterUsage, + parameters::apply + ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueParameterList.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueParameterList.java index 4a0ae840e1..a7acbf5c0e 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueParameterList.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnValueParameterList.java @@ -44,4 +44,13 @@ public class ColumnValueParameterList extends ArrayList im ) ); } + + public void apply(Object parameterRef) { + if ( parameterRef instanceof ColumnValueParameterList ) { + addAll( (ColumnValueParameterList) parameterRef ); + } + else { + add( (ColumnValueParameter) parameterRef ); + } + } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnWriteFragment.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnWriteFragment.java index c0c5e2eda6..1ac77d37ac 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/ColumnWriteFragment.java +++ b/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:
    + *
  • a column's new value in a SET clause
    • + *
  • a column's new value in a SET clause
    • + *
  • a column's old value in a restriction (optimistic locking)
    • + *
* * @see ColumnTransformer#write() * diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableMutationBuilder.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableMutationBuilder.java index 1e08409bd0..3a2df22b16 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableMutationBuilder.java +++ b/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> 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 ); - final ColumnWriteFragment columnWriteFragment; - if ( customWriteExpression.contains( "?" ) ) { - 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( - 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 ) ; + return ColumnValueBindingBuilder.createValueBinding( + columnName, + customWriteExpression, + jdbcMapping, + getMutatingTable(), + parameterUsage, + parameters::apply + ); } @SafeVarargs diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/ColumnValueBindingBuilder.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/ColumnValueBindingBuilder.java new file mode 100644 index 0000000000..bff29bd2be --- /dev/null +++ b/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 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 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 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; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/JdbcParameterRendererTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/JdbcParameterRendererTests.java index b874995297..fd2d737b5b 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/ast/JdbcParameterRendererTests.java +++ b/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 {