diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/logging/impl/Log.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/logging/impl/Log.java index cb1fa5181..d3aa64278 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/logging/impl/Log.java +++ b/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!" ) diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/persister/entity/impl/ReactiveAbstractEntityPersister.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/persister/entity/impl/ReactiveAbstractEntityPersister.java index af04f13a4..7ba16faba 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/persister/entity/impl/ReactiveAbstractEntityPersister.java +++ b/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 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 deleteReactive( diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/IdentityGeneratorTypeForCockroachDBTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/IdentityGeneratorTypeForCockroachDBTest.java index 17712a539..91f2fabbf 100644 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/IdentityGeneratorTypeForCockroachDBTest.java +++ b/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. + *

+ * 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. + *

* * @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,11 +86,8 @@ 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 ) ) ); } @@ -94,14 +95,30 @@ public void integerIdentityType(TestContext context) { @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 getId(); }