HHH-15393 - Improve write-paths to use mapping model

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

@@ -2266,7 +2266,11 @@ public abstract class Dialect implements ConversionContext {
 	 * This is not possible on some databases.
 	 *
 	 * @return The appropriate empty values clause.
+	 *
+	 * @deprecated Override {@link org.hibernate.sql.ast.spi.AbstractSqlAstTranslator#renderInsertIntoNoColumns}
+	 * on the {@link #getSqlAstTranslatorFactory() translator} returned by this dialect
 	 */
+	@Deprecated( since = "6" )
 	public String getNoColumnsInsertString() {
 		return "values ( )";
 	}

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

@@ -22,6 +22,7 @@ import org.hibernate.sql.ast.tree.select.QueryGroup;
 import org.hibernate.sql.ast.tree.select.QueryPart;
 import org.hibernate.sql.ast.tree.select.QuerySpec;
 import org.hibernate.sql.exec.spi.JdbcOperation;
+import org.hibernate.sql.model.internal.TableInsertStandard;
 
 /**
  * A SQL AST translator for PostgreSQL.
@@ -34,6 +35,13 @@ public class PostgreSQLSqlAstTranslator<T extends JdbcOperation> extends Abstrac
 		super( sessionFactory, statement );
 	}
 
+	@Override
+	protected void renderInsertIntoNoColumns(TableInsertStandard tableInsert) {
+		renderIntoIntoAndTable( tableInsert );
+		appendSql( "default values" );
+	}
+
+
 	@Override
 	protected void renderExpressionAsClauseItem(Expression expression) {
 		expression.accept( this );

hibernate-core/src/main/java/org/hibernate/persister/collection/mutation/UpdateRowsCoordinatorOneToMany.java

@@ -41,9 +41,6 @@ public class UpdateRowsCoordinatorOneToMany extends AbstractUpdateRowsCoordinato
 
 	@Override
 	protected int doUpdate(Object key, PersistentCollection<?> collection, SharedSessionContractImplementor session) {
-		// todo (mutation) : an alternative is to allow "filters" for the delete and insert coordinators
-		//		to limit the rows to delete/insert based on `PersistentCollection#needsUpdating`
-
 		if ( rowMutationOperations.hasDeleteRow() ) {
 			deleteRows( key, collection, session );
 		}

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -3195,8 +3195,6 @@ public abstract class AbstractEntityPersister
 					: substituteBrackets( customSQLUpdate[j] );
 		}
 
-		// todo (mutation) : `tableHasColumns` is only used from a now-deprecated method we
-		//  	no longer use internally. See `#getTableHasColumns`
 		tableHasColumns = new boolean[joinSpan];
 		for ( int j = 0; j < joinSpan; j++ ) {
 			final String tableName = getTableName( j );

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/DeleteCoordinator.java

@@ -57,13 +57,6 @@ public class DeleteCoordinator extends AbstractMutationCoordinator {
 		return staticOperationGroup;
 	}
 
-	public MutationOperationGroup getNoVersionDeleteGroup() {
-		if ( noVersionDeleteGroup == null ) {
-			generateOperationGroup( null, false, null );
-		}
-		return noVersionDeleteGroup;
-	}
-
 	@SuppressWarnings("unused")
 	public BasicBatchKey getBatchKey() {
 		return batchKey;
@@ -103,16 +96,6 @@ public class DeleteCoordinator extends AbstractMutationCoordinator {
 				.getServiceRegistry()
 				.getService( MutationExecutorService.class );
 
-
-		// todo (mutation) : here is where we need to hook in the MutationExecutor and consider "caching".  a few options:
-		//		1) cache stuff on the coordinators and somehow pass access to that to the executor
-		//		2) create a "cache" delegate
-
-		// PreparedStatementGroup -
-		//		- previously was "all inclusive".  we expected all to be batched or none to be batched
-		//		- now we have a mix
-
-
 		final MutationExecutor mutationExecutor = mutationExecutorService.createExecutor(
 				() -> batchKey,
 				operationGroup,

hibernate-core/src/main/java/org/hibernate/persister/entity/mutation/InsertCoordinator.java

@@ -81,10 +81,6 @@ public class InsertCoordinator extends AbstractMutationCoordinator {
 	 * @param session The originating context
 	 *
 	 * @return The id
-	 *
-	 * todo (mutation) : Allow passing an id value here even with post-insert id generation strategies;
-	 * 		this is a long-standing request.  It would simply trigger a dynamically built insert which
-	 * 		binds the id value rather than allowing the database to generate it
 	 */
 	public Object coordinateInsert(
 			Object id,
@@ -131,10 +127,6 @@ public class InsertCoordinator extends AbstractMutationCoordinator {
 			} );
 		}
 
-		public List<TableMapping> getTablesWithNonNullValues() {
-			return tablesWithNonNullValues;
-		}
-
 		public boolean hasNonNullBindings(TableMapping tableMapping) {
 			return tablesWithNonNullValues.contains( tableMapping );
 		}
@@ -145,9 +137,7 @@ public class InsertCoordinator extends AbstractMutationCoordinator {
 
 		final TableInclusionChecker tableInclusionChecker = (tableMapping) -> {
 			if ( tableMapping.isOptional() ) {
-				if ( !insertValuesAnalysis.hasNonNullBindings( tableMapping ) ) {
-					return false;
-				}
+				return insertValuesAnalysis.hasNonNullBindings( tableMapping );
 			}
 
 			return true;
@@ -251,12 +241,6 @@ public class InsertCoordinator extends AbstractMutationCoordinator {
 		mutationGroup.forEachOperation( (position, jdbcOperation) -> {
 			final EntityTableMapping tableDetails = (EntityTableMapping) jdbcOperation.getTableDetails();
 
-// todo (mutation) : this did not work at some point, but seems logical.  worth tracking down?
-//		this
-//			if ( !tableInclusionChecker.include( tableDetails ) ) {
-//				return;
-//			}
-
 			final String tableName = tableDetails.getTableName();
 
 			if ( id == null )  {
@@ -403,6 +387,7 @@ public class InsertCoordinator extends AbstractMutationCoordinator {
 			MutationGroupBuilder insertGroupBuilder,
 			boolean[] attributeInclusions) {
 		final List<AttributeMapping> attributeMappings = entityPersister().getAttributeMappings();
+		//noinspection resource
 		final Dialect dialect = factory().getJdbcServices().getDialect();
 
 		insertGroupBuilder.forEachTableMutationBuilder( (builder) -> {

hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java

@@ -7637,46 +7637,59 @@ public abstract class AbstractSqlAstTranslator<T extends JdbcOperation> implemen
 	}
 
 	private void renderInsertInto(TableInsertStandard tableInsert) {
+		if ( tableInsert.getNumberOfValueBindings() == 0 ) {
+			renderInsertIntoNoColumns( tableInsert );
+			return;
+		}
+
+		renderIntoIntoAndTable( tableInsert );
+
+		tableInsert.forEachValueBinding( (columnPosition, columnValueBinding) -> {
+			if ( columnPosition == 0 ) {
+				sqlBuffer.append( '(' );
+			}
+			else {
+				sqlBuffer.append( ',' );
+			}
+			sqlBuffer.append( columnValueBinding.getColumnReference().getColumnExpression() );
+		} );
+
+		getCurrentClauseStack().push( Clause.VALUES );
+		try {
+			sqlBuffer.append( ") values (" );
+
+			tableInsert.forEachValueBinding( (columnPosition, columnValueBinding) -> {
+				if ( columnPosition > 0 ) {
+					sqlBuffer.append( ',' );
+				}
+				columnValueBinding.getValueExpression().accept( this );
+			} );
+		}
+		finally {
+			getCurrentClauseStack().pop();
+		}
+
+		sqlBuffer.append( ")" );
+	}
+
+	/**
+	 * Renders the `insert into <table name> ` portion of an insert
+	 */
+	protected void renderIntoIntoAndTable(TableInsertStandard tableInsert) {
 		sqlBuffer.append( "insert into " );
 
 		appendSql( tableInsert.getMutatingTable().getTableName() );
 		registerAffectedTable( tableInsert.getMutatingTable().getTableName() );
 
 		sqlBuffer.append( ' ' );
-		if ( tableInsert.getNumberOfValueBindings() == 0 ) {
-			sqlBuffer.append( dialect.getNoColumnsInsertString() );
-		}
-		else {
-			tableInsert.forEachValueBinding( (columnPosition, columnValueBinding) -> {
-				if ( columnPosition == 0 ) {
-					sqlBuffer.append( '(' );
-				}
-				else {
-					sqlBuffer.append( ',' );
-				}
-				sqlBuffer.append( columnValueBinding.getColumnReference().getColumnExpression() );
-			} );
+	}
 
-			getCurrentClauseStack().push( Clause.VALUES );
-			try {
-				sqlBuffer.append( ") values " );
-
-				tableInsert.forEachValueBinding( (columnPosition, columnValueBinding) -> {
-					if ( columnPosition == 0 ) {
-						sqlBuffer.append( '(' );
-					}
-					else {
-						sqlBuffer.append( ',' );
-					}
-					columnValueBinding.getValueExpression().accept( this );
-				} );
-			}
-			finally {
-				getCurrentClauseStack().pop();
-			}
-
-			sqlBuffer.append( ")" );
-		}
+	/**
+	 * Handle rendering an insert with no columns (eye roll)
+	 */
+	protected void renderInsertIntoNoColumns(TableInsertStandard tableInsert) {
+		renderIntoIntoAndTable( tableInsert );
+		sqlBuffer.append( dialect.getNoColumnsInsertString() );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/sql/model/ast/builder/TableUpdateBuilderSkipped.java

@@ -35,42 +35,35 @@ public class TableUpdateBuilderSkipped implements TableUpdateBuilder {
 	@Override
 	public void addKeyRestriction(String columnName, String columnWriteFragment, JdbcMapping jdbcMapping) {
 		// nothing to do
-		// todo (mutation) : should this be an exception?
 	}
 
 	@Override
 	public void addNullOptimisticLockRestriction(SelectableMapping column) {
 		// nothing to do
-		// todo (mutation) : should this be an exception?
 	}
 
 	@Override
 	public void addOptimisticLockRestriction(String columnName, String columnWriteFragment, JdbcMapping jdbcMapping) {
 		// nothing to do
-		// todo (mutation) : should this be an exception?
 	}
 
 	@Override
 	public void addWhereFragment(String fragment) {
 		// nothing to do
-		// todo (mutation) : should this be an exception?
 	}
 
 	@Override
 	public void addValueColumn(String columnName, String columnWriteFragment, JdbcMapping jdbcMapping) {
 		// nothing to do
-		// todo (mutation) : should this be an exception?
 	}
 
 	@Override
 	public void addKeyColumn(String columnName, String valueExpression, JdbcMapping jdbcMapping) {
 		// nothing to do
-		// todo (mutation) : should this be an exception?
 	}
 
 	@Override
 	public void setWhere(String fragment) {
 		// nothing to do
-		// todo (mutation) : should this be an exception?
 	}
 }