Skip to content

Commit 7ddd34a

Browse files
committed
HHH-18217 Properly handle empty value bindings for merge operations
1 parent 2abbaad commit 7ddd34a

17 files changed

+252
-70
lines changed

hibernate-core/src/main/java/org/hibernate/dialect/MySQLDeleteOrUpsertOperation.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
/**
2323
* @author Jan Schatteman
2424
*/
25+
@Deprecated(forRemoval = true)
2526
public class MySQLDeleteOrUpsertOperation extends DeleteOrUpsertOperation {
2627

2728
private Expectation customExpectation;

hibernate-core/src/main/java/org/hibernate/dialect/sql/ast/MariaDBSqlAstTranslator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ INSERT INTO employees (id, name, salary, version)
416416
salary = values(salary)
417417
*/
418418
@Override
419-
protected void renderUpdatevalue(ColumnValueBinding columnValueBinding) {
419+
protected void renderUpdateValue(ColumnValueBinding columnValueBinding) {
420420
appendSql( "values(" );
421421
appendSql( columnValueBinding.getColumnReference().getColumnExpression() );
422422
appendSql( ")" );

hibernate-core/src/main/java/org/hibernate/dialect/sql/ast/MySQLSqlAstTranslator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ protected void renderNewRowAlias() {
449449
}
450450

451451
@Override
452-
protected void renderUpdatevalue(ColumnValueBinding columnValueBinding) {
452+
protected void renderUpdateValue(ColumnValueBinding columnValueBinding) {
453453
renderAlias();
454454
appendSql( "." );
455455
appendSql( columnValueBinding.getColumnReference().getColumnExpression() );

hibernate-core/src/main/java/org/hibernate/dialect/sql/ast/SqlAstTranslatorWithOnDuplicateKeyUpdate.java

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,20 @@
55
package org.hibernate.dialect.sql.ast;
66

77

8-
import org.hibernate.dialect.MySQLDeleteOrUpsertOperation;
8+
import org.hibernate.StaleStateException;
99
import org.hibernate.engine.spi.SessionFactoryImplementor;
10+
import org.hibernate.jdbc.Expectation;
1011
import org.hibernate.persister.entity.mutation.EntityTableMapping;
1112
import org.hibernate.sql.ast.spi.SqlAstTranslatorWithUpsert;
1213
import org.hibernate.sql.ast.tree.Statement;
1314
import org.hibernate.sql.exec.spi.JdbcOperation;
1415
import org.hibernate.sql.model.MutationOperation;
1516
import org.hibernate.sql.model.ast.ColumnValueBinding;
1617
import org.hibernate.sql.model.internal.OptionalTableUpdate;
18+
import org.hibernate.sql.model.jdbc.DeleteOrUpsertOperation;
1719
import org.hibernate.sql.model.jdbc.UpsertOperation;
1820

21+
import java.sql.PreparedStatement;
1922
import java.util.List;
2023

2124
/**
@@ -37,17 +40,31 @@ public MutationOperation createMergeOperation(OptionalTableUpdate optionalTableU
3740
optionalTableUpdate.getMutatingTable().getTableMapping(),
3841
optionalTableUpdate.getMutationTarget(),
3942
getSql(),
43+
new MySQLRowCountExpectation(),
4044
getParameterBinders()
4145
);
4246

43-
return new MySQLDeleteOrUpsertOperation(
47+
return new DeleteOrUpsertOperation(
4448
optionalTableUpdate.getMutationTarget(),
4549
(EntityTableMapping) optionalTableUpdate.getMutatingTable().getTableMapping(),
4650
upsertOperation,
4751
optionalTableUpdate
4852
);
4953
}
5054

55+
private static class MySQLRowCountExpectation implements Expectation {
56+
@Override
57+
public final void verifyOutcome(int rowCount, PreparedStatement statement, int batchPosition, String sql) {
58+
if ( rowCount > 2 ) {
59+
throw new StaleStateException(
60+
"Unexpected row count"
61+
+ " (the expected row count for an ON DUPLICATE KEY UPDATE statement should be either 0, 1 or 2 )"
62+
+ " [" + sql + "]"
63+
);
64+
}
65+
}
66+
}
67+
5168
@Override
5269
protected void renderUpsertStatement(OptionalTableUpdate optionalTableUpdate) {
5370
renderInsertInto( optionalTableUpdate );
@@ -56,34 +73,39 @@ protected void renderUpsertStatement(OptionalTableUpdate optionalTableUpdate) {
5673
}
5774

5875
protected void renderInsertInto(OptionalTableUpdate optionalTableUpdate) {
59-
appendSql( "insert into " );
76+
if ( optionalTableUpdate.getValueBindings().isEmpty() ) {
77+
appendSql( "insert ignore into " );
78+
}
79+
else {
80+
appendSql( "insert into " );
81+
}
6082
appendSql( optionalTableUpdate.getMutatingTable().getTableName() );
61-
appendSql( " (" );
83+
appendSql( " " );
6284

6385
final List<ColumnValueBinding> keyBindings = optionalTableUpdate.getKeyBindings();
86+
char separator = '(';
6487
for ( ColumnValueBinding keyBinding : keyBindings ) {
88+
appendSql( separator );
6589
appendSql( keyBinding.getColumnReference().getColumnExpression() );
66-
appendSql( ',' );
90+
separator = ',';
6791
}
6892

6993
optionalTableUpdate.forEachValueBinding( (columnPosition, columnValueBinding) -> {
70-
appendSql( columnValueBinding.getColumnReference().getColumnExpression() );
71-
if ( columnPosition != optionalTableUpdate.getValueBindings().size() - 1 ) {
72-
appendSql( ',' );
73-
}
94+
appendSql( ',' );
95+
appendSql( columnValueBinding.getColumnReference().getColumnExpression() );
7496
} );
7597

76-
appendSql( ") values (" );
98+
appendSql( ") values " );
7799

100+
separator = '(';
78101
for ( ColumnValueBinding keyBinding : keyBindings ) {
102+
appendSql( separator );
79103
keyBinding.getValueExpression().accept( this );
80-
appendSql( ',' );
104+
separator = ',';
81105
}
82106

83107
optionalTableUpdate.forEachValueBinding( (columnPosition, columnValueBinding) -> {
84-
if ( columnPosition > 0 ) {
85-
appendSql( ',' );
86-
}
108+
appendSql( ',' );
87109
columnValueBinding.getValueExpression().accept( this );
88110
} );
89111
appendSql(") ");
@@ -94,19 +116,21 @@ protected void renderNewRowAlias() {
94116
}
95117

96118
protected void renderOnDuplicateKeyUpdate(OptionalTableUpdate optionalTableUpdate) {
97-
appendSql( "on duplicate key update " );
98-
optionalTableUpdate.forEachValueBinding( (columnPosition, columnValueBinding) -> {
99-
final String columnName = columnValueBinding.getColumnReference().getColumnExpression();
100-
if ( columnPosition > 0 ) {
101-
appendSql( ',' );
102-
}
103-
appendSql( columnName );
104-
append( " = " );
105-
renderUpdatevalue( columnValueBinding );
106-
} );
119+
if ( !optionalTableUpdate.getValueBindings().isEmpty() ) {
120+
appendSql( "on duplicate key update " );
121+
optionalTableUpdate.forEachValueBinding( (columnPosition, columnValueBinding) -> {
122+
final String columnName = columnValueBinding.getColumnReference().getColumnExpression();
123+
if ( columnPosition > 0 ) {
124+
appendSql( ',' );
125+
}
126+
appendSql( columnName );
127+
append( " = " );
128+
renderUpdateValue( columnValueBinding );
129+
} );
130+
}
107131
}
108132

109-
protected void renderUpdatevalue(ColumnValueBinding columnValueBinding) {
133+
protected void renderUpdateValue(ColumnValueBinding columnValueBinding) {
110134
}
111135

112136
}

hibernate-core/src/main/java/org/hibernate/jdbc/Expectation.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,31 @@ protected int expectedRowCount() {
179179
}
180180
}
181181

182+
/**
183+
* Row count checking. A row count is an integer value returned by
184+
* {@link java.sql.PreparedStatement#executeUpdate()} or
185+
* {@link java.sql.Statement#executeBatch()}. The row count is checked
186+
* against an expected value, but is also allowed to be 0.
187+
* For example, the expected row count for an {@code UPSERT} statement is 0 or 1.
188+
*/
189+
class OptionalRowCount implements Expectation {
190+
@Override
191+
public final void verifyOutcome(int rowCount, PreparedStatement statement, int batchPosition, String sql) {
192+
if ( rowCount != 0 ) {
193+
if ( batchPosition < 0 ) {
194+
checkNonBatched( expectedRowCount(), rowCount, sql );
195+
}
196+
else {
197+
checkBatched( expectedRowCount(), rowCount, batchPosition, sql );
198+
}
199+
}
200+
}
201+
202+
protected int expectedRowCount() {
203+
return 1;
204+
}
205+
}
206+
182207
/**
183208
* Essentially identical to {@link RowCount} except that the row count
184209
* is obtained via an output parameter of a {@linkplain CallableStatement

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3461,14 +3461,7 @@ protected UpdateCoordinator buildUpdateCoordinator() {
34613461
}
34623462

34633463
protected UpdateCoordinator buildMergeCoordinator() {
3464-
// we only have updates to issue for entities with one or more singular attributes
3465-
for ( int i = 0; i < attributeMappings.size(); i++ ) {
3466-
if ( attributeMappings.get( i ) instanceof SingularAttributeMapping ) {
3467-
return new MergeCoordinator( this, factory );
3468-
}
3469-
}
3470-
// otherwise, nothing to update
3471-
return new UpdateCoordinatorNoOp( this );
3464+
return new MergeCoordinator( this, factory );
34723465
}
34733466

34743467
protected DeleteCoordinator buildDeleteCoordinator() {

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/MergeCoordinator.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package org.hibernate.persister.entity.mutation;
66

77
import org.hibernate.engine.spi.SessionFactoryImplementor;
8+
import org.hibernate.engine.spi.SharedSessionContractImplementor;
89
import org.hibernate.persister.entity.EntityPersister;
910
import org.hibernate.sql.model.MutationOperation;
1011
import org.hibernate.sql.model.ast.builder.AbstractTableUpdateBuilder;
@@ -26,4 +27,49 @@ protected <O extends MutationOperation> AbstractTableUpdateBuilder<O> newTableUp
2627
return new TableMergeBuilder<>( entityPersister(), tableMapping, factory() );
2728
}
2829

30+
@Override
31+
protected UpdateValuesAnalysisImpl analyzeUpdateValues(
32+
Object entity,
33+
Object[] values,
34+
Object oldVersion,
35+
Object[] oldValues,
36+
int[] dirtyAttributeIndexes,
37+
InclusionChecker inclusionChecker,
38+
InclusionChecker lockingChecker,
39+
InclusionChecker dirtinessChecker,
40+
Object rowId,
41+
boolean forceDynamicUpdate,
42+
SharedSessionContractImplementor session) {
43+
final UpdateValuesAnalysisImpl updateValuesAnalysis = super.analyzeUpdateValues(
44+
entity,
45+
values,
46+
oldVersion,
47+
oldValues,
48+
dirtyAttributeIndexes,
49+
inclusionChecker,
50+
lockingChecker,
51+
dirtinessChecker,
52+
rowId,
53+
forceDynamicUpdate,
54+
session
55+
);
56+
if ( oldValues == null ) {
57+
final TableSet tablesNeedingUpdate = updateValuesAnalysis.getTablesNeedingUpdate();
58+
final TableSet tablesWithNonNullValues = updateValuesAnalysis.getTablesWithNonNullValues();
59+
final TableSet tablesWithPreviousNonNullValues = updateValuesAnalysis.getTablesWithPreviousNonNullValues();
60+
for ( EntityTableMapping tableMapping : entityPersister().getTableMappings() ) {
61+
// Need to upsert into all non-optional table mappings
62+
if ( !tableMapping.isOptional() ) {
63+
// If the table was previously not needing an update, remove it from tablesWithPreviousNonNullValues
64+
// to avoid triggering a delete-statement for this operation
65+
if ( !tablesNeedingUpdate.contains( tableMapping ) ) {
66+
tablesWithPreviousNonNullValues.remove( tableMapping );
67+
}
68+
tablesNeedingUpdate.add( tableMapping );
69+
tablesWithNonNullValues.add( tableMapping );
70+
}
71+
}
72+
}
73+
return updateValuesAnalysis;
74+
}
2975
}

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/TableSet.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ public void add(final TableMapping tableMapping) {
3434
bits.set( tableMapping.getRelativePosition() );
3535
}
3636

37+
public void remove(final TableMapping tableMapping) {
38+
if ( bits != null ) {
39+
assert addForChecks( tableMapping );
40+
bits.set( tableMapping.getRelativePosition(), false );
41+
}
42+
}
43+
3744
public boolean isEmpty() {
3845
return bits == null;
3946
}

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/UpdateCoordinatorStandard.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ protected boolean[] getPropertiesToUpdate(final int[] dirtyProperties, final boo
600600
}
601601
}
602602

603-
private UpdateValuesAnalysisImpl analyzeUpdateValues(
603+
protected UpdateValuesAnalysisImpl analyzeUpdateValues(
604604
Object entity,
605605
Object[] values,
606606
Object oldVersion,
@@ -1018,7 +1018,10 @@ protected BatchKey getVersionUpdateBatchkey(){
10181018
}
10191019

10201020
private boolean resultCheck(
1021-
Object id, PreparedStatementDetails statementDetails, int affectedRowCount, int batchPosition) {
1021+
Object id,
1022+
PreparedStatementDetails statementDetails,
1023+
int affectedRowCount,
1024+
int batchPosition) {
10221025
return identifiedResultsCheck(
10231026
statementDetails,
10241027
affectedRowCount,

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

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import org.hibernate.engine.spi.SessionFactoryImplementor;
1010
import org.hibernate.internal.util.StringHelper;
11+
import org.hibernate.jdbc.Expectation;
1112
import org.hibernate.sql.ast.tree.Statement;
1213
import org.hibernate.sql.exec.spi.JdbcOperation;
1314
import org.hibernate.sql.model.ast.ColumnValueBinding;
@@ -44,6 +45,10 @@ public MergeOperation createMergeOperation(OptionalTableUpdate optionalTableUpda
4445
optionalTableUpdate.getMutatingTable().getTableMapping(),
4546
optionalTableUpdate.getMutationTarget(),
4647
getSql(),
48+
// Without value bindings, the upsert may have an update count of 0
49+
optionalTableUpdate.getValueBindings().isEmpty()
50+
? new Expectation.OptionalRowCount()
51+
: new Expectation.RowCount(),
4752
getParameterBinders()
4853
);
4954
}
@@ -232,16 +237,18 @@ protected void renderMergeUpdate(OptionalTableUpdate optionalTableUpdate) {
232237
final List<ColumnValueBinding> valueBindings = optionalTableUpdate.getValueBindings();
233238
final List<ColumnValueBinding> optimisticLockBindings = optionalTableUpdate.getOptimisticLockBindings();
234239

235-
renderWhenMatched( optimisticLockBindings );
236-
appendSql( " then update set " );
237-
for ( int i = 0; i < valueBindings.size(); i++ ) {
238-
final ColumnValueBinding binding = valueBindings.get( i );
239-
if ( i > 0 ) {
240-
appendSql( ", " );
240+
if ( !valueBindings.isEmpty() ) {
241+
renderWhenMatched( optimisticLockBindings );
242+
appendSql( " then update set " );
243+
for ( int i = 0; i < valueBindings.size(); i++ ) {
244+
final ColumnValueBinding binding = valueBindings.get( i );
245+
if ( i > 0 ) {
246+
appendSql( ", " );
247+
}
248+
binding.getColumnReference().appendColumnForWrite( this, null );
249+
appendSql( "=" );
250+
binding.getColumnReference().appendColumnForWrite( this, "s" );
241251
}
242-
binding.getColumnReference().appendColumnForWrite( this, null );
243-
appendSql( "=" );
244-
binding.getColumnReference().appendColumnForWrite( this, "s" );
245252
}
246253
}
247254

0 commit comments

Comments
 (0)