HHH-16084 - MERGE (upsert) for optional table updates - H2

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

@@ -318,7 +318,6 @@ public class H2SqlAstTranslator<T extends JdbcOperation> extends AbstractSqlAstT
 		// merge into [table] as t
 		// using values([bindings]) as s ([column-names])
 		// on t.[key] = s.[key]
-		//		and t.[key] = ?
 		// when not matched
 		// 		then insert ...
 		// when matched
@@ -357,36 +356,29 @@ public class H2SqlAstTranslator<T extends JdbcOperation> extends AbstractSqlAstT
 		final List<ColumnValueBinding> valueBindings = tableUpsert.getValueBindings();
 		final List<ColumnValueBinding> keyBindings = tableUpsert.getKeyBindings();
 
+		final StringBuilder columnList = new StringBuilder();
+
 		appendSql( "using values (" );
 
 		for ( int i = 0; i < keyBindings.size(); i++ ) {
 			final ColumnValueBinding keyBinding = keyBindings.get( i );
 			if ( i > 0 ) {
 				appendSql( ", " );
+				columnList.append( ", " );
 			}
-			keyBinding.getValueExpression().accept( this );
+			columnList.append( keyBinding.getColumnReference().getColumnExpression() );
+			renderCasted( keyBinding.getValueExpression() );
 		}
 		for ( int i = 0; i < valueBindings.size(); i++ ) {
 			appendSql( ", " );
+			columnList.append( ", " );
 			final ColumnValueBinding valueBinding = valueBindings.get( i );
-			valueBinding.getValueExpression().accept( this );
+			columnList.append( valueBinding.getColumnReference().getColumnExpression() );
+			renderCasted( valueBinding.getValueExpression() );
 		}
 
 		appendSql( ") as s(" );
-
-		for ( int i = 0; i < keyBindings.size(); i++ ) {
-			final ColumnValueBinding keyBinding = keyBindings.get( i );
-			if ( i > 0 ) {
-				appendSql( ", " );
-			}
-			appendSql( keyBinding.getColumnReference().getColumnExpression() );
-		}
-		for ( int i = 0; i < valueBindings.size(); i++ ) {
-			appendSql( ", " );
-			final ColumnValueBinding valueBinding = valueBindings.get( i );
-			appendSql( valueBinding.getColumnReference().getColumnExpression() );
-		}
-
+		appendSql( columnList.toString() );
 		appendSql( ")" );
 	}
 
@@ -411,34 +403,28 @@ public class H2SqlAstTranslator<T extends JdbcOperation> extends AbstractSqlAstT
 		final List<ColumnValueBinding> valueBindings = tableUpsert.getValueBindings();
 		final List<ColumnValueBinding> keyBindings = tableUpsert.getKeyBindings();
 
+		final StringBuilder valuesList = new StringBuilder();
+
 		appendSql( "when not matched then insert (" );
 		for ( int i = 0; i < keyBindings.size(); i++ ) {
 			if ( i > 0 ) {
 				appendSql( ", " );
+				valuesList.append( ", " );
 			}
 			final ColumnValueBinding keyBinding = keyBindings.get( i );
 			appendSql( keyBinding.getColumnReference().getColumnExpression() );
+			keyBinding.getColumnReference().appendReadExpression( "s", valuesList::append );
 		}
 		for ( int i = 0; i < valueBindings.size(); i++ ) {
 			appendSql( ", " );
+			valuesList.append( ", " );
 			final ColumnValueBinding valueBinding = valueBindings.get( i );
 			appendSql( valueBinding.getColumnReference().getColumnExpression() );
+			valueBinding.getColumnReference().appendReadExpression( "s", valuesList::append );
 		}
 
 		appendSql( ") values (" );
-		for ( int i = 0; i < keyBindings.size(); i++ ) {
-			if ( i > 0 ) {
-				appendSql( ", " );
-			}
-			final ColumnValueBinding keyBinding = keyBindings.get( i );
-			keyBinding.getColumnReference().appendReadExpression( this, "s" );
-		}
-		for ( int i = 0; i < valueBindings.size(); i++ ) {
-			appendSql( ", " );
-			final ColumnValueBinding valueBinding = valueBindings.get( i );
-			valueBinding.getColumnReference().appendReadExpression( this, "s" );
-		}
-
+		appendSql( valuesList.toString() );
 		appendSql( ")" );
 	}
 

hibernate-core/src/main/java/org/hibernate/sql/ast/tree/expression/ColumnReference.java

@@ -13,8 +13,8 @@ import java.util.Objects;
 import java.util.function.Consumer;
 
 import org.hibernate.internal.util.StringHelper;
-import org.hibernate.metamodel.mapping.SelectableMapping;
 import org.hibernate.metamodel.mapping.JdbcMapping;
+import org.hibernate.metamodel.mapping.SelectableMapping;
 import org.hibernate.metamodel.mapping.SelectablePath;
 import org.hibernate.sql.ast.SqlAstWalker;
 import org.hibernate.sql.ast.spi.SqlAppender;
@@ -193,27 +193,31 @@ public class ColumnReference implements Expression, Assignable {
 		appendReadExpression( appender, qualifier );
 	}
 
-	public void appendReadExpression(SqlAppender appender, String qualifier) {
+	public void appendReadExpression(String qualifier, Consumer<String> appender) {
 		if ( isFormula ) {
-			appender.append( columnExpression );
+			appender.accept( columnExpression );
 		}
 		else if ( readExpression != null ) {
 			if ( qualifier == null ) {
-				appender.append( replace( readExpression, TEMPLATE + ".", "" ) );
+				appender.accept( replace( readExpression, TEMPLATE + ".", "" ) );
 			}
 			else {
-				appender.append( replace( readExpression, TEMPLATE, qualifier ) );
+				appender.accept( replace( readExpression, TEMPLATE, qualifier ) );
 			}
 		}
 		else {
 			if ( qualifier != null ) {
-				appender.append( qualifier );
-				appender.append( '.' );
+				appender.accept( qualifier );
+				appender.accept( "." );
 			}
-			appender.append( columnExpression );
+			appender.accept( columnExpression );
 		}
 	}
 
+	public void appendReadExpression(SqlAppender appender, String qualifier) {
+		appendReadExpression( qualifier, appender::appendSql );
+	}
+
 	public void appendColumnForWrite(SqlAppender appender) {
 		appendColumnForWrite( appender, qualifier );
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/secondarytable/Record.java

@@ -1,5 +1,7 @@
 package org.hibernate.orm.test.secondarytable;
 
+import org.hibernate.annotations.SecondaryRow;
+
 import jakarta.persistence.Column;
 import jakarta.persistence.Entity;
 import jakarta.persistence.GeneratedValue;
@@ -7,20 +9,19 @@ import jakarta.persistence.Id;
 import jakarta.persistence.SecondaryTable;
 import jakarta.persistence.SequenceGenerator;
 import jakarta.persistence.Table;
-import org.hibernate.annotations.SecondaryRow;
 
 @Entity
-@Table(name = "Overview")
-@SecondaryTable(name = "Details")
-@SecondaryTable(name = "Extras")
-@SecondaryRow(table = "Details", optional = false)
-@SecondaryRow(table = "Extras", optional = true)
+@Table(name = "Details")
+@SecondaryTable(name = "NonOptional")
+@SecondaryTable(name = "Optional")
+@SecondaryRow(table = "NonOptional", optional = false)
+@SecondaryRow(table = "Optional", optional = true)
 @SequenceGenerator(name="RecordSeq", sequenceName = "RecordId", allocationSize = 1)
 public class Record {
     @Id @GeneratedValue(generator = "RecordSeq")  long id;
     String name;
-    @Column(table = "Details") String text;
-    @Column(table = "Details") boolean enabled;
-    @Column(table = "Extras", name="`comment`") String comment;
+    @Column(table = "NonOptional") String text;
+    @Column(table = "NonOptional") boolean enabled;
+    @Column(table = "Optional", name="`comment`") String comment;
 }
 

hibernate-core/src/test/java/org/hibernate/orm/test/secondarytable/SecondaryRowTest.java

@@ -1,113 +1,88 @@
 package org.hibernate.orm.test.secondarytable;
 
-import java.time.LocalDateTime;
+import java.time.Instant;
 
-import org.hibernate.dialect.H2Dialect;
+import org.hibernate.engine.spi.SessionImplementor;
 
 import org.hibernate.testing.orm.junit.DomainModel;
 import org.hibernate.testing.orm.junit.SessionFactory;
 import org.hibernate.testing.orm.junit.SessionFactoryScope;
-import org.hibernate.testing.orm.junit.SkipForDialect;
+import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
 
 @DomainModel(annotatedClasses = {Record.class,SpecialRecord.class})
 @SessionFactory
 public class SecondaryRowTest {
 	@Test
-	@SkipForDialect(
-			dialectClass = H2Dialect.class,
-			reason = "This test relies on SQL execution counts which is based on the legacy multi-statement solution.  " +
-					"HHH-16084 adds support for H2 MERGE which handles those cases in one statement, so the counts are off.  " +
-					"Need to change this test to physically check the number of rows.  " +
-					"See e.g. org.hibernate.orm.test.write.UpsertTests"
-	)
-	public void testSecondaryRow(SessionFactoryScope scope) {
-		// we need to check the actual number of rows.
-		// because we now support merge/upsert SQL statements, the
-		// because HHH-16084 implements support for  usage
-		int seq = scope.getSessionFactory().getJdbcServices().getDialect().getSequenceSupport().supportsSequences()
-				? 1 : 0;
+	void testSecondaryTableOptionality(SessionFactoryScope scope) {
+		scope.inSession( (session) -> {
+			verifySecondaryRows( "Optional", 0, session );
+			verifySecondaryRows( "NonOptional", 0, session );
+		} );
 
-		Record record = new Record();
-		record.enabled = true;
-		record.text = "Hello World!";
-		scope.inTransaction(s -> s.persist(record));
-		scope.getCollectingStatementInspector().assertExecutedCount(2+seq);
-		scope.getCollectingStatementInspector().clear();
+		final Record created = scope.fromTransaction( (session) -> {
+			Record record = new Record();
+			record.enabled = true;
+			record.text = "Hello World!";
 
-		Record record2 = new Record();
-		record2.enabled = true;
-		record2.text = "Hello World!";
-		record2.comment = "Goodbye";
-		scope.inTransaction(s -> s.persist(record2));
-		scope.getCollectingStatementInspector().assertExecutedCount(3+seq);
-		scope.getCollectingStatementInspector().clear();
+			session.persist( record );
+			return record;
+		} );
+		scope.inSession( (session) -> {
+			verifySecondaryRows( "Optional", 0, session );
+			verifySecondaryRows( "NonOptional", 1, session );
+		} );
 
-		SpecialRecord specialRecord = new SpecialRecord();
-		specialRecord.enabled = true;
-		specialRecord.text = "Hello World!";
-		specialRecord.validated = LocalDateTime.now();
-		scope.inTransaction(s -> s.persist(specialRecord));
-		scope.getCollectingStatementInspector().assertExecutedCount(3+seq);
-		scope.getCollectingStatementInspector().clear();
+		created.comment = "I was here";
+		final Record merged = scope.fromTransaction( (session) -> session.merge( created ) );
+		scope.inSession( (session) -> {
+			verifySecondaryRows( "Optional", 1, session );
+			verifySecondaryRows( "NonOptional", 1, session );
+		} );
 
-		scope.inTransaction(s -> assertNotNull(s.find(Record.class, record.id)));
-		scope.getCollectingStatementInspector().assertExecutedCount(1);
-		scope.getCollectingStatementInspector().clear();
+		merged.comment = null;
+		scope.inTransaction( (session) -> session.merge( merged ) );
+		scope.inSession( (session) -> {
+			verifySecondaryRows( "Optional", 0, session );
+			verifySecondaryRows( "NonOptional", 1, session );
+		} );
+	}
 
-		scope.inTransaction(s -> assertNotNull(s.find(Record.class, record2.id)));
-		scope.getCollectingStatementInspector().assertExecutedCount(1);
-		scope.getCollectingStatementInspector().clear();
+	@Test
+	public void testOwnedSecondaryTable(SessionFactoryScope scope) {
+		final String View_name = scope.getSessionFactory().getJdbcServices().getDialect().quote( "`View`" );
+		verifySecondaryRows( View_name, 0, scope );
 
-		scope.inTransaction(s -> assertNotNull(s.find(Record.class, specialRecord.id)));
-		scope.getCollectingStatementInspector().assertExecutedCount(1);
-		scope.getCollectingStatementInspector().clear();
+		final SpecialRecord created = scope.fromTransaction( (session) -> {
+			final SpecialRecord record = new SpecialRecord();
+			record.enabled = true;
+			record.text = "Hello World!";
+			session.persist( record );
+			return record;
+		} );
+		verifySecondaryRows( View_name, 0, scope );
 
-		scope.inTransaction(s -> assertEquals(3, s.createQuery("from Record").getResultList().size()));
-		scope.getCollectingStatementInspector().assertExecutedCount(1);
-		scope.getCollectingStatementInspector().clear();
+		created.timestamp = Instant.now();
+		final SpecialRecord merged = scope.fromTransaction( (session) -> session.merge( created ) );
+		verifySecondaryRows( View_name, 0, scope );
+	}
 
-		scope.inTransaction(s -> assertEquals(1, s.createQuery("from SpecialRecord").getResultList().size()));
-		scope.getCollectingStatementInspector().assertExecutedCount(1);
-		scope.getCollectingStatementInspector().clear();
+	@AfterEach
+	void cleanUpTestData(SessionFactoryScope scope) {
+		scope.inTransaction( (session) -> {
+			session.createSelectionQuery( "from Record" ).stream().forEach( session::remove );
+		} );
+	}
 
-		scope.inTransaction(s -> {
-			Record r = s.find(Record.class, record.id);
-			r.text = "new text";
-			r.comment = "the comment";
-		});
-		scope.getCollectingStatementInspector().assertExecutedCount(3);
-		assertTrue( scope.getCollectingStatementInspector().getSqlQueries().get(1).startsWith("update ") );
-		assertTrue( scope.getCollectingStatementInspector().getSqlQueries().get(2).startsWith("insert ") );
-		scope.getCollectingStatementInspector().clear();
+	private static void verifySecondaryRows(String table, int expectedCount, SessionFactoryScope sfScope) {
+		sfScope.inTransaction( (session) -> verifySecondaryRows( table, expectedCount, session ) );
+	}
 
-		scope.inTransaction(s -> {
-			Record r = s.find(Record.class, record.id);
-			r.comment = "new comment";
-		});
-		scope.getCollectingStatementInspector().assertExecutedCount(2);
-		assertTrue( scope.getCollectingStatementInspector().getSqlQueries().get(1).startsWith("update ") );
-		scope.getCollectingStatementInspector().clear();
-
-		scope.inTransaction(s -> {
-			Record r = s.find(Record.class, record2.id);
-			r.comment = null;
-		});
-		scope.getCollectingStatementInspector().assertExecutedCount(2);
-		assertTrue( scope.getCollectingStatementInspector().getSqlQueries().get(1).startsWith("delete ") );
-		scope.getCollectingStatementInspector().clear();
-
-		scope.inTransaction(s -> {
-			SpecialRecord r = s.find(SpecialRecord.class, specialRecord.id);
-			r.validated = null;
-			r.timestamp = System.currentTimeMillis();
-		});
-		scope.getCollectingStatementInspector().assertExecutedCount(2);
-		assertTrue( scope.getCollectingStatementInspector().getSqlQueries().get(1).startsWith("delete ") );
-		scope.getCollectingStatementInspector().clear();
+	private static void verifySecondaryRows(String table, int expectedCount, SessionImplementor session) {
+		final String sql = "select count(1) from " + table;
+		final int count = session.createNativeQuery( sql, Integer.class ).getSingleResult();
+		assertThat( count ).isEqualTo( expectedCount );
 	}
 }

hibernate-core/src/test/java/org/hibernate/orm/test/secondarytable/SpecialRecord.java

@@ -1,19 +1,17 @@
 package org.hibernate.orm.test.secondarytable;
 
+import java.time.Instant;
+
+import org.hibernate.annotations.SecondaryRow;
+
 import jakarta.persistence.Column;
 import jakarta.persistence.Entity;
 import jakarta.persistence.SecondaryTable;
-import org.hibernate.annotations.SecondaryRow;
-
-import java.time.LocalDateTime;
 
 @Entity
-@SecondaryTable(name = "Options")
 @SecondaryTable(name = "`View`")
 @SecondaryRow(table = "`View`", owned = false)
 public class SpecialRecord extends Record {
-    @Column(table = "Options")
-    LocalDateTime validated;
     @Column(table = "`View`", name="`timestamp`")
-    Long timestamp;
+    Instant timestamp;
 }

hibernate-core/src/test/java/org/hibernate/orm/test/write/UpsertTests.java

@@ -43,13 +43,19 @@ public class UpsertTests {
 		verifySecondaryRows( scope, 1 );
 	}
 
-	private void verifySecondaryRows(SessionFactoryScope scope, int expectedCount) {
-		scope.inTransaction( (session) -> {
-			final int count = session.createNativeQuery( "select count(1) from supplements", Integer.class ).getSingleResult();
+
+	private static void verifySecondaryRows(String table, int expectedCount, SessionFactoryScope sfScope) {
+		final String sql = "select count(1) from " + table;
+		sfScope.inTransaction( (session) -> {
+			final int count = session.createNativeQuery( sql, Integer.class ).getSingleResult();
 			assertThat( count ).isEqualTo( expectedCount );
 		} );
 	}
 
+	private static void verifySecondaryRows(SessionFactoryScope scope, int expectedCount) {
+		verifySecondaryRows( "supplements", expectedCount, scope );
+	}
+
 	@Test
 	void testUpsertUpdate(SessionFactoryScope scope) {
 		scope.inTransaction( (session) -> {