From 1c083a5863637d2c0db2da9f034b1c78307b4af5 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Wed, 30 Nov 2022 15:36:28 -0600 Subject: [PATCH] HHH-15393 - Improve write-paths to use mapping model --- .../insert/TableInsertReturningBuilder.java | 1 - .../AbstractCollectionPersister.java | 1 + .../collection/OneToManyPersister.java | 1 + .../entity/AbstractEntityPersister.java | 71 +++++----- .../entity/mutation/UpdateCoordinator.java | 6 + .../mutation/UpdateCoordinatorNoOp.java | 5 + .../mutation/UpdateCoordinatorStandard.java | 50 ++++---- .../sql/ast/spi/AbstractSqlAstTranslator.java | 16 +++ .../sql/model/ast/AbstractTableUpdate.java | 6 +- .../builder/AbstractTableInsertBuilder.java | 12 ++ .../builder/AbstractTableUpdateBuilder.java | 11 ++ .../builder/TableDeleteBuilderStandard.java | 13 ++ .../builder/TableInsertBuilderStandard.java | 39 +----- .../builder/TableUpdateBuilderStandard.java | 3 + .../model/internal/TableDeleteCustomSql.java | 3 +- .../model/internal/TableDeleteStandard.java | 3 +- .../model/internal/TableInsertStandard.java | 3 - .../model/internal/TableUpdateCustomSql.java | 6 +- .../model/internal/TableUpdateStandard.java | 11 +- .../jdbc/OptionalTableUpdateOperation.java | 5 +- .../orm/test/sql/StatementCommentTests.java | 121 ++++++++++++++++++ .../testing/orm/domain/helpdesk/Account.java | 2 + 22 files changed, 280 insertions(+), 109 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/sql/StatementCommentTests.java diff --git a/hibernate-core/src/main/java/org/hibernate/id/insert/TableInsertReturningBuilder.java b/hibernate-core/src/main/java/org/hibernate/id/insert/TableInsertReturningBuilder.java index 46cb32971f..f975265e85 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/insert/TableInsertReturningBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/id/insert/TableInsertReturningBuilder.java @@ -38,7 +38,6 @@ public class TableInsertReturningBuilder extends AbstractTableInsertBuilder { getMutatingTable(), getMutationTarget(), combine( getValueBindingList(), getKeyBindingList(), getLobValueBindingList() ), - true, Collections.singletonList( new ColumnReference( getMutatingTable(), identifierMapping ) ), getParameters() ); diff --git a/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java index c7cc406103..a347292c39 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java @@ -1785,6 +1785,7 @@ public abstract class AbstractCollectionPersister return (RestrictedTableMutation) new TableDeleteStandard( tableReference, this, + "one-shot delete for " + getRolePath(), keyRestrictionBindings, Collections.emptyList(), parameters 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 d69fc47ec4..aa4d3c7176 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 @@ -389,6 +389,7 @@ public class OneToManyPersister extends AbstractCollectionPersister { return new TableUpdateStandard( tableReference, this, + "one-shot delete for " + getRolePath(), valueBindings, keyRestrictionBindings, null, diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 5a7a11522b..059ce97f83 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -256,10 +256,10 @@ import org.hibernate.sql.results.graph.embeddable.EmbeddableResultGraphNode; import org.hibernate.sql.results.graph.entity.internal.EntityResultImpl; import org.hibernate.sql.results.internal.SqlSelectionImpl; import org.hibernate.stat.spi.StatisticsImplementor; -import org.hibernate.tuple.ValueGenerationStrategy; -import org.hibernate.tuple.InDatabaseValueGenerationStrategy; import org.hibernate.tuple.GenerationTiming; +import org.hibernate.tuple.InDatabaseValueGenerationStrategy; import org.hibernate.tuple.NonIdentifierAttribute; +import org.hibernate.tuple.ValueGenerationStrategy; import org.hibernate.tuple.entity.EntityBasedAssociationAttribute; import org.hibernate.tuple.entity.EntityMetamodel; import org.hibernate.type.AnyType; @@ -1934,6 +1934,10 @@ public abstract class AbstractEntityPersister @Override public Object forceVersionIncrement(Object id, Object currentVersion, SharedSessionContractImplementor session) { + if ( superMappingType != null ) { + return superMappingType.getEntityPersister().forceVersionIncrement( id, currentVersion, session ); + } + if ( !isVersioned() ) { throw new AssertionFailure( "cannot force version increment on non-versioned entity" ); } @@ -1944,6 +1948,7 @@ public abstract class AbstractEntityPersister throw new HibernateException( "LockMode.FORCE is currently not supported for generated version properties" ); } + final EntityVersionMapping versionMapping = getVersionMapping(); final Object nextVersion = getVersionJavaType().next( currentVersion, @@ -1960,36 +1965,38 @@ public abstract class AbstractEntityPersister ); } - // todo : cache this sql... - String versionIncrementString = generateVersionIncrementUpdateString(); - PreparedStatement st; - try { - st = session - .getJdbcCoordinator() - .getStatementPreparer() - .prepareStatement( versionIncrementString, false ); - try { - getVersionType().nullSafeSet( st, nextVersion, 1, session ); - getIdentifierType().nullSafeSet( st, id, 2, session ); - getVersionType().nullSafeSet( st, currentVersion, 2 + getIdentifierColumnSpan(), session ); - int rows = session.getJdbcCoordinator().getResultSetReturn().executeUpdate( st ); - if ( rows != 1 ) { - throw new StaleObjectStateException( getEntityName(), id ); - } - } - finally { - session.getJdbcCoordinator().getLogicalConnection().getResourceRegistry().release( st ); - session.getJdbcCoordinator().afterStatementExecution(); - } - } - catch (SQLException sqle) { - throw session.getJdbcServices().getSqlExceptionHelper().convert( - sqle, - "could not retrieve version: " + - MessageHelper.infoString( this, id, getFactory() ), - getVersionSelectString() - ); - } + updateCoordinator.forceVersionIncrement( id, currentVersion, nextVersion, session ); + +// // todo : cache this sql... +// String versionIncrementString = generateVersionIncrementUpdateString(); +// PreparedStatement st; +// try { +// st = session +// .getJdbcCoordinator() +// .getStatementPreparer() +// .prepareStatement( versionIncrementString, false ); +// try { +// getVersionType().nullSafeSet( st, nextVersion, 1, session ); +// getIdentifierType().nullSafeSet( st, id, 2, session ); +// getVersionType().nullSafeSet( st, currentVersion, 2 + getIdentifierColumnSpan(), session ); +// int rows = session.getJdbcCoordinator().getResultSetReturn().executeUpdate( st ); +// if ( rows != 1 ) { +// throw new StaleObjectStateException( getEntityName(), id ); +// } +// } +// finally { +// session.getJdbcCoordinator().getLogicalConnection().getResourceRegistry().release( st ); +// session.getJdbcCoordinator().afterStatementExecution(); +// } +// } +// catch (SQLException sqle) { +// throw session.getJdbcServices().getSqlExceptionHelper().convert( +// sqle, +// "could not retrieve version: " + +// MessageHelper.infoString( this, id, getFactory() ), +// getVersionSelectString() +// ); +// } return nextVersion; } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinator.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinator.java index 41e4e959a0..2f766b0960 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinator.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinator.java @@ -31,4 +31,10 @@ public interface UpdateCoordinator { int[] dirtyAttributeIndexes, boolean hasDirtyCollection, SharedSessionContractImplementor session); + + void forceVersionIncrement( + Object id, + Object currentVersion, + Object nextVersion, + SharedSessionContractImplementor session); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorNoOp.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorNoOp.java index 9408b457f1..75c6475f81 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorNoOp.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorNoOp.java @@ -31,4 +31,9 @@ public class UpdateCoordinatorNoOp implements UpdateCoordinator { public void coordinateUpdate(Object entity, Object id, Object rowId, Object[] values, Object oldVersion, Object[] incomingOldValues, int[] dirtyAttributeIndexes, boolean hasDirtyCollection, SharedSessionContractImplementor session) { // nothing to do } + + @Override + public void forceVersionIncrement(Object id, Object currentVersion, Object nextVersion, SharedSessionContractImplementor session) { + // nothing to do + } } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java index da6a81aafa..ded890f254 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java @@ -12,6 +12,7 @@ import java.util.List; import java.util.Locale; import java.util.Set; +import org.hibernate.HibernateException; import org.hibernate.Internal; import org.hibernate.Session; import org.hibernate.engine.OptimisticLockStyle; @@ -49,9 +50,9 @@ import org.hibernate.sql.model.ast.builder.TableUpdateBuilderSkipped; import org.hibernate.sql.model.ast.builder.TableUpdateBuilderStandard; import org.hibernate.sql.model.internal.MutationOperationGroupSingle; import org.hibernate.sql.model.jdbc.JdbcMutationOperation; -import org.hibernate.tuple.ValueGenerationStrategy; import org.hibernate.tuple.InDatabaseValueGenerationStrategy; import org.hibernate.tuple.InMemoryValueGenerationStrategy; +import org.hibernate.tuple.ValueGenerationStrategy; import org.hibernate.tuple.entity.EntityMetamodel; import static org.hibernate.engine.OptimisticLockStyle.ALL; @@ -102,6 +103,18 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple return ( entry == null ? entityPersister().isMutable() : entry.isModifiableEntity() ); } + @Override + public void forceVersionIncrement( + Object id, + Object currentVersion, + Object nextVersion, + SharedSessionContractImplementor session) { + if ( versionUpdateGroup == null ) { + throw new HibernateException( "Cannot force version increment relative to sub-type; use the root type" ); + } + doVersionUpdate( null, id, nextVersion, currentVersion, session ); + } + @Override public void coordinateUpdate( Object entity, @@ -606,6 +619,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple ); try { + //noinspection SuspiciousMethodCalls mutationExecutor.execute( entity, valuesAnalysis, @@ -799,6 +813,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple entity, valuesAnalysis, (tableMapping) -> { + //noinspection SuspiciousMethodCalls if ( tableMapping.isOptional() && !valuesAnalysis.tablesWithNonNullValues.contains( tableMapping ) ) { // the table is optional, and we have null values for all of its columns @@ -806,7 +821,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple return false; } - //noinspection RedundantIfStatement + //noinspection SuspiciousMethodCalls,RedundantIfStatement if ( !valuesAnalysis.tablesNeedingUpdate.contains( tableMapping ) ) { // nothing changed return false; @@ -842,6 +857,7 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple final RestrictedTableMutationBuilder tableUpdateBuilder; final MutatingTableReference tableReference = new MutatingTableReference( tableMapping ); + //noinspection SuspiciousMethodCalls if ( ! valuesAnalysis.tablesNeedingUpdate.contains( tableReference.getTableMapping() ) ) { // this table does not need updating tableUpdateBuilder = new TableUpdateBuilderSkipped( tableReference ); @@ -1371,26 +1387,6 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple null, valuesAnalysis, (position, attribute) -> valuesAnalysis.getAttributeAnalyses().get( position ).isDirty(), -// // optimistic locking (restriction) checker -// (position, attribute) -> { -// final OptimisticLockStyle optimisticLockStyle = entityPersister() -// .getEntityMetamodel() -// .getOptimisticLockStyle(); -// -// if ( optimisticLockStyle == ALL ) { -// return true; -// } -// -// if ( optimisticLockStyle == VERSION ) { -// final EntityVersionMapping versionMapping = entityPersister().getVersionMapping(); -// if ( versionMapping == null ) { -// return false; -// } -// return attribute == versionMapping.getVersionAttribute(); -// } -// -// return false; -// }, // session null ); @@ -1406,12 +1402,18 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple return null; } + if ( entityPersister().getSuperMappingType() != null ) { + return null; + } + final TableUpdateBuilderStandard updateBuilder = new TableUpdateBuilderStandard( entityPersister(), entityPersister().getIdentifierTableMapping(), factory() ); + updateBuilder.setSqlComment( "forced version increment for " + entityPersister().getRolePath() ); + updateBuilder.addValueColumn( versionMapping ); entityPersister().getIdentifierMapping().forEachSelectable( (selectionIndex, selectableMapping) -> { @@ -1439,10 +1441,6 @@ public class UpdateCoordinatorStandard extends AbstractMutationCoordinator imple @FunctionalInterface private interface InclusionChecker { boolean include(int position, SingularAttributeMapping attribute); - - default boolean reject(int position, SingularAttributeMapping attribute) { - return !include( position, attribute ); - } } @FunctionalInterface 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 2b7293a882..52653caf5d 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 @@ -7643,6 +7643,8 @@ public abstract class AbstractSqlAstTranslator implemen } private void renderInsertInto(TableInsertStandard tableInsert) { + applySqlComment( tableInsert.getMutationComment() ); + if ( tableInsert.getNumberOfValueBindings() == 0 ) { renderInsertIntoNoColumns( tableInsert ); return; @@ -7710,6 +7712,8 @@ public abstract class AbstractSqlAstTranslator implemen public void visitStandardTableUpdate(TableUpdateStandard tableUpdate) { getCurrentClauseStack().push( Clause.UPDATE ); try { + applySqlComment( tableUpdate.getMutationComment() ); + sqlBuffer.append( "update " ); appendSql( tableUpdate.getMutatingTable().getTableName() ); registerAffectedTable( tableUpdate.getMutatingTable().getTableName() ); @@ -7775,6 +7779,16 @@ public abstract class AbstractSqlAstTranslator implemen } } + private void applySqlComment(String comment) { + if ( sessionFactory.getSessionFactoryOptions().isCommentsEnabled() ) { + if ( comment != null ) { + appendSql( "/* " ); + appendSql( Dialect.escapeComment( comment ) ); + appendSql( " */" ); + } + } + } + @Override public void visitCustomTableUpdate(TableUpdateCustomSql tableUpdate) { assert sqlBuffer.toString().isEmpty(); @@ -7787,6 +7801,8 @@ public abstract class AbstractSqlAstTranslator implemen public void visitStandardTableDelete(TableDeleteStandard tableDelete) { getCurrentClauseStack().push( Clause.DELETE ); try { + applySqlComment( tableDelete.getMutationComment() ); + sqlBuffer.append( "delete from " ); appendSql( tableDelete.getMutatingTable().getTableName() ); registerAffectedTable( tableDelete.getMutatingTable().getTableName() ); 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 57f0d9f4fa..bb8d5a77bd 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 @@ -31,13 +31,14 @@ public abstract class AbstractTableUpdate public AbstractTableUpdate( MutatingTableReference mutatingTable, MutationTarget mutationTarget, + String sqlComment, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings) { super( mutatingTable, mutationTarget, - "update for " + mutationTarget.getNavigableRole(), + sqlComment, keyRestrictionBindings, optLockRestrictionBindings, collectParameters( valueBindings, keyRestrictionBindings, optLockRestrictionBindings ) @@ -49,6 +50,7 @@ public abstract class AbstractTableUpdate public AbstractTableUpdate( MutatingTableReference tableReference, MutationTarget mutationTarget, + String sqlComment, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings, @@ -56,7 +58,7 @@ public abstract class AbstractTableUpdate super( tableReference, mutationTarget, - "update for " + mutationTarget.getNavigableRole(), + sqlComment, keyRestrictionBindings, optLockRestrictionBindings, parameters 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 bb14fe1391..e70cdef01a 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 @@ -33,11 +33,14 @@ public abstract class AbstractTableInsertBuilder private final List parameters = new ArrayList<>(); + private String sqlComment; + public AbstractTableInsertBuilder( MutationTarget mutationTarget, TableMapping table, SessionFactoryImplementor sessionFactory) { super( MutationType.INSERT, mutationTarget, table, sessionFactory ); + this.sqlComment = "insert for " + mutationTarget.getRolePath(); } public AbstractTableInsertBuilder( @@ -45,6 +48,15 @@ public abstract class AbstractTableInsertBuilder MutatingTableReference tableReference, SessionFactoryImplementor sessionFactory) { super( MutationType.INSERT, mutationTarget, tableReference, sessionFactory ); + this.sqlComment = "insert for " + mutationTarget.getRolePath(); + } + + public String getSqlComment() { + return sqlComment; + } + + public void setSqlComment(String sqlComment) { + this.sqlComment = sqlComment; } protected List getKeyBindingList() { 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 f64a52803f..4bade25276 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 @@ -32,11 +32,14 @@ public abstract class AbstractTableUpdateBuilder private final List valueBindings = new ArrayList<>(); private List lobValueBindings; + private String sqlComment; + public AbstractTableUpdateBuilder( MutationTarget mutationTarget, TableMapping tableMapping, SessionFactoryImplementor sessionFactory) { super( MutationType.UPDATE, mutationTarget, tableMapping, sessionFactory ); + this.sqlComment = "update for " + mutationTarget.getRolePath(); } public AbstractTableUpdateBuilder( @@ -46,6 +49,14 @@ public abstract class AbstractTableUpdateBuilder super( MutationType.UPDATE, mutationTarget, tableReference, sessionFactory ); } + public String getSqlComment() { + return sqlComment; + } + + public void setSqlComment(String sqlComment) { + this.sqlComment = sqlComment; + } + /** * The bindings for each key restriction (WHERE clause). * 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 59c52dc8f4..24b8dda769 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 @@ -34,6 +34,8 @@ public class TableDeleteBuilderStandard private final List parameters = new ArrayList<>(); + private String sqlComment; + public TableDeleteBuilderStandard( MutationTarget mutationTarget, TableMapping table, @@ -48,6 +50,15 @@ public class TableDeleteBuilderStandard super( MutationType.DELETE, mutationTarget, tableReference, sessionFactory ); this.isCustomSql = tableReference.getTableMapping().getDeleteDetails().getCustomSql() != null; + this.sqlComment = "delete for " + mutationTarget.getRolePath(); + } + + public String getSqlComment() { + return sqlComment; + } + + public void setSqlComment(String sqlComment) { + this.sqlComment = sqlComment; } @Override @@ -76,6 +87,7 @@ public class TableDeleteBuilderStandard return new TableDeleteCustomSql( getMutatingTable(), getMutationTarget(), + sqlComment, getKeyRestrictionBindings(), getOptimisticLockBindings(), getParameters() @@ -85,6 +97,7 @@ public class TableDeleteBuilderStandard return new TableDeleteStandard( getMutatingTable(), getMutationTarget(), + sqlComment, getKeyRestrictionBindings(), getOptimisticLockBindings(), getParameters() diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableInsertBuilderStandard.java b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableInsertBuilderStandard.java index 6650c49c63..0b44b8fe7f 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableInsertBuilderStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableInsertBuilderStandard.java @@ -6,14 +6,9 @@ */ package org.hibernate.sql.model.ast.builder; -import java.util.ArrayList; import java.util.Collections; -import java.util.List; -import org.hibernate.dialect.Dialect; import org.hibernate.engine.spi.SessionFactoryImplementor; -import org.hibernate.metamodel.mapping.SelectableMapping; -import org.hibernate.sql.ast.tree.expression.ColumnReference; import org.hibernate.sql.model.MutationTarget; import org.hibernate.sql.model.TableMapping; import org.hibernate.sql.model.ast.MutatingTableReference; @@ -27,51 +22,24 @@ import org.hibernate.sql.model.internal.TableInsertStandard; * @author Steve Ebersole */ public class TableInsertBuilderStandard extends AbstractTableInsertBuilder { - private final boolean returningGeneratedKeys; private final boolean isCustomSql; - private List returningColumnsList; public TableInsertBuilderStandard( - MutationTarget mutationTarget, + MutationTarget mutationTarget, TableMapping table, SessionFactoryImplementor sessionFactory) { super( mutationTarget, table, sessionFactory ); - - final Dialect dialect = getJdbcServices().getDialect(); - this.returningGeneratedKeys = dialect.getDefaultUseGetGeneratedKeys(); - this.isCustomSql = table.getInsertDetails().getCustomSql() != null; } public TableInsertBuilderStandard( - MutationTarget mutationTarget, + MutationTarget mutationTarget, MutatingTableReference tableReference, SessionFactoryImplementor sessionFactory) { super( mutationTarget, tableReference, sessionFactory ); - - final Dialect dialect = getJdbcServices().getDialect(); - this.returningGeneratedKeys = dialect.getDefaultUseGetGeneratedKeys(); - this.isCustomSql = tableReference.getTableMapping().getInsertDetails().getCustomSql() != null; } - public boolean isReturningGeneratedKeys() { - return returningGeneratedKeys; - } - - public List getReturningColumns() { - return returningColumnsList == null ? Collections.emptyList() : returningColumnsList; - } - - public ColumnReference addReturningColumn(SelectableMapping selectableMapping) { - final ColumnReference columnReference = new ColumnReference( (String) null, selectableMapping, null ); - if ( returningColumnsList == null ) { - returningColumnsList = new ArrayList<>(); - } - returningColumnsList.add( columnReference ); - return columnReference; - } - @Override public TableInsert buildMutation() { if ( isCustomSql ) { @@ -87,8 +55,7 @@ public class TableInsertBuilderStandard extends AbstractTableInsertBuilder { getMutatingTable(), getMutationTarget(), combine( getValueBindingList(), getKeyBindingList(), getLobValueBindingList() ), - returningGeneratedKeys, - returningColumnsList, + Collections.emptyList(), getParameters() ); } 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 3d4dd640fd..dc8de1cf87 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 @@ -27,6 +27,7 @@ import org.hibernate.sql.model.internal.TableUpsert; * @author Steve Ebersole */ public class TableUpdateBuilderStandard extends AbstractTableUpdateBuilder { + public TableUpdateBuilderStandard( MutationTarget mutationTarget, TableMapping tableMapping, @@ -53,6 +54,7 @@ public class TableUpdateBuilderStandard extends Abs return (RestrictedTableMutation) new TableUpdateCustomSql( getMutatingTable(), getMutationTarget(), + getSqlComment(), valueBindings, getKeyRestrictionBindings(), getOptimisticLockBindings() @@ -72,6 +74,7 @@ public class TableUpdateBuilderStandard extends Abs return (RestrictedTableMutation) new TableUpdateStandard( getMutatingTable(), getMutationTarget(), + getSqlComment(), valueBindings, getKeyRestrictionBindings(), getOptimisticLockBindings() diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableDeleteCustomSql.java b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableDeleteCustomSql.java index 4208f13be1..9bf830d22b 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableDeleteCustomSql.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableDeleteCustomSql.java @@ -28,10 +28,11 @@ public class TableDeleteCustomSql extends AbstractTableDelete implements CustomS public TableDeleteCustomSql( MutatingTableReference mutatingTable, MutationTarget mutationTarget, + String sqlComment, List keyRestrictionBindings, List optLockRestrictionBindings, List parameters) { - super( mutatingTable, mutationTarget, keyRestrictionBindings, optLockRestrictionBindings, parameters ); + super( mutatingTable, mutationTarget, sqlComment, keyRestrictionBindings, optLockRestrictionBindings, parameters ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableDeleteStandard.java b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableDeleteStandard.java index 1cbdcca4f1..ab19d38120 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableDeleteStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableDeleteStandard.java @@ -22,10 +22,11 @@ public class TableDeleteStandard extends AbstractTableDelete { public TableDeleteStandard( MutatingTableReference mutatingTable, MutationTarget mutationTarget, + String sqlComment, List keyRestrictionBindings, List optLockRestrictionBindings, List parameters) { - super( mutatingTable, mutationTarget, keyRestrictionBindings, optLockRestrictionBindings, parameters ); + super( mutatingTable, mutationTarget, sqlComment, keyRestrictionBindings, optLockRestrictionBindings, parameters ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableInsertStandard.java b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableInsertStandard.java index 9b1298f768..bb0dcf54a9 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableInsertStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/model/internal/TableInsertStandard.java @@ -21,18 +21,15 @@ import org.hibernate.sql.model.ast.MutatingTableReference; * @author Steve Ebersole */ public class TableInsertStandard extends AbstractTableInsert { - private final boolean returnGeneratedKeys; private final List returningColumns; public TableInsertStandard( MutatingTableReference mutatingTable, MutationTarget mutationTarget, List valueBindings, - boolean returnGeneratedKeys, List returningColumns, List parameters) { super( mutatingTable, mutationTarget, parameters, valueBindings ); - this.returnGeneratedKeys = returnGeneratedKeys; this.returningColumns = returningColumns; } 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 94c8452949..35412e0896 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,20 +30,22 @@ public class TableUpdateCustomSql public TableUpdateCustomSql( MutatingTableReference mutatingTable, MutationTarget mutationTarget, + String sqlComment, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings) { - super( mutatingTable, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings ); + super( mutatingTable, mutationTarget, sqlComment, valueBindings, keyRestrictionBindings, optLockRestrictionBindings ); } public TableUpdateCustomSql( MutatingTableReference mutatingTable, MutationTarget mutationTarget, + String sqlComment, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings, List parameters) { - super( mutatingTable, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters ); + super( mutatingTable, mutationTarget, sqlComment, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters ); } @Override 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 28d616765f..c6c788c537 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,38 +25,41 @@ public class TableUpdateStandard extends AbstractTableUpdate mutationTarget, + String sqlComment, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings) { - super( mutatingTable, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings ); + super( mutatingTable, mutationTarget, sqlComment, valueBindings, keyRestrictionBindings, optLockRestrictionBindings ); this.whereFragment = null; } public TableUpdateStandard( MutatingTableReference tableReference, MutationTarget mutationTarget, + String sqlComment, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings, List parameters) { - this( tableReference, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters, null ); + this( tableReference, mutationTarget, sqlComment, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters, null ); } public TableUpdateStandard( MutatingTableReference tableReference, MutationTarget mutationTarget, + String sqlComment, List valueBindings, List keyRestrictionBindings, List optLockRestrictionBindings, List parameters, String whereFragment) { - super( tableReference, mutationTarget, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters ); + super( tableReference, mutationTarget, sqlComment, valueBindings, keyRestrictionBindings, optLockRestrictionBindings, parameters ); this.whereFragment = whereFragment; } @Override public boolean isCustomSql() { - return true; + return false; } @Override 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 2f049c2aa7..2c617cf43c 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 @@ -271,6 +271,7 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio tableDelete = new TableDeleteCustomSql( new MutatingTableReference( tableMapping ), getMutationTarget(), + "upsert delete for " + mutationTarget.getRolePath(), keyBindings, optimisticLockBindings, parameters @@ -280,6 +281,7 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio tableDelete = new TableDeleteStandard( new MutatingTableReference( tableMapping ), getMutationTarget(), + "upsert delete for " + mutationTarget.getRolePath(), keyBindings, optimisticLockBindings, parameters @@ -311,6 +313,7 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio tableUpdate = new TableUpdateCustomSql( new MutatingTableReference( tableMapping ), mutationTarget, + "upsert update for " + mutationTarget.getRolePath(), valueBindings, keyBindings, optimisticLockBindings, @@ -321,6 +324,7 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio tableUpdate = new TableUpdateStandard( new MutatingTableReference( tableMapping ), mutationTarget, + "upsert update for " + mutationTarget.getRolePath(), valueBindings, keyBindings, optimisticLockBindings, @@ -427,7 +431,6 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio new MutatingTableReference( tableMapping ), getMutationTarget(), CollectionHelper.combine( valueBindings, keyBindings ), - false, Collections.emptyList(), parameters ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/StatementCommentTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/StatementCommentTests.java new file mode 100644 index 0000000000..af40d50323 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/StatementCommentTests.java @@ -0,0 +1,121 @@ +/* + * 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.orm.test.sql; + +import org.hibernate.LockMode; +import org.hibernate.cfg.AvailableSettings; + +import org.hibernate.testing.jdbc.SQLStatementInspector; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.Setting; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Basic; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Table; +import jakarta.persistence.Version; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Steve Ebersole + */ +@ServiceRegistry( + settings = @Setting( name = AvailableSettings.USE_SQL_COMMENTS, value = "true" ) +) +@DomainModel( annotatedClasses = StatementCommentTests.VersionedEntity.class ) +@SessionFactory( useCollectingStatementInspector = true ) +public class StatementCommentTests { + @Test + public void testEntityMutationComments(SessionFactoryScope scope) { + final SQLStatementInspector inspector = scope.getCollectingStatementInspector(); + inspector.clear(); + + // insert + scope.inTransaction( (session) -> { + session.persist( new VersionedEntity( 1, "tbd" ) ); + } ); + checkEntityComments( inspector ); + + // update + scope.inTransaction( (session) -> { + final VersionedEntity entity = session.find( VersionedEntity.class, 1 ); + inspector.clear(); + entity.setName( "The One" ); + } ); + checkEntityComments( inspector ); + + // forced version increment + scope.inTransaction( (session) -> { + final VersionedEntity entity = session.find( VersionedEntity.class, 1 ); + inspector.clear(); + session.lock( entity, LockMode.OPTIMISTIC_FORCE_INCREMENT ); + } ); + checkEntityComments( inspector ); + + // delete + scope.inTransaction( (session) -> { + final VersionedEntity entity = session.find( VersionedEntity.class, 1 ); + inspector.clear(); + session.remove( entity ); + } ); + checkEntityComments( inspector ); + } + + private void checkEntityComments(SQLStatementInspector inspector) { + assertThat( inspector.getSqlQueries() ).hasSize( 1 ); + assertThat( inspector.getSqlQueries().get( 0 ) ).contains( "VersionedEntity */" ); + } + + @AfterEach + public void dropTestData(SessionFactoryScope scope) { + scope.inTransaction( (session) -> { + session.createMutationQuery( "delete VersionedEntity" ).executeUpdate(); + } ); + } + + @Entity( name = "VersionedEntity" ) + @Table( name = "entity_table" ) + public static class VersionedEntity { + @Id + private Integer id; + @Version + private Integer version; + @Basic + private String name; + + protected VersionedEntity() { + // for use by Hibernate + } + + public VersionedEntity(Integer id, String name) { + this.id = id; + this.name = name; + } + + public Integer getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Integer getVersion() { + return version; + } + } +} diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/helpdesk/Account.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/helpdesk/Account.java index bbd53a3fe9..5b55a6fb5a 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/helpdesk/Account.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/helpdesk/Account.java @@ -13,11 +13,13 @@ import jakarta.persistence.Entity; import jakarta.persistence.EnumType; import jakarta.persistence.Enumerated; import jakarta.persistence.Id; +import jakarta.persistence.Table; /** * @author Steve Ebersole */ @Entity +@Table( name = "user_accounts" ) public class Account { private Integer id;