finally sort out handling of @id and @Version in query validator

also remove 'this' hacks made obsolete by Steve's work on core

Author: gavinking

hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/AbstractIdentifiableType.java

@@ -420,11 +420,10 @@ else if ( id != null ) {
 			}
 			else {
 				assert type instanceof EmbeddableDomainType;
-				final EmbeddableDomainType<?> compositeType = (EmbeddableDomainType<?>) type;
 				return new EmbeddedSqmPathSource<>(
 						EntityIdentifierMapping.ID_ROLE_NAME,
 						(SqmPathSource) id,
-						compositeType,
+						(EmbeddableDomainType<?>) type,
 						Bindable.BindableType.SINGULAR_ATTRIBUTE,
 						id.isGeneric()
 				);

hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java

@@ -2971,17 +2971,23 @@ public SqmPath<?> visitEntityIdReference(HqlParser.EntityIdReferenceContext ctx)
 
 		final SqmPath<?> sqmPath = consumeDomainPath( ctx.path() );
 		final DomainType<?> sqmPathType = sqmPath.getReferencedPathSource().getSqmPathType();
-
 		if ( sqmPathType instanceof IdentifiableDomainType<?> ) {
-			final SqmPathSource<?> identifierDescriptor = ( (IdentifiableDomainType<?>) sqmPathType ).getIdentifierDescriptor();
+			final IdentifiableDomainType<?> identifiableType = (IdentifiableDomainType<?>) sqmPathType;
+			final SqmPathSource<?> identifierDescriptor = identifiableType.getIdentifierDescriptor();
+			if ( identifierDescriptor == null ) {
+				// mainly for benefit of Hibernate Processor
+				throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+						+ "' of 'id()' is a '" + identifiableType.getTypeName()
+						+ "' and does not have a well-defined '@Id' attribute" );
+			}
 			return sqmPath.get( identifierDescriptor.getPathName() );
 		}
 		else if ( sqmPath instanceof SqmAnyValuedSimplePath<?> ) {
 			return sqmPath.resolvePathPart( AnyKeyPart.KEY_NAME, true, processingStateStack.getCurrent().getCreationState() );
 		}
 		else {
 			throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
-					+ "' of 'id()' function does not resolve to an entity type" );
+					+ "' of 'id()' does not resolve to an entity type" );
 		}
 	}
 
@@ -2994,26 +3000,22 @@ public SqmExpression<?> visitEntityVersionExpression(HqlParser.EntityVersionExpr
 	public SqmPath<?> visitEntityVersionReference(HqlParser.EntityVersionReferenceContext ctx) {
 		final SqmPath<?> sqmPath = consumeDomainPath( ctx.path() );
 		final DomainType<?> sqmPathType = sqmPath.getReferencedPathSource().getSqmPathType();
-
 		if ( sqmPathType instanceof IdentifiableDomainType<?> ) {
-			@SuppressWarnings("unchecked")
-			final IdentifiableDomainType<Object> identifiableType = (IdentifiableDomainType<Object>) sqmPathType;
-			final SingularPersistentAttribute<Object, ?> versionAttribute = identifiableType.findVersionAttribute();
-			if ( versionAttribute == null ) {
-				throw new FunctionArgumentException(
-						String.format(
-								"Argument '%s' of 'version()' function resolved to entity type '%s' which does not have a '@Version' attribute",
-								sqmPath.getNavigablePath(),
-								identifiableType.getTypeName()
-						)
-				);
+			final IdentifiableDomainType<?> identifiableType = (IdentifiableDomainType<?>) sqmPathType;
+			if ( !identifiableType.hasVersionAttribute() ) {
+				throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+						+ "' of 'version()' is a '" + identifiableType.getTypeName()
+						+ "' and does not have a '@Version' attribute" );
 			}
-
+			@SuppressWarnings("unchecked")
+			final SingularPersistentAttribute<Object, ?> versionAttribute =
+					(SingularPersistentAttribute<Object, ?>) identifiableType.findVersionAttribute();
 			return sqmPath.get( versionAttribute );
 		}
-
-		throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
-				+ "' of 'version()' function does not resolve to an entity type" );
+		else {
+			throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+					+ "' of 'version()' does not resolve to an entity type" );
+		}
 	}
 
 	@Override
@@ -3032,35 +3034,29 @@ public SqmPath<?> visitEntityNaturalIdReference(HqlParser.EntityNaturalIdReferen
 
 		if ( sqmPathType instanceof IdentifiableDomainType<?> ) {
 			@SuppressWarnings("unchecked")
-			final IdentifiableDomainType<Object> identifiableType = (IdentifiableDomainType<? super Object>) sqmPathType;
+			final IdentifiableDomainType<Object> identifiableType = (IdentifiableDomainType<Object>) sqmPathType;
 			final List<? extends PersistentAttribute<Object, ?>> attributes = identifiableType.findNaturalIdAttributes();
 			if ( attributes == null ) {
-				throw new FunctionArgumentException(
-						String.format(
-								"Argument '%s' of 'naturalid()' function resolved to entity type '%s' which does not have a natural id",
-								sqmPath.getNavigablePath(),
-								identifiableType.getTypeName()
-						)
+				throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+						+ "' of 'naturalid()' is a '" + identifiableType.getTypeName()
+						+ "' and does not have a natural id"
 				);
 			}
 			else if ( attributes.size() >1 ) {
-				throw new FunctionArgumentException(
-						String.format(
-								"Argument '%s' of 'naturalid()' function resolved to entity type '%s' which has a composite natural id",
-								sqmPath.getNavigablePath(),
-								identifiableType.getTypeName()
-						)
+				throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
+						+ "' of 'naturalid()' is a '" + identifiableType.getTypeName()
+						+ "' and has a composite natural id"
 				);
 			}
 
 			@SuppressWarnings("unchecked")
-			SingularAttribute<Object, ?> naturalIdAttribute
+			final SingularAttribute<Object, ?> naturalIdAttribute
 					= (SingularAttribute<Object, ?>) attributes.get(0);
 			return sqmPath.get( naturalIdAttribute );
 		}
 
 		throw new FunctionArgumentException( "Argument '" + sqmPath.getNavigablePath()
-				+ "' of 'naturalid()' function does not resolve to an entity type" );
+				+ "' of 'naturalid()' does not resolve to an entity type" );
 	}
 //
 //	@Override

tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/Concrete.java

@@ -3,5 +3,5 @@
 import jakarta.data.repository.Repository;
 
 @Repository
-public interface Concrete extends IdOperations<Book> {
+public interface Concrete extends IdOperations<Thing> {
 }

tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/DataTest.java

@@ -19,7 +19,7 @@
  */
 public class DataTest extends CompilationTest {
 	@Test
-	@WithClasses({ Author.class, Book.class, BookAuthorRepository.class, IdOperations.class, Concrete.class })
+	@WithClasses({ Author.class, Book.class, BookAuthorRepository.class, IdOperations.class, Concrete.class, Thing.class })
 	public void test() {
 		System.out.println( getMetaModelSourceAsString( Author.class ) );
 		System.out.println( getMetaModelSourceAsString( Book.class ) );

tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/IdOperations.java

@@ -23,11 +23,6 @@
 import jakarta.data.Sort;
 import jakarta.data.repository.Query;
 
-/**
- * This interface contains common operations for the NaturalNumbers and AsciiCharacters repositories.
- *
- * @param <T> type of entity.
- */
 public interface IdOperations<T> {
     @Query("where id(this) between ?1 and ?2")
     Stream<T> findByIdBetween(long minimum, long maximum, Sort<T> sort);

tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/basic/Thing.java

@@ -0,0 +1,9 @@
+package org.hibernate.processor.test.data.basic;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+
+@Entity
+public class Thing {
+	@Id long id;
+}

tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/SpecialVersioned.java

@@ -0,0 +1,7 @@
+package org.hibernate.processor.test.data.versioned;
+
+import jakarta.persistence.Entity;
+
+@Entity
+public class SpecialVersioned extends Versioned {
+}

tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/SpecialVersionedRepo.java

@@ -0,0 +1,17 @@
+package org.hibernate.processor.test.data.versioned;
+
+import jakarta.data.repository.Query;
+import jakarta.data.repository.Repository;
+
+@Repository
+public interface SpecialVersionedRepo {
+	@Query("where id(this) = ?1")
+	SpecialVersioned forId(long id);
+
+	@Query("where id(this) = ?1 and version(this) = ?2")
+	SpecialVersioned forIdAndVersion(long id, int version);
+
+	@Query("select count(this) from SpecialVersioned")
+	long count();
+
+}

tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/Versioned.java

@@ -0,0 +1,11 @@
+package org.hibernate.processor.test.data.versioned;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.Version;
+
+@Entity
+public class Versioned {
+	@Id long id;
+	@Version int version;
+}

tooling/metamodel-generator/src/jakartaData/java/org/hibernate/processor/test/data/versioned/VersionedRepo.java

@@ -0,0 +1,17 @@
+package org.hibernate.processor.test.data.versioned;
+
+import jakarta.data.repository.Query;
+import jakarta.data.repository.Repository;
+
+@Repository
+public interface VersionedRepo {
+	@Query("where id(this) = ?1")
+	Versioned forId(long id);
+
+	@Query("where id(this) = ?1 and version(this) = ?2")
+	Versioned forIdAndVersion(long id, int version);
+
+	@Query("select count(this) from Versioned")
+	long count();
+
+}