[hibernate#1338] Validate identity types for CockroachDB

Author: DavideD

hibernate-reactive-core/src/main/java/org/hibernate/reactive/logging/impl/Log.java

@@ -223,6 +223,9 @@ public interface Log extends BasicLogger {
 	@Message(id = 72, value= "Cannot update an uninitialized proxy. Make sure to fetch the value before trying to update it: %1$s")
 	HibernateException uninitializedProxyUpdate(Object entity);
 
+	@Message(id = 73, value = "%1$s is an invalid identity type when using CockroachDB (entity %2$s) - CockroachDB might generates identifiers that are too big and won't always fit in a %1$s. java.lang.Long is valid replacement")
+	HibernateException invalidIdentifierTypeForCockroachDB(@FormatWith(ClassFormatter.class) Class<?> idType, String entityName);
+
 	// Same method that exists in CoreMessageLogger
 	@LogMessage(level = WARN)
 	@Message(id = 104, value = "firstResult/maxResults specified with collection fetch; applying in memory!" )

hibernate-reactive-core/src/main/java/org/hibernate/reactive/persister/entity/impl/ReactiveAbstractEntityPersister.java

@@ -26,6 +26,7 @@
 import org.hibernate.StaleObjectStateException;
 import org.hibernate.bytecode.enhance.spi.interceptor.LazyAttributeDescriptor;
 import org.hibernate.collection.spi.PersistentCollection;
+import org.hibernate.dialect.CockroachDB192Dialect;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.engine.OptimisticLockStyle;
 import org.hibernate.engine.internal.Versioning;
@@ -44,6 +45,7 @@
 import org.hibernate.jdbc.Expectation;
 import org.hibernate.loader.entity.UniqueEntityLoader;
 import org.hibernate.persister.entity.AbstractEntityPersister;
+import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.persister.entity.Lockable;
 import org.hibernate.persister.entity.MultiLoadOptions;
 import org.hibernate.persister.entity.OuterJoinLoadable;
@@ -72,6 +74,8 @@
 import org.hibernate.tuple.NonIdentifierAttribute;
 import org.hibernate.tuple.ValueGenerator;
 import org.hibernate.type.EntityType;
+import org.hibernate.type.IntegerType;
+import org.hibernate.type.ShortType;
 import org.hibernate.type.Type;
 import org.hibernate.type.VersionType;
 
@@ -399,13 +403,28 @@ default CompletionStage<Serializable> insertReactive(
 				//      getGeneratedKeys(), or an extra round select statement. But we
 				//      don't need these extra options.
 				.insertAndSelectIdentifier( sql, params, idClass, delegate().getIdentifierColumnNames()[0] )
-				.thenApply( generatedId -> {
-					log.debugf( "Natively generated identity: %s", generatedId );
-					if ( generatedId == null ) {
-						throw log.noNativelyGeneratedValueReturned();
-					}
-					return castToIdentifierType( generatedId, this );
-				} );
+				.thenApply( this::convertGeneratedId );
+	}
+
+	private Serializable convertGeneratedId(Object generatedId) {
+		log.debugf( "Natively generated identity: %s", generatedId );
+		validateGeneratedIdentityId( generatedId, this );
+		return castToIdentifierType( generatedId, this );
+	}
+
+	private static void validateGeneratedIdentityId(Object generatedId, EntityPersister persister) {
+		if ( generatedId == null ) {
+			throw log.noNativelyGeneratedValueReturned();
+		}
+
+		// CockroachDB might generate an identifier that fits an integer (and maybe a short) from time to time.
+		// Users should not rely on it, or they might have random, hard to debug failures.
+		Type identifierType = persister.getIdentifierType();
+		if ( ( identifierType == IntegerType.INSTANCE || identifierType == ShortType.INSTANCE )
+				&& persister.getFactory().getJdbcServices().getDialect() instanceof CockroachDB192Dialect
+		) {
+			throw log.invalidIdentifierTypeForCockroachDB( identifierType.getReturnedClass(), persister.getEntityName() );
+		}
 	}
 
 	default CompletionStage<Void> deleteReactive(

hibernate-reactive-core/src/test/java/org/hibernate/reactive/IdentityGeneratorTypeForCockroachDBTest.java

@@ -29,7 +29,11 @@
 import static org.hibernate.reactive.testing.ReactiveAssertions.assertThrown;
 
 /**
- * Test supported types for ids generated by CockroachDB
+ * Test supported types for ids generated by CockroachDB.
+ * <p>
+ * It won't work with {@link Short} and {@link Integer} because the id generated by CockroachDB don't map
+ * those types - they are less than 0 or bigger.
+ * </p>
  *
  * @see IdentityGeneratorTest
  * @see IdentityGeneratorTypeTest
@@ -71,7 +75,7 @@ public void longIdentityType(TestContext context) {
 		LongTypeEntity entity = new LongTypeEntity();
 
 		test( context, getMutinySessionFactory()
-				.withTransaction( (s, tx) -> s.persist( entity ) )
+				.withTransaction( s -> s.persist( entity ) )
 				.invoke( () -> {
 					context.assertNotNull( entity );
 					context.assertTrue( entity.id > 0 );
@@ -82,26 +86,39 @@ public void longIdentityType(TestContext context) {
 	@Test
 	public void integerIdentityType(TestContext context) {
 		test( context, assertThrown( PersistenceException.class, getMutinySessionFactory()
-				.withTransaction( (s, tx) -> s.persist( new IntegerTypeEntity() )  ) )
-				.invoke( exception -> {
-					assertThat( exception.getMessage() ).contains( "too big" );
-					assertThat( exception.getMessage() ).contains( "Integer" );
-				} )
+				.withTransaction( s -> s.persist( new IntegerTypeEntity() )  ) )
+				.invoke( exception -> validateErrorMessage( Integer.class, IntegerTypeEntity.class, exception ) )
 		);
 
 	}
 
 	@Test
 	public void shortIdentityType(TestContext context) {
 		test( context, assertThrown( PersistenceException.class, getMutinySessionFactory()
-				.withTransaction( (s, tx) -> s.persist( new ShortTypeEntity() )  ) )
-				.invoke( exception -> {
-					assertThat( exception.getMessage() ).contains( "too big" );
-					assertThat( exception.getMessage() ).contains( "Short" );
-				} )
+				.withTransaction( s -> s.persist( new ShortTypeEntity() )  ) )
+				.invoke( exception -> validateErrorMessage( Short.class, ShortTypeEntity.class, exception ) )
 		);
 	}
 
+
+	private void validateErrorMessage(Class<?> idType, Class<?> entityTYpe, PersistenceException exception) {
+		assertThat( exception.getMessage() )
+				.as( "Unexpected error code - this should be a CockroachDB specific issue" )
+				.contains( "HR000073" );
+		assertThat( exception.getMessage() )
+				.as( "Error message should contain the entity name" )
+				.contains( entityTYpe.getName() );
+		assertThat( exception.getMessage() )
+				.as( "Error message should contain the invalid type" )
+				.contains( idType.getName() );
+		assertThat( exception.getMessage() )
+				.as( "Error message should contain the valid type" )
+				.contains( Long.class.getName() );
+		assertThat( exception.getMessage() )
+				.as( "Error message should mention that it happens only for CockroachDB" )
+				.contains( "CockroachDB" );
+	}
+
 	interface TypeIdentity<T extends Number> {
 		T getId();
 	}