From 8436326fc5f3dd410ea72773e44080900d0d9dca Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Tue, 16 Jul 2024 16:23:18 +0200 Subject: [PATCH] HHH-17949 Fix upsert handling when optimistic locking is involved --- .../dialect/SqlAstTranslatorWithUpsert.java | 17 ++++++++++ .../jdbc/OptionalTableUpdateOperation.java | 33 ++++++++++--------- .../test/stateless/UpsertVersionedTest.java | 13 ++------ 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SqlAstTranslatorWithUpsert.java b/hibernate-core/src/main/java/org/hibernate/dialect/SqlAstTranslatorWithUpsert.java index 50988e8adc..6f9a9f67f6 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SqlAstTranslatorWithUpsert.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SqlAstTranslatorWithUpsert.java @@ -191,6 +191,7 @@ public class SqlAstTranslatorWithUpsert extends Abstrac protected void renderMergeUpdate(OptionalTableUpdate optionalTableUpdate) { final List valueBindings = optionalTableUpdate.getValueBindings(); + final List optimisticLockBindings = optionalTableUpdate.getOptimisticLockBindings(); appendSql( " when matched then update set " ); for ( int i = 0; i < valueBindings.size(); i++ ) { @@ -202,5 +203,21 @@ public class SqlAstTranslatorWithUpsert extends Abstrac appendSql( "=" ); binding.getColumnReference().appendColumnForWrite( this, "s" ); } + renderMatchedWhere( optimisticLockBindings ); + } + + private void renderMatchedWhere(List optimisticLockBindings) { + if ( !optimisticLockBindings.isEmpty() ) { + appendSql( " where " ); + for (int i = 0; i < optimisticLockBindings.size(); i++) { + final ColumnValueBinding binding = optimisticLockBindings.get( i ); + if ( i>0 ) { + appendSql(" and "); + } + binding.getColumnReference().appendColumnForWrite( this, "t" ); + appendSql("="); + binding.getValueExpression().accept( this ); + } + } } } 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 25fa3ef0cf..30d35748c9 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 @@ -386,21 +386,24 @@ public class OptionalTableUpdateOperation implements SelfExecutingUpdateOperatio final BindingGroup bindingGroup = jdbcValueBindings.getBindingGroup( tableMapping.getTableName() ); if ( bindingGroup != null ) { - bindingGroup.forEachBinding( (binding) -> { - try { - binding.getValueBinder().bind( - insertStatement, - binding.getValue(), - binding.getPosition(), - session - ); - } - catch (SQLException e) { - throw session.getJdbcServices().getSqlExceptionHelper().convert( - e, - "Unable to bind parameter for upsert insert", - jdbcInsert.getSqlString() - ); + bindingGroup.forEachBinding( binding -> { + // Skip parameter bindings for e.g. optimistic version check + if ( binding.getPosition() <= jdbcInsert.getParameterBinders().size() ) { + try { + binding.getValueBinder().bind( + insertStatement, + binding.getValue(), + binding.getPosition(), + session + ); + } + catch (SQLException e) { + throw session.getJdbcServices().getSqlExceptionHelper().convert( + e, + "Unable to bind parameter for upsert insert", + jdbcInsert.getSqlString() + ); + } } } ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java index 669150a20f..07921e8e92 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/stateless/UpsertVersionedTest.java @@ -3,11 +3,7 @@ package org.hibernate.orm.test.stateless; import jakarta.persistence.Entity; import jakarta.persistence.Id; import jakarta.persistence.Version; -import org.hibernate.dialect.H2Dialect; -import org.hibernate.dialect.PostgreSQLDialect; -import org.hibernate.dialect.SQLServerDialect; import org.hibernate.testing.orm.junit.DomainModel; -import org.hibernate.testing.orm.junit.RequiresDialect; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.Test; @@ -17,9 +13,6 @@ import static org.junit.Assert.assertEquals; @SessionFactory @DomainModel(annotatedClasses = UpsertVersionedTest.Record.class) public class UpsertVersionedTest { - @RequiresDialect(H2Dialect.class) - @RequiresDialect(SQLServerDialect.class) - @RequiresDialect(value = PostgreSQLDialect.class, matchSubTypes = false) @Test void test(SessionFactoryScope scope) { scope.inStatelessTransaction(s-> { s.upsert(new Record(123L,null,"hello earth")); @@ -30,21 +23,21 @@ public class UpsertVersionedTest { assertEquals("hello mars",s.get(Record.class,456L).message); }); scope.inStatelessTransaction(s-> { - s.upsert(new Record(123L,1L,"goodbye earth")); + s.upsert(new Record(123L,0L,"goodbye earth")); }); scope.inStatelessTransaction(s-> { assertEquals("goodbye earth",s.get(Record.class,123L).message); assertEquals("hello mars",s.get(Record.class,456L).message); }); scope.inStatelessTransaction(s-> { - s.upsert(new Record(456L,4L,"goodbye mars")); + s.upsert(new Record(456L,3L,"goodbye mars")); }); scope.inStatelessTransaction(s-> { assertEquals("goodbye earth",s.get(Record.class,123L).message); assertEquals("goodbye mars",s.get(Record.class,456L).message); }); } - @Entity + @Entity(name = "Record") static class Record { @Id Long id; @Version Long version;