From c04caa18de3c5ff81c17ebb151430b9cf18b6284 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Wed, 30 Nov 2022 12:59:31 -0600 Subject: [PATCH] HHH-15393 - Improve write-paths to use mapping model --- .../collection/OneToManyPersister.java | 5 +- .../sql/ast/spi/AbstractSqlAstTranslator.java | 47 ++++++------ .../ast/AbstractRestrictedTableMutation.java | 8 +-- .../sql/model/ast/AbstractTableMutation.java | 17 ++++- .../sql/model/ast/AbstractTableUpdate.java | 71 +++++++++++++++---- .../model/ast/RestrictedTableMutation.java | 23 ++++++ .../sql/model/ast/TableMutation.java | 26 ++++++- .../builder/AbstractTableInsertBuilder.java | 16 ++++- .../builder/AbstractTableMutationBuilder.java | 11 +-- .../builder/AbstractTableUpdateBuilder.java | 24 ++++++- .../builder/TableDeleteBuilderStandard.java | 19 ++++- .../builder/TableUpdateBuilderStandard.java | 18 +++-- .../model/internal/TableUpdateCustomSql.java | 13 +++- .../sql/model/internal/TableUpdateNoSet.java | 6 ++ .../model/internal/TableUpdateStandard.java | 20 ++++-- .../sql/model/internal/TableUpsert.java | 35 +++++++-- .../jdbc/OptionalTableUpdateOperation.java | 16 ++--- .../customsql/CustomSqlGeneratedTest.java | 7 +- .../orm/test/customsql/CustomSqlTest.java | 2 - 19 files changed, 290 insertions(+), 94 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/persister/collection/OneToManyPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/collection/OneToManyPersister.java index 3f298dc427..d69fc47ec4 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/collection/OneToManyPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/collection/OneToManyPersister.java @@ -6,7 +6,6 @@ */ package org.hibernate.persister.collection; -import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.function.Consumer; @@ -390,10 +389,10 @@ public class OneToManyPersister extends AbstractCollectionPersister { return new TableUpdateStandard( tableReference, this, - parameters, valueBindings, keyRestrictionBindings, - Collections.emptyList(), + null, + parameters, sqlWhereString ); } 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 4e7a225862..2b7293a882 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 @@ -178,6 +178,7 @@ import org.hibernate.sql.exec.spi.JdbcOperationQueryUpdate; import org.hibernate.sql.exec.spi.JdbcParameterBinder; import org.hibernate.sql.exec.spi.JdbcParameterBinding; import org.hibernate.sql.exec.spi.JdbcParameterBindings; +import org.hibernate.sql.model.ast.ColumnValueParameter; import org.hibernate.sql.model.ast.ColumnWriteFragment; import org.hibernate.sql.model.ast.TableMutation; import org.hibernate.sql.model.internal.TableDeleteCustomSql; @@ -615,6 +616,7 @@ public abstract class AbstractSqlAstTranslator implemen this.limitParameter = limitParameter; } + @SuppressWarnings("unchecked") protected R interpretExpression(Expression expression, JdbcParameterBindings jdbcParameterBindings) { if ( expression instanceof Literal ) { return (R) ( (Literal) expression ).getLiteralValue(); @@ -766,6 +768,7 @@ public abstract class AbstractSqlAstTranslator implemen } } + //noinspection unchecked return (T) jdbcOperation; } finally { @@ -902,6 +905,7 @@ public abstract class AbstractSqlAstTranslator implemen int startPosition, JdbcParameterBindings jdbcParamBindings, ExecutionContext executionContext) throws SQLException { + //noinspection unchecked getJdbcMapping().getJdbcValueBinder().bind( statement, executionContext.getQueryOptions().getLimit().getFirstRow(), @@ -923,6 +927,7 @@ public abstract class AbstractSqlAstTranslator implemen int startPosition, JdbcParameterBindings jdbcParamBindings, ExecutionContext executionContext) throws SQLException { + //noinspection unchecked getJdbcMapping().getJdbcValueBinder().bind( statement, executionContext.getQueryOptions().getLimit().getMaxRows(), @@ -1288,7 +1293,7 @@ public abstract class AbstractSqlAstTranslator implemen private enum LockKind { NONE, SHARE, - UPDATE; + UPDATE } protected String getForUpdate() { @@ -1625,12 +1630,10 @@ public abstract class AbstractSqlAstTranslator implemen if ( cte.getCteTable().getCteColumns() == null ) { final List columnExpressions = new ArrayList<>(); cte.getCteTable().getTableGroupProducer().visitSubParts( - modelPart -> { - modelPart.forEachSelectable( - 0, - (index, mapping) -> columnExpressions.add( mapping.getSelectionExpression() ) - ); - }, + modelPart -> modelPart.forEachSelectable( + 0, + (index, mapping) -> columnExpressions.add( mapping.getSelectionExpression() ) + ), null ); for ( String columnExpression : columnExpressions ) { @@ -4013,6 +4016,7 @@ public abstract class AbstractSqlAstTranslator implemen throw new ExecutionException( "JDBC parameter value not bound - " + offsetParameter ); } final Number bindValue = (Number) binding.getBindValue(); + //noinspection unchecked offsetParameter.getExpressionType().getJdbcMappings().get( 0 ).getJdbcValueBinder().bind( statement, bindValue.intValue() + offsetValue, @@ -4092,6 +4096,7 @@ public abstract class AbstractSqlAstTranslator implemen offsetValue = dynamicOffset.intValue() + staticOffset; dynamicOffset = null; } + //noinspection unchecked fetchParameter.getExpressionType().getJdbcMappings().get( 0 ).getJdbcValueBinder().bind( statement, bindValue.intValue() + offsetValue, @@ -5062,7 +5067,7 @@ public abstract class AbstractSqlAstTranslator implemen assert literal.getExpressionType().getJdbcTypeCount() == 1; final JdbcMapping jdbcMapping = literal.getJdbcMapping(); - final JdbcLiteralFormatter literalFormatter = jdbcMapping.getJdbcLiteralFormatter(); + final JdbcLiteralFormatter literalFormatter = jdbcMapping.getJdbcLiteralFormatter(); // If we encounter a plain literal in the select clause which has no literal formatter, we must render it as parameter if ( literalFormatter == null ) { @@ -6510,7 +6515,8 @@ public abstract class AbstractSqlAstTranslator implemen else { assert jdbcParameter.getExpressionType().getJdbcTypeCount() == 1; final JdbcMapping jdbcMapping = jdbcParameter.getExpressionType().getJdbcMappings().get( 0 ); - final JdbcLiteralFormatter literalFormatter = jdbcMapping.getJdbcLiteralFormatter(); + //noinspection unchecked + final JdbcLiteralFormatter literalFormatter = jdbcMapping.getJdbcLiteralFormatter(); if ( literalFormatter == null ) { throw new IllegalArgumentException( "Can't render parameter as literal, no literal formatter found" ); } @@ -7685,7 +7691,7 @@ public abstract class AbstractSqlAstTranslator implemen } /** - * Handle rendering an insert with no columns (eye roll) + * Handle rendering an insert with no columns */ protected void renderInsertIntoNoColumns(TableInsertStandard tableInsert) { renderIntoIntoAndTable( tableInsert ); @@ -7697,10 +7703,7 @@ public abstract class AbstractSqlAstTranslator implemen assert sqlBuffer.toString().isEmpty(); sqlBuffer.append( tableInsert.getCustomSql() ); - tableInsert.getParameters().forEach( (parameter) -> { - parameterBinders.add( parameter.getParameterBinder() ); - jdbcParameters.addParameter( parameter ); - } ); + tableInsert.forEachParameter( this::applyParameter ); } @Override @@ -7777,10 +7780,7 @@ public abstract class AbstractSqlAstTranslator implemen assert sqlBuffer.toString().isEmpty(); sqlBuffer.append( tableUpdate.getCustomSql() ); - tableUpdate.getParameters().forEach( (parameter) -> { - parameterBinders.add( parameter.getParameterBinder() ); - jdbcParameters.addParameter( parameter ); - } ); + tableUpdate.forEachParameter( this::applyParameter ); } @Override @@ -7838,10 +7838,13 @@ public abstract class AbstractSqlAstTranslator implemen assert sqlBuffer.toString().isEmpty(); sqlBuffer.append( tableDelete.getCustomSql() ); - tableDelete.getParameters().forEach( (parameter) -> { - parameterBinders.add( parameter.getParameterBinder() ); - jdbcParameters.addParameter( parameter ); - } ); + tableDelete.forEachParameter( this::applyParameter ); + } + + protected void applyParameter(ColumnValueParameter parameter) { + assert parameter != null; + parameterBinders.add( parameter.getParameterBinder() ); + jdbcParameters.addParameter( parameter ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractRestrictedTableMutation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractRestrictedTableMutation.java index f4db77dd45..40234c5e31 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractRestrictedTableMutation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractRestrictedTableMutation.java @@ -40,9 +40,7 @@ public abstract class AbstractRestrictedTableMutation consumer) { - for ( int i = 0; i < keyRestrictionBindings.size(); i++ ) { - consumer.accept( i, keyRestrictionBindings.get( i ) ); - } + forEachThing( keyRestrictionBindings, consumer ); } @Override @@ -52,8 +50,6 @@ public abstract class AbstractRestrictedTableMutation consumer) { - for ( int i = 0; i < optLockRestrictionBindings.size(); i++ ) { - consumer.accept( i, optLockRestrictionBindings.get( i ) ); - } + forEachThing( optLockRestrictionBindings, consumer ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractTableMutation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractTableMutation.java index f8bc1213e5..efed4fee2d 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractTableMutation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractTableMutation.java @@ -8,6 +8,7 @@ package org.hibernate.sql.model.ast; import java.util.List; import java.util.function.BiConsumer; +import java.util.function.Consumer; import org.hibernate.engine.jdbc.mutation.internal.MutationQueryOptions; import org.hibernate.engine.spi.SessionFactoryImplementor; @@ -75,7 +76,21 @@ public abstract class AbstractTableMutation return parameters; } - protected void forEachThing(List list, BiConsumer action) { + public void forEachParameter(Consumer consumer) { + if ( parameters == null ) { + return; + } + + for ( int i = 0; i < parameters.size(); i++ ) { + consumer.accept( parameters.get( i ) ); + } + } + + protected static void forEachThing(List list, BiConsumer action) { + if ( list == null ) { + return; + } + for ( int i = 0; i < list.size(); i++ ) { action.accept( i, list.get( i ) ); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractTableUpdate.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractTableUpdate.java index 65012b22d3..57f0d9f4fa 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractTableUpdate.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/AbstractTableUpdate.java @@ -6,8 +6,10 @@ */ package org.hibernate.sql.model.ast; +import java.util.ArrayList; import java.util.List; import java.util.function.BiConsumer; +import java.util.function.Consumer; import org.hibernate.jdbc.Expectation; import org.hibernate.sql.exec.spi.JdbcParameterBinder; @@ -17,6 +19,8 @@ import org.hibernate.sql.model.TableMapping; import org.hibernate.sql.model.jdbc.JdbcUpdateMutation; /** + * Base support for TableUpdate implementations + * * @author Steve Ebersole */ public abstract class AbstractTableUpdate @@ -27,31 +31,60 @@ public abstract class AbstractTableUpdate public AbstractTableUpdate( MutatingTableReference mutatingTable, MutationTarget mutationTarget, - List parameters, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings) { - this( + super( mutatingTable, mutationTarget, - "update for " + mutationTarget.getRolePath(), - parameters, - valueBindings, + "update for " + mutationTarget.getNavigableRole(), keyRestrictionBindings, - optLockRestrictionBindings + optLockRestrictionBindings, + collectParameters( valueBindings, keyRestrictionBindings, optLockRestrictionBindings ) ); + + this.valueBindings = valueBindings; } - public AbstractTableUpdate( - MutatingTableReference mutatingTable, + public AbstractTableUpdate( + MutatingTableReference tableReference, MutationTarget mutationTarget, - String sqlComment, - List parameters, + List valueBindings, + List keyRestrictionBindings, + List optLockRestrictionBindings, + List parameters) { + super( + tableReference, + mutationTarget, + "update for " + mutationTarget.getNavigableRole(), + keyRestrictionBindings, + optLockRestrictionBindings, + parameters + ); + this.valueBindings = valueBindings; + } + + public static List collectParameters( List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings) { - super( mutatingTable, mutationTarget, sqlComment, keyRestrictionBindings, optLockRestrictionBindings, parameters ); - this.valueBindings = valueBindings; + final List params = new ArrayList<>(); + + final BiConsumer intermediateConsumer = (index, binding) -> { + final ColumnWriteFragment valueExpression = binding.getValueExpression(); + if ( valueExpression != null ) { + final ColumnValueParameter parameter = valueExpression.getParameter(); + if ( parameter != null ) { + params.add( parameter ); + } + } + }; + + forEachThing( valueBindings, intermediateConsumer ); + forEachThing( keyRestrictionBindings, intermediateConsumer ); + forEachThing( optLockRestrictionBindings, intermediateConsumer ); + + return params; } @Override @@ -74,6 +107,20 @@ public abstract class AbstractTableUpdate forEachThing( valueBindings, consumer ); } + @Override + public void forEachParameter(Consumer consumer) { + final BiConsumer intermediateConsumer = (index, binding) -> { + final ColumnValueParameter parameter = binding.getValueExpression().getParameter(); + if ( parameter != null ) { + consumer.accept( parameter ); + } + }; + + forEachThing( getValueBindings(), intermediateConsumer ); + forEachThing( getKeyBindings(), intermediateConsumer ); + forEachThing( getOptimisticLockBindings(), intermediateConsumer ); + } + @Override protected O createMutationOperation(TableMapping tableDetails, String sql, List effectiveBinders) { //noinspection unchecked diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/RestrictedTableMutation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/RestrictedTableMutation.java index bf6dffcf44..e7a28c1805 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/RestrictedTableMutation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/RestrictedTableMutation.java @@ -9,6 +9,7 @@ package org.hibernate.sql.model.ast; import java.util.List; import java.util.function.BiConsumer; +import org.hibernate.annotations.OptimisticLockType; import org.hibernate.sql.model.MutationOperation; /** @@ -19,20 +20,42 @@ import org.hibernate.sql.model.MutationOperation; */ public interface RestrictedTableMutation extends TableMutation { + /** + * The bindings for each key restriction (WHERE clause). + */ List getKeyBindings(); + /** + * The number of {@linkplain #getKeyBindings() key bindings} + */ default int getNumberOfKeyBindings() { return getKeyBindings().size(); } + /** + * Visit each {@linkplain #getKeyBindings() key binding} + */ void forEachKeyBinding(BiConsumer consumer); + /** + * All optimistic-lock bindings (WHERE clause), appended after + * {@linkplain #getKeyBindings() key bindings} + * + * @see OptimisticLockType + */ List getOptimisticLockBindings(); + /** + * The number of {@linkplain #getOptimisticLockBindings() optimistic-lock bindings} + */ + default int getNumberOfOptimisticLockBindings() { final List bindings = getOptimisticLockBindings(); return bindings == null ? 0 : bindings.size(); } + /** + * Visit each {@linkplain #getOptimisticLockBindings() optimistic-lock binding} + */ void forEachOptimisticLockBinding(BiConsumer consumer); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/TableMutation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/TableMutation.java index 6c864c7a19..059da88d84 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/TableMutation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/TableMutation.java @@ -7,6 +7,7 @@ package org.hibernate.sql.model.ast; import java.util.List; +import java.util.function.Consumer; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.jdbc.Expectation; @@ -24,6 +25,10 @@ import org.hibernate.sql.model.ValuesAnalysis; * Acts as a factory for {@link org.hibernate.sql.model.MutationOperation} instances, * which are the forms used to "perform" the mutation using JDBC. * + * @apiNote The parameter order returned from here is the expected order of binding + * to the {@link java.sql.PreparedStatement} - see {@link #getParameters()} and + * {@link #forEachParameter} + * * @author Steve Ebersole */ public interface TableMutation extends Statement { @@ -57,11 +62,30 @@ public interface TableMutation extends Statement { Expectation getExpectation(); /** - * The JDBC parameters associated with this mutation + * The JDBC parameters associated with this mutation. + * + * The order here is the expected binding order for the + * {@link java.sql.PreparedStatement}. + * + * @see #forEachParameter */ List getParameters(); + /** + * Visit the JDBC parameters associated with this mutation. + * + * The order here is the expected binding order for the + * {@link java.sql.PreparedStatement}. + * + * @see #getParameters + */ + void forEachParameter(Consumer consumer); + O createMutationOperation(ValuesAnalysis valuesAnalysis, SessionFactoryImplementor sessionFactory); + /** + * A {@link org.hibernate.sql.ast.SqlAstTranslator} callback to create + * an appropriate mutation using the translated sql and parameter binders. + */ O createMutationOperation(String sql, List parameterBinders); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableInsertBuilder.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableInsertBuilder.java index 035e868830..bb14fe1391 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableInsertBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableInsertBuilder.java @@ -15,6 +15,7 @@ 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.MutatingTableReference; import org.hibernate.sql.model.ast.TableInsert; @@ -30,15 +31,17 @@ public abstract class AbstractTableInsertBuilder private final List valueBindingList = new ArrayList<>(); private List lobValueBindingList; + private final List parameters = new ArrayList<>(); + public AbstractTableInsertBuilder( - MutationTarget mutationTarget, + MutationTarget mutationTarget, TableMapping table, SessionFactoryImplementor sessionFactory) { super( MutationType.INSERT, mutationTarget, table, sessionFactory ); } public AbstractTableInsertBuilder( - MutationTarget mutationTarget, + MutationTarget mutationTarget, MutatingTableReference tableReference, SessionFactoryImplementor sessionFactory) { super( MutationType.INSERT, mutationTarget, tableReference, sessionFactory ); @@ -56,6 +59,10 @@ public abstract class AbstractTableInsertBuilder return lobValueBindingList; } + protected List getParameters() { + return parameters; + } + @Override public void addValueColumn( String columnName, @@ -81,4 +88,9 @@ public abstract class AbstractTableInsertBuilder JdbcMapping jdbcMapping) { addColumn( columnName, columnWriteFragment, jdbcMapping, keyBindingList ); } + + @Override + protected void handleParameterCreation(ColumnValueParameter parameter) { + parameters.add( parameter ); + } } 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 53ab2b1096..eecb6c9ad5 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 @@ -36,9 +36,6 @@ public abstract class AbstractTableMutationBuilder> i private final MutatingTableReference mutatingTable; - private final List parameters = new ArrayList<>(); - - public AbstractTableMutationBuilder( MutationType mutationType, MutationTarget mutationTarget, @@ -76,10 +73,6 @@ public abstract class AbstractTableMutationBuilder> i return sessionFactory.getJdbcServices(); } - protected List getParameters() { - return parameters; - } - protected void addColumn( String columnName, String columnWriteFragment, @@ -116,7 +109,7 @@ public abstract class AbstractTableMutationBuilder> i final ColumnValueParameter parameter; if ( columnWriteFragment.contains( "?" ) ) { parameter = new ColumnValueParameter( columnReference, parameterUsage ); - parameters.add( parameter ); + handleParameterCreation( parameter ); } else { parameter = null; @@ -125,6 +118,8 @@ public abstract class AbstractTableMutationBuilder> i return new ColumnValueBinding( columnReference, new ColumnWriteFragment( columnWriteFragment, parameter, jdbcMapping ) ); } + protected abstract void handleParameterCreation(ColumnValueParameter parameter); + @SafeVarargs protected final List combine(List list1, List... additionalLists) { final ArrayList combined = list1 == null diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableUpdateBuilder.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableUpdateBuilder.java index 7c77e7c13f..f64a52803f 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableUpdateBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/AbstractTableUpdateBuilder.java @@ -18,8 +18,11 @@ import org.hibernate.sql.model.TableMapping; import org.hibernate.sql.model.ast.ColumnValueBinding; import org.hibernate.sql.model.ast.MutatingTableReference; import org.hibernate.sql.model.ast.RestrictedTableMutation; +import org.hibernate.sql.model.ast.TableUpdate; /** + * Base support for TableUpdateBuilder implementations + * * @author Steve Ebersole */ public abstract class AbstractTableUpdateBuilder @@ -30,27 +33,44 @@ public abstract class AbstractTableUpdateBuilder private List lobValueBindings; public AbstractTableUpdateBuilder( - MutationTarget mutationTarget, + MutationTarget mutationTarget, TableMapping tableMapping, SessionFactoryImplementor sessionFactory) { super( MutationType.UPDATE, mutationTarget, tableMapping, sessionFactory ); } public AbstractTableUpdateBuilder( - MutationTarget mutationTarget, + MutationTarget mutationTarget, MutatingTableReference tableReference, SessionFactoryImplementor sessionFactory) { super( MutationType.UPDATE, mutationTarget, tableReference, sessionFactory ); } + /** + * The bindings for each key restriction (WHERE clause). + * + * @see TableUpdate#getKeyBindings + */ protected List getKeyBindings() { return keyBindings; } + /** + * The (non-LOB) bindings for each column being updated (SET clause) + * + * @see TableUpdate#getValueBindings + */ protected List getValueBindings() { return valueBindings; } + /** + * @apiNote The distinction with {@link #getValueBindings} is to help + * in cases e.g. where a dialect needs to order all LOB bindings after + * all non-LOB bindings + * + * @see TableUpdate#getValueBindings + */ protected List getLobValueBindings() { return lobValueBindings; } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableDeleteBuilderStandard.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableDeleteBuilderStandard.java index 9f82a39611..59c52dc8f4 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableDeleteBuilderStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableDeleteBuilderStandard.java @@ -6,11 +6,15 @@ */ package org.hibernate.sql.model.ast.builder; +import java.util.ArrayList; +import java.util.List; + import org.hibernate.HibernateException; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.sql.model.MutationTarget; import org.hibernate.sql.model.MutationType; import org.hibernate.sql.model.TableMapping; +import org.hibernate.sql.model.ast.ColumnValueParameter; import org.hibernate.sql.model.ast.MutatingTableReference; import org.hibernate.sql.model.ast.TableDelete; import org.hibernate.sql.model.internal.TableDeleteCustomSql; @@ -28,15 +32,17 @@ public class TableDeleteBuilderStandard implements TableDeleteBuilder { private final boolean isCustomSql; + private final List parameters = new ArrayList<>(); + public TableDeleteBuilderStandard( - MutationTarget mutationTarget, + MutationTarget mutationTarget, TableMapping table, SessionFactoryImplementor sessionFactory) { this( mutationTarget, new MutatingTableReference( table ), sessionFactory ); } public TableDeleteBuilderStandard( - MutationTarget mutationTarget, + MutationTarget mutationTarget, MutatingTableReference tableReference, SessionFactoryImplementor sessionFactory) { super( MutationType.DELETE, mutationTarget, tableReference, sessionFactory ); @@ -84,4 +90,13 @@ public class TableDeleteBuilderStandard getParameters() ); } + + protected List getParameters() { + return parameters; + } + + @Override + protected void handleParameterCreation(ColumnValueParameter parameter) { + parameters.add( parameter ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableUpdateBuilderStandard.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableUpdateBuilderStandard.java index ffdced47a0..3d4dd640fd 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableUpdateBuilderStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableUpdateBuilderStandard.java @@ -13,6 +13,7 @@ import org.hibernate.sql.model.MutationOperation; import org.hibernate.sql.model.MutationTarget; 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.MutatingTableReference; import org.hibernate.sql.model.ast.RestrictedTableMutation; import org.hibernate.sql.model.internal.TableUpdateCustomSql; @@ -21,18 +22,20 @@ import org.hibernate.sql.model.internal.TableUpdateStandard; import org.hibernate.sql.model.internal.TableUpsert; /** + * Standard TableUpdateBuilder implementation + * * @author Steve Ebersole */ public class TableUpdateBuilderStandard extends AbstractTableUpdateBuilder { public TableUpdateBuilderStandard( - MutationTarget mutationTarget, + MutationTarget mutationTarget, TableMapping tableMapping, SessionFactoryImplementor sessionFactory) { super( mutationTarget, tableMapping, sessionFactory ); } public TableUpdateBuilderStandard( - MutationTarget mutationTarget, + MutationTarget mutationTarget, MutatingTableReference tableReference, SessionFactoryImplementor sessionFactory) { super( mutationTarget, tableReference, sessionFactory ); @@ -50,7 +53,6 @@ public class TableUpdateBuilderStandard extends Abs return (RestrictedTableMutation) new TableUpdateCustomSql( getMutatingTable(), getMutationTarget(), - getParameters(), valueBindings, getKeyRestrictionBindings(), getOptimisticLockBindings() @@ -63,18 +65,22 @@ public class TableUpdateBuilderStandard extends Abs getMutationTarget(), valueBindings, getKeyRestrictionBindings(), - getOptimisticLockBindings(), - getParameters() + getOptimisticLockBindings() ); } return (RestrictedTableMutation) new TableUpdateStandard( getMutatingTable(), getMutationTarget(), - getParameters(), valueBindings, getKeyRestrictionBindings(), getOptimisticLockBindings() ); } + + @Override + protected void handleParameterCreation(ColumnValueParameter parameter) { + // nothing to do for updates... each TableUpdate impl collects + // the parameters from the bindings in a specific order + } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateCustomSql.java b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateCustomSql.java index 71688309a3..94c8452949 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateCustomSql.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateCustomSql.java @@ -30,11 +30,20 @@ public class TableUpdateCustomSql public TableUpdateCustomSql( MutatingTableReference mutatingTable, MutationTarget mutationTarget, - List parameters, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings) { - super( mutatingTable, mutationTarget, parameters, valueBindings, keyRestrictionBindings, optLockRestrictionBindings ); + super( mutatingTable, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings ); + } + + public TableUpdateCustomSql( + MutatingTableReference mutatingTable, + MutationTarget mutationTarget, + List valueBindings, + List keyRestrictionBindings, + List optLockRestrictionBindings, + List parameters) { + super( mutatingTable, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateNoSet.java b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateNoSet.java index a52d34a78b..58b95b4ff7 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateNoSet.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateNoSet.java @@ -9,6 +9,7 @@ package org.hibernate.sql.model.internal; import java.util.Collections; import java.util.List; import java.util.function.BiConsumer; +import java.util.function.Consumer; import org.hibernate.jdbc.Expectation; import org.hibernate.sql.ast.SqlAstWalker; @@ -18,6 +19,7 @@ import org.hibernate.sql.model.MutationTarget; import org.hibernate.sql.model.TableMapping; import org.hibernate.sql.model.ast.AbstractRestrictedTableMutation; import org.hibernate.sql.model.ast.ColumnValueBinding; +import org.hibernate.sql.model.ast.ColumnValueParameter; import org.hibernate.sql.model.ast.MutatingTableReference; import org.hibernate.sql.model.ast.TableUpdate; import org.hibernate.sql.model.jdbc.JdbcMutationOperation; @@ -84,4 +86,8 @@ public class TableUpdateNoSet @Override public void forEachValueBinding(BiConsumer consumer) { } + + @Override + public void forEachParameter(Consumer consumer) { + } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateStandard.java b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateStandard.java index e36ee1d400..28d616765f 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpdateStandard.java @@ -25,22 +25,32 @@ public class TableUpdateStandard extends AbstractTableUpdate mutationTarget, - List parameters, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings) { - this( mutatingTable, mutationTarget, parameters, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, null ); + super( mutatingTable, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings ); + this.whereFragment = null; } public TableUpdateStandard( - MutatingTableReference mutatingTable, + MutatingTableReference tableReference, MutationTarget mutationTarget, - List parameters, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings, + List parameters) { + this( tableReference, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters, null ); + } + + public TableUpdateStandard( + MutatingTableReference tableReference, + MutationTarget mutationTarget, + List valueBindings, + List keyRestrictionBindings, + List optLockRestrictionBindings, + List parameters, String whereFragment) { - super( mutatingTable, mutationTarget, parameters, valueBindings, keyRestrictionBindings, optLockRestrictionBindings ); + super( tableReference, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters ); this.whereFragment = whereFragment; } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpsert.java b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpsert.java index 2b4aa131a5..e9bd4b5269 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpsert.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableUpsert.java @@ -8,6 +8,7 @@ package org.hibernate.sql.model.internal; import java.util.List; import java.util.function.BiConsumer; +import java.util.function.Consumer; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.jdbc.Expectation; @@ -25,6 +26,8 @@ import org.hibernate.sql.model.ast.MutatingTableReference; import org.hibernate.sql.model.ast.RestrictedTableMutation; import org.hibernate.sql.model.ast.TableUpdate; +import static org.hibernate.sql.model.ast.AbstractTableUpdate.collectParameters; + /** * @apiNote Implements {@link TableUpdate} because it is fundamentally an update * @@ -40,16 +43,14 @@ public class TableUpsert MutationTarget mutationTarget, List valueBindings, List keyRestrictionBindings, - List optLockRestrictionBindings, - List parameters) { + List optLockRestrictionBindings) { this( mutatingTable, mutationTarget, "upsert for " + mutationTarget.getRolePath(), valueBindings, keyRestrictionBindings, - optLockRestrictionBindings, - parameters + optLockRestrictionBindings ); } @@ -59,9 +60,15 @@ public class TableUpsert String comment, List valueBindings, List keyRestrictionBindings, - List optLockRestrictionBindings, - List parameters) { - super( mutatingTable, mutationTarget, comment, keyRestrictionBindings, optLockRestrictionBindings, parameters ); + List optLockRestrictionBindings) { + super( + mutatingTable, + mutationTarget, + comment, + keyRestrictionBindings, + optLockRestrictionBindings, + collectParameters( valueBindings, keyRestrictionBindings, optLockRestrictionBindings ) + ); this.valueBindings = valueBindings; } @@ -90,6 +97,20 @@ public class TableUpsert return getMutatingTable().getTableMapping().getUpdateDetails().getExpectation(); } + @Override + public void forEachParameter(Consumer consumer) { + final BiConsumer intermediateConsumer = (index, binding) -> { + final ColumnValueParameter parameter = binding.getValueExpression().getParameter(); + if ( parameter != null ) { + consumer.accept( parameter ); + } + }; + + forEachThing( getValueBindings(), intermediateConsumer ); + forEachThing( getKeyBindings(), intermediateConsumer ); + forEachThing( getOptimisticLockBindings(), intermediateConsumer ); + } + public List getValueBindings() { return valueBindings; } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/OptionalTableUpdateOperation.java b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/OptionalTableUpdateOperation.java index 89d25613b8..2f049c2aa7 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/OptionalTableUpdateOperation.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/jdbc/OptionalTableUpdateOperation.java @@ -140,8 +140,9 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio } } else { - // there are some non-null values for the table - we need to update or insert the values + // there are some non-null values for the table - we need to update or insert the values. + // first, try the update and see if any row was affected final boolean wasUpdated; if ( valuesAnalysis.getTablesWithPreviousNonNullValues().contains( tableMapping ) ) { // either @@ -302,10 +303,7 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio private boolean performUpdate( JdbcValueBindings jdbcValueBindings, SharedSessionContractImplementor session) { - MODEL_MUTATION_LOGGER.tracef( - "#performUpdate(%s)", - tableMapping.getTableName() - ); + MODEL_MUTATION_LOGGER.tracef( "#performUpdate(%s)", tableMapping.getTableName() ); final TableUpdate tableUpdate; if ( tableMapping.getUpdateDetails() != null @@ -313,20 +311,20 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio tableUpdate = new TableUpdateCustomSql( new MutatingTableReference( tableMapping ), mutationTarget, - parameters, valueBindings, keyBindings, - optimisticLockBindings + optimisticLockBindings, + parameters ); } else { tableUpdate = new TableUpdateStandard( new MutatingTableReference( tableMapping ), mutationTarget, - parameters, valueBindings, keyBindings, - optimisticLockBindings + optimisticLockBindings, + parameters ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/customsql/CustomSqlGeneratedTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/customsql/CustomSqlGeneratedTest.java index 5af79907d3..c807984c02 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/customsql/CustomSqlGeneratedTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/customsql/CustomSqlGeneratedTest.java @@ -6,7 +6,6 @@ import org.hibernate.annotations.SQLInsert; import org.hibernate.annotations.SQLUpdate; import org.hibernate.testing.orm.junit.DomainModel; -import org.hibernate.testing.orm.junit.FailureExpected; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.Test; @@ -19,6 +18,7 @@ import jakarta.persistence.SecondaryTable; import jakarta.persistence.Table; import jakarta.persistence.Version; +import static org.assertj.core.api.Assertions.assertThat; import static org.hibernate.annotations.GenerationTime.ALWAYS; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -26,7 +26,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; @DomainModel(annotatedClasses = CustomSqlGeneratedTest.Custom.class) public class CustomSqlGeneratedTest { @Test - @FailureExpected( reason = "Change in expected order of columns in UPDATE SET clause causes problems with `@SQLUpdate`" ) public void testCustomSqlWithGenerated(SessionFactoryScope scope) { Custom c = new Custom(); c.name = "name"; @@ -41,8 +40,8 @@ public class CustomSqlGeneratedTest { cc.text = "more text"; s.flush(); cc = s.find(Custom.class, c.id); - assertEquals(cc.text, "MORE TEXT"); - assertEquals(cc.name, "EMAN"); + assertThat(cc.text ).isEqualTo( "MORE TEXT"); + assertThat( cc.name ).isEqualTo( "EMAN" ); s.remove(cc); s.flush(); cc = s.find(Custom.class, c.id); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/customsql/CustomSqlTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/customsql/CustomSqlTest.java index 92d8fea409..ae081effdd 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/customsql/CustomSqlTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/customsql/CustomSqlTest.java @@ -5,7 +5,6 @@ import org.hibernate.annotations.SQLInsert; import org.hibernate.annotations.SQLUpdate; import org.hibernate.testing.orm.junit.DomainModel; -import org.hibernate.testing.orm.junit.FailureExpected; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.Test; @@ -24,7 +23,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; @DomainModel(annotatedClasses = CustomSqlTest.Custom.class) public class CustomSqlTest { @Test - @FailureExpected( reason = "Change in expected order of columns in UPDATE SET clause causes problems with `@SQLUpdate`" ) public void testCustomSql(SessionFactoryScope scope) { Custom c = new Custom(); c.name = "name";