hand over responsibilities of SelectGenerator to @Generated

at the end of all this work on SelectGenerator, a cruel twist of fate!

hibernate-core/src/main/java/org/hibernate/annotations/Generated.java

@@ -6,14 +6,15 @@
  */
 package org.hibernate.annotations;
 
-import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 import org.hibernate.generator.EventType;
 import org.hibernate.generator.internal.GeneratedGeneration;
 
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
 import static org.hibernate.generator.EventType.INSERT;
 import static org.hibernate.generator.EventType.UPDATE;
 
@@ -56,8 +57,9 @@ import static org.hibernate.generator.EventType.UPDATE;
  * @see GeneratedColumn
  */
 @ValueGenerationType( generatedBy = GeneratedGeneration.class )
-@Target({ElementType.FIELD, ElementType.METHOD})
+@IdGeneratorType( GeneratedGeneration.class )
-@Retention(RetentionPolicy.RUNTIME)
+@Target( {FIELD, METHOD} )
+@Retention( RUNTIME )
 public @interface Generated {
 	/**
 	 * Specifies the events that cause the value to be generated by the

hibernate-core/src/main/java/org/hibernate/annotations/GeneratedColumn.java

@@ -27,9 +27,9 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
  *
  * @see DialectOverride.GeneratedColumn
  */
+@ValueGenerationType( generatedBy = GeneratedAlwaysGeneration.class )
 @Target( {FIELD, METHOD} )
 @Retention( RUNTIME )
-@ValueGenerationType(generatedBy = GeneratedAlwaysGeneration.class)
 public @interface GeneratedColumn {
 	/**
 	 * The expression to include in the generated DDL.

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

@@ -3448,7 +3448,7 @@ public abstract class Dialect implements ConversionContext {
 		return false;
 	}
 
-	public boolean supportedInsertReturningGeneratedKeys() {
+	public boolean supportsInsertReturningGeneratedKeys() {
 		return false;
 	}
 	/**

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

@@ -324,7 +324,7 @@ public class OracleDialect extends Dialect {
 	}
 
 	@Override
-	public boolean supportedInsertReturningGeneratedKeys() {
+	public boolean supportsInsertReturningGeneratedKeys() {
 		return true;
 	}
 

hibernate-core/src/main/java/org/hibernate/generator/InDatabaseGenerator.java

@@ -6,10 +6,16 @@
  */
 package org.hibernate.generator;
 
+import org.hibernate.Incubating;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.id.PostInsertIdentityPersister;
-import org.hibernate.id.insert.BasicSelectingDelegate;
+import org.hibernate.id.insert.GetGeneratedKeysDelegate;
 import org.hibernate.id.insert.InsertGeneratedIdentifierDelegate;
+import org.hibernate.id.insert.InsertReturningDelegate;
+import org.hibernate.id.insert.UniqueKeySelectingDelegate;
+import org.hibernate.persister.entity.EntityPersister;
+
+import static org.hibernate.generator.internal.NaturalIdHelper.getNaturalIdPropertyName;
 
 /**
  * A value generated by the database might be generated implicitly, by a trigger, or using
@@ -71,15 +77,60 @@ public interface InDatabaseGenerator extends Generator {
 	String[] getReferencedColumnValues(Dialect dialect);
 
 	/**
-	 * The {@link InsertGeneratedIdentifierDelegate} used to retrieve the generates value if this
+	 * The {@link InsertGeneratedIdentifierDelegate} used to retrieve the generated value if this
 	 * object is an identifier generator.
 	 * <p>
 	 * This is ignored by {@link org.hibernate.metamodel.mapping.internal.GeneratedValuesProcessor},
-	 * which handles multiple generators at once. This method arguably breaks the separation of
+	 * which handles multiple generators at once. So if this object is not an identifier generator,
-	 * concerns between the generator and the coordinating code.
+	 * this method is never called.
+	 * <p>
+	 * Note that this method arguably breaks the separation of concerns between the generator and
+	 * coordinating code, by specifying how the generated value should be <em>retrieved</em>.
+	 * <p>
+	 * The problem solved here is: we want to obtain an insert-generated primary key. But, sadly,
+	 * without already knowing the primary key, there's no completely-generic way to locate the
+	 * just-inserted row to obtain it.
+	 * <p>
+	 * We need one of the following things:
+	 * <ul>
+	 *     <li>a database which supports some form of {@link Dialect#supportsInsertReturning()
+	 *         insert ... returning} syntax, or can do the same thing using the JDBC
+	 *         {@link Dialect#supportsInsertReturningGeneratedKeys() getGeneratedKeys()} API, or
+	 *     <li>a second unique key of the entity, that is, a property annotated
+	 *         {@link org.hibernate.annotations.NaturalId @NaturalId}.
+	 * </ul>
+	 * Alternatively, if the generated id is an identity/"autoincrement" column, we can take
+	 * advantage of special platform-specific functionality to retrieve it. Taking advantage
+	 * of the specialness of identity columns is the job of one particular implementation:
+	 * {@link org.hibernate.id.IdentityGenerator}. And the need for customized behavior for
+	 * identity columns is the reason why this layer-breaking method exists.
 	 */
+	@Incubating
 	default InsertGeneratedIdentifierDelegate getGeneratedIdentifierDelegate(PostInsertIdentityPersister persister) {
-		return new BasicSelectingDelegate( persister, persister.getFactory().getJdbcServices().getDialect() );
+		Dialect dialect = persister.getFactory().getJdbcServices().getDialect();
+		if ( dialect.supportsInsertReturningGeneratedKeys() ) {
+			return new GetGeneratedKeysDelegate( persister, dialect, false );
+		}
+		else if ( dialect.supportsInsertReturning() ) {
+			return new InsertReturningDelegate( persister, dialect );
+		}
+		else {
+			// let's just hope the entity has a @NaturalId!
+			return new UniqueKeySelectingDelegate( persister, dialect, getUniqueKeyPropertyName( persister ) );
+		}
+	}
+
+	/**
+	 * The name of a property of the entity which may be used to locate the just-{@code insert}ed
+	 * row containing the generated value. Of course, the columns mapped by this property should
+	 * form a unique key of the entity.
+	 * <p>
+	 * The default implementation uses the {@link org.hibernate.annotations.NaturalId @NaturalId}
+	 * property, if there is one.
+	 */
+	@Incubating
+	default String getUniqueKeyPropertyName(EntityPersister persister) {
+		return getNaturalIdPropertyName( persister );
 	}
 
 	default boolean generatedByDatabase() {

hibernate-core/src/main/java/org/hibernate/generator/internal/NaturalIdHelper.java

@@ -0,0 +1,35 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.generator.internal;
+
+import org.hibernate.id.IdentifierGenerationException;
+import org.hibernate.persister.entity.EntityPersister;
+
+public class NaturalIdHelper {
+	public static String getNaturalIdPropertyName(EntityPersister persister) {
+		int[] naturalIdPropertyIndices = persister.getNaturalIdentifierProperties();
+		if ( naturalIdPropertyIndices == null ) {
+			throw new IdentifierGenerationException(
+					"no natural-id property defined; " +
+							"need to specify [key] in generator parameters"
+			);
+		}
+		if ( naturalIdPropertyIndices.length > 1 ) {
+			throw new IdentifierGenerationException(
+					"generator does not currently support composite natural-id properties;" +
+							" need to specify [key] in generator parameters"
+			);
+		}
+		if ( persister.getEntityMetamodel().isNaturalIdentifierInsertGenerated() ) {
+			throw new IdentifierGenerationException(
+					"natural-id also defined as insert-generated; " +
+							"need to specify [key] in generator parameters"
+			);
+		}
+		return persister.getPropertyNames()[naturalIdPropertyIndices[0]];
+	}
+}

hibernate-core/src/main/java/org/hibernate/id/SelectGenerator.java

@@ -11,14 +11,12 @@ import java.util.Properties;
 import org.hibernate.dialect.Dialect;
 import org.hibernate.generator.InDatabaseGenerator;
 import org.hibernate.id.factory.spi.StandardGenerator;
-import org.hibernate.id.insert.GetGeneratedKeysDelegate;
-import org.hibernate.id.insert.InsertGeneratedIdentifierDelegate;
-import org.hibernate.id.insert.InsertReturningDelegate;
-import org.hibernate.id.insert.UniqueKeySelectingDelegate;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.service.ServiceRegistry;
 import org.hibernate.type.Type;
 
+import static org.hibernate.generator.internal.NaturalIdHelper.getNaturalIdPropertyName;
+
 /**
  * A generator that {@code select}s the just-{@code insert}ed row to determine the
  * column value assigned by the database. The correct row is located using a unique
@@ -45,6 +43,21 @@ import org.hibernate.type.Type;
  *     ...
  * }
  * }</pre>
+ * However, after a very long working life, this generator is now handing over its
+ * work to {@link org.hibernate.generator.internal.GeneratedGeneration}, and the
+ * above code may be written as:
+ * <pre>{@code
+ * @Entity @Table(name="TableWithPKAssignedByTrigger")
+ * public class TriggeredEntity {
+ *     @Id @Generated(event = INSERT)
+ *     private Long id;
+ *
+ *     @NaturalId
+ *     private String name;
+ *
+ *     ...
+ * }
+ * }</pre>
  * For tables with identity/autoincrement columns, use {@link IdentityGenerator}.
  * <p>
  * The actual work involved in retrieving the primary key value is the job of
@@ -68,49 +81,11 @@ public class SelectGenerator
 		uniqueKeyPropertyName = parameters.getProperty( "key" );
 	}
 
-	/**
-	 * The name of a property of the entity which may be used to locate the just-{@code insert}ed
-	 * row containing the generated value. Of course, the columns mapped by this property should
-	 * form a unique key of the entity.
-	 */
-	protected String getUniqueKeyPropertyName(EntityPersister persister) {
-		if ( uniqueKeyPropertyName != null ) {
-			return uniqueKeyPropertyName;
-		}
-		int[] naturalIdPropertyIndices = persister.getNaturalIdentifierProperties();
-		if ( naturalIdPropertyIndices == null ) {
-			throw new IdentifierGenerationException(
-					"no natural-id property defined; need to specify [key] in " +
-							"generator parameters"
-			);
-		}
-		if ( naturalIdPropertyIndices.length > 1 ) {
-			throw new IdentifierGenerationException(
-					"select generator does not currently support composite " +
-							"natural-id properties; need to specify [key] in generator parameters"
-			);
-		}
-		if ( persister.getEntityMetamodel().isNaturalIdentifierInsertGenerated() ) {
-			throw new IdentifierGenerationException(
-					"natural-id also defined as insert-generated; need to specify [key] " +
-							"in generator parameters"
-			);
-		}
-		return persister.getPropertyNames()[naturalIdPropertyIndices[0]];
-	}
-
 	@Override
-	public InsertGeneratedIdentifierDelegate getGeneratedIdentifierDelegate(PostInsertIdentityPersister persister) {
+	public String getUniqueKeyPropertyName(EntityPersister persister) {
-		Dialect dialect = persister.getFactory().getJdbcServices().getDialect();
+		return uniqueKeyPropertyName != null
-		if ( dialect.supportedInsertReturningGeneratedKeys() ) {
+				? uniqueKeyPropertyName
-			return new GetGeneratedKeysDelegate( persister, dialect, false );
+				: getNaturalIdPropertyName( persister );
-		}
-		else if ( dialect.supportsInsertReturning() ) {
-			return new InsertReturningDelegate( persister, dialect );
-		}
-		else {
-			return new UniqueKeySelectingDelegate( persister, dialect, getUniqueKeyPropertyName( persister ) );
-		}
 	}
 
 	@Override
@@ -120,6 +95,6 @@ public class SelectGenerator
 
 	@Override
 	public String[] getReferencedColumnValues(Dialect dialect) {
-		return new String[0];
+		return null;
 	}
 }

hibernate-core/src/test/java/org/hibernate/orm/test/generatedkeys/generated/GeneratedTest.java

@@ -0,0 +1,85 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.orm.test.generatedkeys.generated;
+
+import org.hibernate.dialect.DB2Dialect;
+import org.hibernate.dialect.H2Dialect;
+import org.hibernate.dialect.MySQLDialect;
+import org.hibernate.dialect.OracleDialect;
+import org.hibernate.dialect.PostgreSQLDialect;
+import org.hibernate.dialect.SQLServerDialect;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.JiraKey;
+import org.hibernate.testing.orm.junit.RequiresDialect;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.hibernate.tool.hbm2ddl.SchemaExport;
+import org.hibernate.tool.schema.TargetType;
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.EnumSet;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+/**
+ * @author Steve Ebersole
+ * @author Marco Belladelli
+ */
+@DomainModel(
+		annotatedClasses = MyEntity.class,
+		xmlMappings = "org/hibernate/orm/test/generatedkeys/selectannotated/MyEntity.hbm.xml"
+)
+@SessionFactory
+@RequiresDialect(OracleDialect.class)
+@RequiresDialect(PostgreSQLDialect.class)
+@RequiresDialect(MySQLDialect.class)
+@RequiresDialect(H2Dialect.class)
+@RequiresDialect(DB2Dialect.class)
+@RequiresDialect(SQLServerDialect.class)
+public class GeneratedTest {
+
+	@Test
+	public void test(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					MyEntity e1 = new MyEntity( "entity-1" );
+					session.persist( e1 );
+					// this insert should happen immediately!
+					assertEquals( Long.valueOf( 1L ), e1.getId(), "id not generated through forced insertion" );
+
+					MyEntity e2 = new MyEntity( "entity-2" );
+					session.persist( e2 );
+					assertEquals( Long.valueOf( 2L ), e2.getId(), "id not generated through forced insertion" );
+
+					session.remove( e1 );
+					session.remove( e2 );
+				}
+		);
+	}
+
+	@Test
+	@JiraKey("HHH-15900")
+	public void testGeneratedKeyNotIdentityColumn(SessionFactoryScope scope) throws IOException {
+		File output = File.createTempFile( "schema_export", ".sql" );
+		output.deleteOnExit();
+
+		final SchemaExport schemaExport = new SchemaExport();
+		schemaExport.setOutputFile( output.getAbsolutePath() );
+		schemaExport.execute(
+				EnumSet.of( TargetType.SCRIPT ),
+				SchemaExport.Action.CREATE,
+				scope.getMetadataImplementor()
+		);
+
+		String fileContent = new String( Files.readAllBytes( output.toPath() ) );
+		assertFalse( fileContent.toLowerCase().contains( "identity" ), "Column was generated as identity" );
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/generatedkeys/generated/H2Trigger.java

@@ -0,0 +1,16 @@
+package org.hibernate.orm.test.generatedkeys.generated;
+
+import org.h2.tools.TriggerAdapter;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+
+public class H2Trigger extends TriggerAdapter {
+	@Override
+	public void fire(Connection conn, ResultSet oldRow, ResultSet newRow) throws SQLException {
+		ResultSet resultSet = conn.createStatement().executeQuery("select coalesce(max(id), 0) from my_entity");
+		resultSet.next();
+		newRow.updateInt( "id", resultSet.getInt(1) + 1 );
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/generatedkeys/generated/MyEntity.hbm.xml

@@ -0,0 +1,115 @@
+<?xml version="1.0"?>
+<!--
+  ~ Hibernate, Relational Persistence for Idiomatic Java
+  ~
+  ~ License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+  ~ See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+  -->
+<!DOCTYPE hibernate-mapping PUBLIC
+	"-//Hibernate/Hibernate Mapping DTD 3.0//EN"
+	"http://www.hibernate.org/dtd/hibernate-mapping-3.0.dtd">
+
+<hibernate-mapping package="org.hibernate.orm.test.generatedkeys.select" default-access="field">
+
+    <database-object>
+        <create>
+            <![CDATA[create trigger my_entity_trigger
+            before insert on my_entity
+            for each row call "org.hibernate.orm.test.generatedkeys.selectannotated.H2Trigger"]]>
+        </create>
+        <drop>
+            <![CDATA[drop trigger if exists my_entity_trigger]]>
+        </drop>
+        <dialect-scope name="org.hibernate.dialect.H2Dialect"/>
+    </database-object>
+
+    <database-object>
+        <create>
+            <![CDATA[create or replace function gen_id_my_entity() returns trigger as
+            $$
+            begin
+                select coalesce(max(id), 0) + 1
+                into new.id
+                from my_entity;
+                return new;
+            end
+            $$
+            language plpgsql;
+
+            create or replace trigger my_entity_trigger
+            before insert on my_entity
+            for each row
+            execute procedure gen_id_my_entity();]]>
+        </create>
+        <drop>
+            <![CDATA[drop trigger if exists my_entity_trigger on my_entity;
+            drop function if exists gen_id_my_entity;]]>
+        </drop>
+        <dialect-scope name="org.hibernate.dialect.PostgreSQLDialect"/>
+        <dialect-scope name="org.hibernate.dialect.PostgresPlusDialect"/>
+    </database-object>
+
+    <database-object>
+        <create>
+            <![CDATA[create trigger my_entity_trigger
+            before insert on my_entity
+            for each row
+            set new.id = (select coalesce(max(id), 0) + 1 from my_entity)]]>
+        </create>
+        <drop>
+            <![CDATA[drop trigger if exists my_entity_trigger]]>
+        </drop>
+        <dialect-scope name="org.hibernate.dialect.MySQLDialect"/>
+        <dialect-scope name="org.hibernate.dialect.MariaDBDialect"/>
+    </database-object>
+
+    <database-object>
+        <create>
+            <![CDATA[create or alter trigger my_entity_trigger
+            on my_entity
+            instead of insert
+            as
+            begin
+                insert into my_entity (id, name) values ( (select coalesce(max(id), 0) + 1 from my_entity), (select name from inserted) );
+            end]]>
+        </create>
+        <drop>
+            <![CDATA[drop trigger if exists my_entity_trigger]]>
+        </drop>
+        <dialect-scope name="org.hibernate.dialect.SQLServerDialect"/>
+    </database-object>
+
+    <database-object>
+        <create>
+            <![CDATA[create or replace trigger my_entity_trigger
+            before insert on my_entity
+            referencing new as new_entity
+            for each row
+            begin
+                set new_entity.id = (select coalesce(max(id), 0) + 1 from my_entity);
+            end]]>
+        </create>
+        <drop>
+            <![CDATA[drop trigger my_entity_trigger]]>
+        </drop>
+        <dialect-scope name="org.hibernate.dialect.DB2Dialect"/>
+    </database-object>
+
+    <database-object>
+        <create>
+            <![CDATA[create or replace trigger my_entity_trigger
+            before insert on my_entity
+            for each row
+            begin
+                select nvl(max(id), 0) + 1
+                into :new.id
+                from my_entity;
+            end;]]>
+        </create>
+        <drop>
+            <![CDATA[drop trigger my_entity_trigger]]>
+        </drop>
+        <dialect-scope name="org.hibernate.dialect.OracleDialect"/>
+    </database-object>
+
+</hibernate-mapping>

hibernate-core/src/test/java/org/hibernate/orm/test/generatedkeys/generated/MyEntity.java

@@ -0,0 +1,49 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.orm.test.generatedkeys.generated;
+
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.Table;
+import org.hibernate.annotations.ColumnDefault;
+import org.hibernate.annotations.Generated;
+import org.hibernate.annotations.NaturalId;
+
+import static org.hibernate.generator.EventType.INSERT;
+
+/**
+ * @author <a href="mailto:steve@hibernate.org">Steve Ebersole </a>
+ */
+@Entity @Table(name="my_entity")
+public class MyEntity {
+	@Id @Generated(event = INSERT)
+	@ColumnDefault("-666") //workaround for h2 'before insert' triggers being crap
+	private Long id;
+
+	@NaturalId
+	private String name;
+
+	public MyEntity() {
+	}
+
+	public MyEntity(String name) {
+		this.name = name;
+	}
+
+	public Long getId() {
+		return id;
+	}
+
+	public String getName() {
+		return name;
+	}
+
+	public void setName(String name) {
+		this.name = name;
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/generatedkeys/select/SelectGeneratorTest.java

@@ -45,7 +45,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
 public class SelectGeneratorTest {
 
 	@Test
-	public void testJDBC3GetGeneratedKeysSupportOnOracle(SessionFactoryScope scope) {
+	public void test(SessionFactoryScope scope) {
 		scope.inTransaction(
 				session -> {
 					MyEntity e = new MyEntity( "entity-1" );

hibernate-core/src/test/java/org/hibernate/orm/test/generatedkeys/selectannotated/SelectGeneratorTest.java

@@ -47,7 +47,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
 public class SelectGeneratorTest {
 
 	@Test
-	public void testJDBC3GetGeneratedKeysSupportOnOracle(SessionFactoryScope scope) {
+	public void test(SessionFactoryScope scope) {
 		scope.inTransaction(
 				session -> {
 					MyEntity e1 = new MyEntity( "entity-1" );