Skip to content

Commit e331c28

Browse files
gavinkinggavinking
committed
more cosmetic improvements to HQL error reporting
makes the messages and exception types a bit more consistent
1 parent 60ad64b commit e331c28

File tree

6 files changed

+53
-32
lines changed

6 files changed

+53
-32
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ private ConsumerDelegate resolveBase(String identifier, boolean isTerminal) {
118118
// identifier is an alias (identification variable)
119119

120120
if ( isTerminal ) {
121-
throw new SemanticException( "Cannot join to root : " + identifier );
121+
throw new SemanticException( "Cannot join to root '" + identifier + "'" );
122122
}
123123

124124
return new AttributeJoinDelegate(
@@ -181,9 +181,9 @@ private static SqmFrom<?, Object> createJoin(
181181
throw new HqlInterpretationException(
182182
String.format(
183183
Locale.ROOT,
184-
"Could not locate specified joinable path : %s -> %s",
185-
lhs.getNavigablePath(),
186-
name
184+
"Could not resolve joined attribute '%s' of '%s'",
185+
name,
186+
lhs.getNavigablePath()
187187
)
188188
);
189189
}

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

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,10 @@ public SqmInsertStatement<R> visitInsertStatement(HqlParser.InsertStatementConte
423423
final SqmRoot<R> root = visitTargetEntity( dmlTargetContext );
424424
if ( root.getReferencedPathSource() instanceof SqmPolymorphicRootDescriptor<?> ) {
425425
throw new SemanticException(
426-
"Can't create an INSERT for a non entity name: " + root.getReferencedPathSource().getHibernateEntityName()
426+
String.format(
427+
"Target type '%s' in insert statement is not an entity",
428+
root.getReferencedPathSource().getHibernateEntityName()
429+
)
427430
);
428431
}
429432

@@ -511,7 +514,10 @@ public SqmUpdateStatement<R> visitUpdateStatement(HqlParser.UpdateStatementConte
511514
final SqmRoot<R> root = visitTargetEntity( dmlTargetContext );
512515
if ( root.getReferencedPathSource() instanceof SqmPolymorphicRootDescriptor<?> ) {
513516
throw new SemanticException(
514-
"Can't create an UPDATE for a non entity name: " + root.getReferencedPathSource().getHibernateEntityName()
517+
String.format(
518+
"Target type '%s' in update statement is not an entity",
519+
root.getReferencedPathSource().getHibernateEntityName()
520+
)
515521
);
516522
}
517523

@@ -779,7 +785,7 @@ else if ( fetchClauseContext == null ) {
779785
sqmQueryPart.setFetchExpression( visitLimitClause( limitClauseContext ) );
780786
}
781787
else {
782-
throw new SemanticException("Can't use both, limit and fetch clause!" );
788+
throw new SemanticException("Can't use both limit and fetch clause" );
783789
}
784790
}
785791
}
@@ -961,7 +967,7 @@ public SqmDynamicInstantiation<?> visitInstantiation(HqlParser.InstantiationCont
961967
);
962968
}
963969
catch (ClassLoadingException e) {
964-
throw new SemanticException( "Unable to resolve class named for dynamic instantiation : " + className );
970+
throw new SemanticException( "Could not resolve class '" + className + "' named for instantiation" );
965971
}
966972
}
967973
else {
@@ -1362,7 +1368,7 @@ public EntityDomainType<?> visitEntityName(HqlParser.EntityNameContext parserEnt
13621368
.getJpaMetamodel()
13631369
.getHqlEntityReference( entityName );
13641370
if ( entityReference == null ) {
1365-
throw new UnknownEntityException( "Could not resolve entity name [" + entityName + "] as DML target", entityName );
1371+
throw new UnknownEntityException( "Could not resolve target entity '" + entityName + "'", entityName );
13661372
}
13671373
checkFQNEntityNameJpaComplianceViolationIfNeeded( entityName, entityReference );
13681374
if ( entityReference instanceof SqmPolymorphicRootDescriptor<?> && getCreationOptions().useStrictJpaCompliance() ) {
@@ -1476,7 +1482,7 @@ public SqmRoot<?> visitRootEntity(HqlParser.RootEntityContext ctx) {
14761482
}
14771483
throw new SemanticException( "Could not resolve entity or correlation path '" + name + "'" );
14781484
}
1479-
throw new SemanticException( "Could not resolve entity '" + name + "'" );
1485+
throw new UnknownEntityException( "Could not resolve root entity '" + name + "'", name );
14801486
}
14811487
checkFQNEntityNameJpaComplianceViolationIfNeeded( name, entityDescriptor );
14821488

@@ -1491,7 +1497,7 @@ public SqmRoot<?> visitRootEntity(HqlParser.RootEntityContext ctx) {
14911497

14921498
if ( processingStateStack.depth() > 1 ) {
14931499
throw new SemanticException(
1494-
"Illegal implicit-polymorphic domain path in sub-query : " + entityDescriptor.getName()
1500+
"Illegal implicit-polymorphic domain path in subquery '" + entityDescriptor.getName() +"'"
14951501
);
14961502
}
14971503
}
@@ -2140,7 +2146,7 @@ public SqmPath<?> visitEntityIdReference(HqlParser.EntityIdReferenceContext ctx)
21402146
throw new NotYetImplementedFor6Exception( "Path continuation from `id()` reference not yet implemented" );
21412147
}
21422148

2143-
throw new SemanticException( "Path does not reference an identifiable-type : " + sqmPath.getNavigablePath().getFullPath() );
2149+
throw new SemanticException( "Path does not resolve to an entity type '" + sqmPath.getNavigablePath().getFullPath() + "'" );
21442150
}
21452151

21462152
@Override
@@ -2159,15 +2165,17 @@ public SqmPath<?> visitEntityVersionReference(HqlParser.EntityVersionReferenceCo
21592165
final SingularPersistentAttribute<Object, ?> versionAttribute = identifiableType.findVersionAttribute();
21602166
if ( versionAttribute == null ) {
21612167
throw new SemanticException(
2162-
"`" + sqmPath.getNavigablePath().getFullPath() + "` resolved to an identifiable-type (`" +
2163-
identifiableType.getTypeName() + "`) which does not define a version"
2168+
String.format(
2169+
"Path '%s' resolved to entity type '%s' which does not define a version",
2170+
sqmPath.getNavigablePath().getFullPath(), identifiableType.getTypeName()
2171+
)
21642172
);
21652173
}
21662174

21672175
return sqmPath.get( versionAttribute );
21682176
}
21692177

2170-
throw new SemanticException( "Path does not reference an identifiable-type : " + sqmPath.getNavigablePath().getFullPath() );
2178+
throw new SemanticException( "Path does not resolve to an entity type '" + sqmPath.getNavigablePath().getFullPath() + "'" );
21712179
}
21722180

21732181
@Override
@@ -2186,14 +2194,18 @@ public SqmPath<?> visitEntityNaturalIdReference(HqlParser.EntityNaturalIdReferen
21862194
final List<? extends PersistentAttribute<Object, ?>> attributes = identifiableType.findNaturalIdAttributes();
21872195
if ( attributes == null ) {
21882196
throw new SemanticException(
2189-
"`" + sqmPath.getNavigablePath().getFullPath() + "` resolved to an identifiable-type (`" +
2190-
identifiableType.getTypeName() + "`) which does not define a natural id"
2197+
String.format(
2198+
"Path '%s' resolved to entity type '%s' which does not define a natural id",
2199+
sqmPath.getNavigablePath().getFullPath(), identifiableType.getTypeName()
2200+
)
21912201
);
21922202
}
21932203
else if ( attributes.size() >1 ) {
21942204
throw new SemanticException(
2195-
"`" + sqmPath.getNavigablePath().getFullPath() + "` resolved to an identifiable-type (`" +
2196-
identifiableType.getTypeName() + "`) which defines multiple natural ids"
2205+
String.format(
2206+
"Path '%s' resolved to entity type '%s' which defines multiple natural ids",
2207+
sqmPath.getNavigablePath().getFullPath(), identifiableType.getTypeName()
2208+
)
21972209
);
21982210
}
21992211

@@ -2203,7 +2215,7 @@ else if ( attributes.size() >1 ) {
22032215
return sqmPath.get( naturalIdAttribute );
22042216
}
22052217

2206-
throw new SemanticException( "Path does not reference an identifiable-type : " + sqmPath.getNavigablePath().getFullPath() );
2218+
throw new SemanticException( "Path does not resolve to an entity type '" + sqmPath.getNavigablePath().getFullPath() + "'" );
22072219
}
22082220

22092221
@Override
@@ -3501,7 +3513,7 @@ public Object visitExtractFunction(HqlParser.ExtractFunctionContext ctx) {
35013513
public Object visitFormat(HqlParser.FormatContext ctx) {
35023514
String format = QuotingHelper.unquoteStringLiteral( ctx.getChild( 0 ).getText() );
35033515
if (!FORMAT.matcher(format).matches()) {
3504-
throw new SemanticException("illegal format pattern: '" + format + "'");
3516+
throw new SemanticException("illegal format pattern '" + format + "'");
35053517
}
35063518
return new SqmFormat(
35073519
format,
@@ -3681,7 +3693,7 @@ private <X> SqmSubQuery<X> createCollectionReferenceSubQuery(
36813693

36823694
if ( !(referencedPathSource instanceof PluralPersistentAttribute ) ) {
36833695
throw new PathException(
3684-
"Illegal attempt to treat non-plural path as a plural path : " + pluralAttributePath.getNavigablePath()
3696+
"Path is not a plural path '" + pluralAttributePath.getNavigablePath() + "'"
36853697
);
36863698
}
36873699
final SqmSubQuery<?> subQuery = new SqmSubQuery<>(
@@ -3809,7 +3821,7 @@ public SqmLiteral<Character> visitPadCharacter(HqlParser.PadCharacterContext ctx
38093821
final String padCharText = ctx.STRING_LITERAL().getText();
38103822

38113823
if ( padCharText.length() != 3 ) {
3812-
throw new SemanticException( "Pad character for pad() function must be single character, found: " + padCharText );
3824+
throw new SemanticException( "Pad character for pad() function must be single character, found '" + padCharText + "'" );
38133825
}
38143826

38153827
return new SqmLiteral<>(
@@ -3878,7 +3890,7 @@ public SqmLiteral<Character> visitTrimCharacter(HqlParser.TrimCharacterContext c
38783890
: " "; // JPA says space is the default
38793891

38803892
if ( trimCharText.length() != 1 ) {
3881-
throw new SemanticException( "Trim character for trim() function must be single character, found: " + trimCharText );
3893+
throw new SemanticException( "Trim character for trim() function must be single character, found '" + trimCharText + "'" );
38823894
}
38833895

38843896
return new SqmLiteral<>(
@@ -4181,10 +4193,15 @@ protected void reset() {
41814193
public SqmPath<?> visitCollectionElementNavigablePath(HqlParser.CollectionElementNavigablePathContext ctx) {
41824194
final SqmPath<?> pluralAttributePath = consumeDomainPath( (HqlParser.PathContext) ctx.getChild( 2 ) );
41834195
final SqmPathSource<?> referencedPathSource = pluralAttributePath.getReferencedPathSource();
4196+
final TerminalNode firstNode = (TerminalNode) ctx.getChild( 0 );
41844197

41854198
if ( !(referencedPathSource instanceof PluralPersistentAttribute<?, ?, ?> ) ) {
41864199
throw new PathException(
4187-
"Illegal attempt to treat non-plural path as a plural path : " + pluralAttributePath.getNavigablePath()
4200+
String.format(
4201+
"Argument of '%s' is not a plural path '%s'",
4202+
firstNode.getSymbol().getText(),
4203+
pluralAttributePath.getNavigablePath()
4204+
)
41884205
);
41894206
}
41904207

@@ -4194,7 +4211,6 @@ public SqmPath<?> visitCollectionElementNavigablePath(HqlParser.CollectionElemen
41944211
if ( attribute.getCollectionClassification() != CollectionClassification.MAP ) {
41954212
throw new StrictJpaComplianceViolation( StrictJpaComplianceViolation.Type.VALUE_FUNCTION_ON_NON_MAP );
41964213
}
4197-
final TerminalNode firstNode = (TerminalNode) ctx.getChild( 0 );
41984214
if ( firstNode.getSymbol().getType() == HqlParser.ELEMENTS ) {
41994215
throw new StrictJpaComplianceViolation( StrictJpaComplianceViolation.Type.HQL_COLLECTION_FUNCTION );
42004216
}
@@ -4219,8 +4235,13 @@ public SqmPath<?> visitCollectionIndexNavigablePath(HqlParser.CollectionIndexNav
42194235
final SqmPathSource<?> referencedPathSource = pluralAttributePath.getReferencedPathSource();
42204236

42214237
if ( !(referencedPathSource instanceof PluralPersistentAttribute<?, ?, ?> ) ) {
4238+
final TerminalNode firstNode = (TerminalNode) ctx.getChild( 0 );
42224239
throw new PathException(
4223-
"Illegal attempt to treat non-plural path as a plural path : " + pluralAttributePath.getNavigablePath()
4240+
String.format(
4241+
"Argument of '%s' is not a plural path '%s'",
4242+
firstNode.getSymbol().getText(),
4243+
pluralAttributePath.getNavigablePath()
4244+
)
42244245
);
42254246
}
42264247

‎hibernate-core/src/main/java/org/hibernate/query/sqm/UnknownEntityException.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class UnknownEntityException extends SemanticException {
2222
private final String entityName;
2323

2424
public UnknownEntityException(String entityName) {
25-
this( "Unable to resolve [" + entityName + "] as entity", entityName );
25+
this( "Could not resolve entity '" + entityName + "'", entityName );
2626
}
2727

2828
public UnknownEntityException(String message, String entityName) {

‎hibernate-core/src/test/java/org/hibernate/orm/test/annotations/ConfigurationTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
import org.hibernate.cfg.Configuration;
1616
import org.hibernate.cfg.Environment;
1717
import org.hibernate.engine.spi.SessionFactoryImplementor;
18-
import org.hibernate.query.SemanticException;
1918

19+
import org.hibernate.query.sqm.UnknownEntityException;
2020
import org.hibernate.test.annotations.Boat;
2121
import org.hibernate.test.annotations.Ferry;
2222
import org.hibernate.test.annotations.Port;
@@ -68,7 +68,7 @@ public void testIgnoringHbm() {
6868
fail( "Boat should not be mapped" );
6969
}
7070
catch (IllegalArgumentException expected) {
71-
assertEquals( SemanticException.class, expected.getCause().getClass() );
71+
assertEquals( UnknownEntityException.class, expected.getCause().getClass() );
7272
// expected outcome
7373

7474
// see org.hibernate.test.jpa.compliance.tck2_2.QueryApiTest#testInvalidQueryMarksTxnForRollback

‎hibernate-core/src/test/java/org/hibernate/orm/test/annotations/loader/LoaderWithInvalidQueryTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public void buildEntityManagerFactory() {
4444
Throwable[] suppressed = rootCause.getSuppressed();
4545
assertEquals( 2, suppressed.length );
4646
assertTrue( ExceptionUtil.rootCause( suppressed[0] ).getMessage().contains( "Could not resolve attribute 'valid'" ) );
47-
assertTrue( ExceptionUtil.rootCause( suppressed[1] ).getMessage().contains( "Could not resolve entity '_Person'" ) );
47+
assertTrue( ExceptionUtil.rootCause( suppressed[1] ).getMessage().contains( "Could not resolve root entity '_Person'" ) );
4848
}
4949
}
5050

‎hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/MappedSuperclassInheritanceTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public void test() {
7676
fail();
7777
} catch (Exception expected) {
7878
SemanticException rootException = (SemanticException) ExceptionUtil.rootCause( expected);
79-
assertEquals("Could not resolve entity 'Employee'", rootException.getMessage());
79+
assertEquals("Could not resolve root entity 'Employee'", rootException.getMessage());
8080
}
8181
} );
8282
}

0 commit comments

Comments
 (0)