HHH-19364 proper implementation of validate()

because @sebersole complained

Author: gavinking

hibernate-core/src/main/java/org/hibernate/query/programmatic/MutationSpecification.java

@@ -76,6 +76,9 @@ interface Augmentation<T> {
 	@Override
 	MutationQuery createQuery(StatelessSession session);
 
+	@Override
+	MutationSpecification<T> validate(CriteriaBuilder builder);
+
 	/**
 	 * Returns a specification reference which can be used to programmatically,
 	 * iteratively build a {@linkplain MutationQuery} based on a base HQL statement,

hibernate-core/src/main/java/org/hibernate/query/programmatic/QuerySpecification.java

@@ -10,9 +10,7 @@
 
 import org.hibernate.Incubating;
 import org.hibernate.Session;
-import org.hibernate.SessionFactory;
 import org.hibernate.StatelessSession;
-import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.query.CommonQueryContract;
 import org.hibernate.query.restriction.Restriction;
 
@@ -69,15 +67,9 @@ public interface QuerySpecification<T> {
 
 	/**
 	 * Validate the query.
+	 *
+	 * @return {@code this} if everything is fine
+	 * @throws Exception if it ain't all good
 	 */
-	default void validate(SessionFactory factory) {
-		// Extremely temporary implementation.
-		// We don't actually want to open a session here,
-		// nor create an instance of CommonQueryContract.
-		final SessionFactoryImplementor factoryImplementor =
-				(SessionFactoryImplementor) factory;
-		try ( var session = factoryImplementor.openTemporarySession() ) {
-			createQuery( session );
-		}
-	}
+	QuerySpecification<T> validate(CriteriaBuilder builder);
 }

hibernate-core/src/main/java/org/hibernate/query/programmatic/SelectionSpecification.java

@@ -177,8 +177,12 @@ interface Augmentation<T> {
 	 *
 	 * @return a new criteria query
 	 */
+	@Override
 	CriteriaQuery<T> buildCriteriaQuery(CriteriaBuilder builder);
 
+	@Override
+	SelectionSpecification<T> validate(CriteriaBuilder builder);
+
 	/**
 	 * Returns a specification reference which can be used to programmatically,
 	 * iteratively build a {@linkplain SelectionQuery} for the given entity type,

hibernate-core/src/main/java/org/hibernate/query/programmatic/internal/MutationSpecificationImpl.java

@@ -25,6 +25,7 @@
 import org.hibernate.query.sqm.SqmQuerySource;
 import org.hibernate.query.sqm.internal.QuerySqmImpl;
 import org.hibernate.query.sqm.internal.SqmUtil;
+import org.hibernate.query.sqm.tree.AbstractSqmDmlStatement;
 import org.hibernate.query.sqm.tree.SqmDeleteOrUpdateStatement;
 import org.hibernate.query.sqm.tree.delete.SqmDeleteStatement;
 import org.hibernate.query.sqm.tree.from.SqmRoot;
@@ -133,6 +134,14 @@ public CommonAbstractCriteria buildCriteriaQuery(CriteriaBuilder builder) {
 		return build( nodeBuilder.getQueryEngine() );
 	}
 
+	@Override
+	public MutationSpecification<T> validate(CriteriaBuilder builder) {
+		final NodeBuilder nodeBuilder = (NodeBuilder) builder;
+		final var statement = build( nodeBuilder.getQueryEngine() );
+		( (AbstractSqmDmlStatement<?>) statement ).validate( hql );
+		return this;
+	}
+
 	/**
 	 * Used during construction to parse/interpret the incoming HQL
 	 * and produce the corresponding SQM tree.

hibernate-core/src/main/java/org/hibernate/query/programmatic/internal/SelectionSpecificationImpl.java

@@ -38,6 +38,7 @@
 import java.util.Set;
 import java.util.function.BiConsumer;
 
+import static org.hibernate.query.sqm.internal.SqmUtil.validateCriteriaQuery;
 import static org.hibernate.query.sqm.tree.SqmCopyContext.noParamCopyContext;
 
 /**
@@ -176,6 +177,18 @@ public CriteriaQuery<T> buildCriteriaQuery(CriteriaBuilder builder) {
 		return build( nodeBuilder.getQueryEngine() );
 	}
 
+	@Override
+	public SelectionSpecification<T> validate(CriteriaBuilder builder) {
+		final NodeBuilder nodeBuilder = (NodeBuilder) builder;
+		final var statement = build( nodeBuilder.getQueryEngine() );
+		final var queryPart = statement.getQueryPart();
+		// For criteria queries, we have to validate the fetch structure here
+		queryPart.validateQueryStructureAndFetchOwners();
+		validateCriteriaQuery( queryPart );
+		statement.validateResultType( resultType );
+		return this;
+	}
+
 	/**
 	 * Used during construction to parse/interpret the incoming HQL
 	 * and produce the corresponding SQM tree.

hibernate-core/src/main/java/org/hibernate/query/sqm/internal/AbstractSqmSelectionQuery.java

@@ -33,10 +33,6 @@
 import org.hibernate.query.sqm.tree.expression.JpaCriteriaParameter;
 import org.hibernate.query.sqm.tree.expression.SqmJpaCriteriaParameterWrapper;
 import org.hibernate.query.sqm.tree.expression.SqmParameter;
-import org.hibernate.query.sqm.tree.from.SqmRoot;
-import org.hibernate.query.sqm.tree.select.SqmQueryGroup;
-import org.hibernate.query.sqm.tree.select.SqmQueryPart;
-import org.hibernate.query.sqm.tree.select.SqmQuerySpec;
 import org.hibernate.query.sqm.tree.select.SqmSelectStatement;
 import org.hibernate.query.sqm.tree.select.SqmSelectableNode;
 import org.hibernate.query.sqm.tree.select.SqmSelection;
@@ -421,33 +417,6 @@ private static String[] buildTupleAliasArray(List<SqmSelection<?>> selections) {
 		}
 	}
 
-	protected static void validateCriteriaQuery(SqmQueryPart<?> queryPart) {
-		if ( queryPart instanceof SqmQuerySpec<?> sqmQuerySpec ) {
-			if ( sqmQuerySpec.getSelectClause().getSelections().isEmpty() ) {
-				// make sure there is at least one root
-				final List<SqmRoot<?>> sqmRoots = sqmQuerySpec.getFromClause().getRoots();
-				if ( sqmRoots == null || sqmRoots.isEmpty() ) {
-					throw new IllegalArgumentException( "Criteria did not define any query roots" );
-				}
-				// if there is a single root, use that as the selection
-				if ( sqmRoots.size() == 1 ) {
-					sqmQuerySpec.getSelectClause().add( sqmRoots.get( 0 ), null );
-				}
-				else {
-					throw new IllegalArgumentException( "Criteria has multiple query roots" );
-				}
-			}
-		}
-		else if ( queryPart instanceof SqmQueryGroup<?> queryGroup ) {
-			for ( SqmQueryPart<?> part : queryGroup.getQueryParts() ) {
-				validateCriteriaQuery( part );
-			}
-		}
-		else {
-			assert false;
-		}
-	}
-
 	protected static <T> HqlInterpretation<T> interpretation(
 			NamedSqmQueryMemento<?> memento,
 			Class<T> expectedResultType,

hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java

@@ -106,6 +106,7 @@
 import static org.hibernate.query.sqm.internal.SqmInterpretationsKey.createInterpretationsKey;
 import static org.hibernate.query.sqm.internal.SqmInterpretationsKey.generateNonSelectKey;
 import static org.hibernate.query.sqm.internal.SqmUtil.isSelect;
+import static org.hibernate.query.sqm.internal.SqmUtil.validateCriteriaQuery;
 import static org.hibernate.query.sqm.internal.SqmUtil.verifyIsNonSelectStatement;
 
 /**
@@ -244,6 +245,14 @@ public QuerySqmImpl(
 				bindCriteriaParameter( wrapper );
 			}
 		}
+
+		validateQuery( expectedResultType, sqm, hql );
+
+		resultType = expectedResultType;
+		tupleMetadata = buildTupleMetadata( criteria, expectedResultType );
+	}
+
+	private static <R> void validateQuery(Class<R> expectedResultType, SqmStatement<R> sqm, String hql) {
 		if ( sqm instanceof SqmSelectStatement<R> selectStatement ) {
 			final SqmQueryPart<R> queryPart = selectStatement.getQueryPart();
 			// For criteria queries, we have to validate the fetch structure here
@@ -257,9 +266,6 @@ else if ( sqm instanceof AbstractSqmDmlStatement<R> update ) {
 			}
 			update.validate( hql );
 		}
-
-		resultType = expectedResultType;
-		tupleMetadata = buildTupleMetadata( criteria, expectedResultType );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java

@@ -214,7 +214,7 @@ public SqmSelectionQueryImpl(
 		final SqmQueryPart<R> queryPart = sqm.getQueryPart();
 		// For criteria queries, we have to validate the fetch structure here
 		queryPart.validateQueryStructureAndFetchOwners();
-		validateCriteriaQuery( queryPart );
+		SqmUtil.validateCriteriaQuery( queryPart );
 		sqm.validateResultType( resultType );
 
 		setComment( hql );

hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java

@@ -977,6 +977,33 @@ public static <X> SqmPredicate restriction(
 		return  (SqmPredicate) restriction.toPredicate( root, sqmStatement.nodeBuilder() );
 	}
 
+	public static void validateCriteriaQuery(SqmQueryPart<?> queryPart) {
+		if ( queryPart instanceof SqmQuerySpec<?> sqmQuerySpec ) {
+			if ( sqmQuerySpec.getSelectClause().getSelections().isEmpty() ) {
+				// make sure there is at least one root
+				final List<SqmRoot<?>> sqmRoots = sqmQuerySpec.getFromClause().getRoots();
+				if ( sqmRoots == null || sqmRoots.isEmpty() ) {
+					throw new IllegalArgumentException( "Criteria did not define any query roots" );
+				}
+				// if there is a single root, use that as the selection
+				if ( sqmRoots.size() == 1 ) {
+					sqmQuerySpec.getSelectClause().add( sqmRoots.get( 0 ), null );
+				}
+				else {
+					throw new IllegalArgumentException( "Criteria has multiple query roots" );
+				}
+			}
+		}
+		else if ( queryPart instanceof SqmQueryGroup<?> queryGroup ) {
+			for ( SqmQueryPart<?> part : queryGroup.getQueryParts() ) {
+				validateCriteriaQuery( part );
+			}
+		}
+		else {
+			assert false;
+		}
+	}
+
 	private static class CriteriaParameterCollector {
 		private Set<SqmParameter<?>> sqmParameters;
 		private Map<JpaCriteriaParameter<?>, List<SqmJpaCriteriaParameterWrapper<?>>> jpaCriteriaParamResolutions;

hibernate-core/src/test/java/org/hibernate/orm/test/query/dynamic/SimpleQuerySpecificationTests.java

@@ -4,6 +4,9 @@
  */
 package org.hibernate.orm.test.query.dynamic;
 
+import jakarta.persistence.criteria.CommonAbstractCriteria;
+import jakarta.persistence.criteria.CriteriaQuery;
+
 import org.hibernate.SessionFactory;
 import org.hibernate.query.IllegalMutationQueryException;
 import org.hibernate.query.IllegalSelectQueryException;
@@ -255,7 +258,7 @@ void testIllegalSelection(SessionFactoryScope factoryScope) {
 		final SessionFactory factory = factoryScope.getSessionFactory();
 		try {
 			SelectionSpecification.create( BasicEntity.class, "delete BasicEntity" )
-					.validate( factory );
+					.validate( factory.getCriteriaBuilder() );
 			fail( "Expecting a IllegalSelectQueryException, but not thrown" );
 		}
 		catch (IllegalSelectQueryException expected) {
@@ -267,10 +270,30 @@ void testIllegalMutation(SessionFactoryScope factoryScope) {
 		final SessionFactory factory = factoryScope.getSessionFactory();
 		try {
 			MutationSpecification.create( BasicEntity.class, "from BasicEntity" )
-					.validate( factory );
+					.validate( factory.getCriteriaBuilder() );
 			fail( "Expecting a IllegalMutationQueryException, but not thrown" );
 		}
 		catch (IllegalMutationQueryException expected) {
 		}
 	}
+
+	@Test
+	void testBuildCriteriaDelete(SessionFactoryScope factoryScope) {
+		final SessionFactory factory = factoryScope.getSessionFactory();
+		CommonAbstractCriteria deleteBasicEntity =
+				MutationSpecification.create( BasicEntity.class,
+								"delete BasicEntity" )
+						.validate( factory.getCriteriaBuilder() )
+						.buildCriteriaQuery( factory.getCriteriaBuilder() );
+	}
+
+	@Test
+	void testBuildCriteriaQuery(SessionFactoryScope factoryScope) {
+		final SessionFactory factory = factoryScope.getSessionFactory();
+		CriteriaQuery<BasicEntity> query =
+				SelectionSpecification.create( BasicEntity.class,
+								"from BasicEntity" )
+						.validate( factory.getCriteriaBuilder() )
+						.buildCriteriaQuery( factory.getCriteriaBuilder() );
+	}
 }