From d3e15a7cc17130424ea258e8dc6b391b4ca0130b Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 17 Jun 2023 14:08:14 +0200 Subject: [PATCH] don't throw ParsingException (it represents a bug in the parser) (#6819) - we should throw SyntaxException for expected conditions - also, avoid the use of weirdo non-standard hyphenation in error messages --- .../org/hibernate/query/SyntaxException.java | 3 +++ .../hql/internal/SemanticQueryBuilder.java | 21 ++++++++++--------- .../sql/ast/spi/SqlExpressionResolver.java | 6 +++--- .../test/query/hql/WindowFunctionTest.java | 6 +++--- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/SyntaxException.java b/hibernate-core/src/main/java/org/hibernate/query/SyntaxException.java index 7b1a559a27..edae4f8f6a 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/SyntaxException.java +++ b/hibernate-core/src/main/java/org/hibernate/query/SyntaxException.java @@ -21,4 +21,7 @@ public class SyntaxException extends QueryException { public SyntaxException(String message, String queryString) { super( message, queryString ); } + public SyntaxException(String message) { + super( message ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java b/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java index 4c3b0f148e..cddb6391ef 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java @@ -58,6 +58,7 @@ import org.hibernate.query.ParameterLabelException; import org.hibernate.query.PathException; import org.hibernate.query.ReturnableType; import org.hibernate.query.SemanticException; +import org.hibernate.query.SyntaxException; import org.hibernate.query.sqm.TerminalPathException; import org.hibernate.query.criteria.JpaCteCriteria; import org.hibernate.query.criteria.JpaCteCriteriaAttribute; @@ -1443,11 +1444,11 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem if ( child instanceof TerminalNode ) { if ( definedCollate ) { // This is syntactically disallowed - throw new ParsingException( "COLLATE is not allowed for position based order-by or group-by items" ); + throw new SyntaxException( "'collate' is not allowed for position based 'order by' or 'group by' items" ); } else if ( !allowPositionalOrAliases ) { // This is syntactically disallowed - throw new ParsingException( "Position based order-by is not allowed in OVER or WITHIN GROUP clauses" ); + throw new SyntaxException( "Position based 'order by' is not allowed in 'over' or 'within group' clauses" ); } final int position = Integer.parseInt( child.getText() ); @@ -1462,7 +1463,7 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem nodeByPosition = processingState.getPathRegistry().findAliasedNodeByPosition( position ); } if ( nodeByPosition == null ) { - throw new ParsingException( "Numeric literal '" + position + "' used in group-by does not match a registered select-item" ); + throw new SemanticException( "Numeric literal '" + position + "' used in 'group by' does not match a registered select item" ); } return new SqmAliasedNodeRef( position, integerDomainType, creationContext.getNodeBuilder() ); @@ -1531,7 +1532,7 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem if ( correspondingPosition != null ) { if ( definedCollate ) { // This is syntactically disallowed - throw new ParsingException( "COLLATE is not allowed for alias based order-by or group-by items" ); + throw new SyntaxException( "'collate' is not allowed for alias-based 'order by' or 'group by' items" ); } return new SqmAliasedNodeRef( correspondingPosition, @@ -1547,7 +1548,7 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem if ( sqmFrom != null ) { if ( definedCollate ) { // This is syntactically disallowed - throw new ParsingException( "COLLATE is not allowed for alias based order-by or group-by items" ); + throw new SyntaxException( "'collate' is not allowed for alias-based 'order by' or 'group by' items" ); } // this will group-by all the sub-parts in the from-element's model part return sqmFrom; @@ -1603,11 +1604,11 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem HqlParser.SortSpecificationContext ctx, boolean allowPositionalOrAliases) { final SqmExpression sortExpression = visitSortExpression( - (HqlParser.SortExpressionContext) ctx.getChild( 0 ), + ctx.sortExpression(), allowPositionalOrAliases ); if ( sortExpression == null ) { - throw new ParsingException( "Could not resolve sort-expression : " + ctx.getChild( 0 ).getText() ); + throw new SemanticException( "Could not resolve sort expression: '" + ctx.sortExpression().getText() + "'" ); } if ( sortExpression instanceof SqmLiteral || sortExpression instanceof SqmParameter ) { HqlLogging.QUERY_LOGGER.debugf( "Questionable sorting by constant value : %s", sortExpression ); @@ -2923,7 +2924,7 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem @Override public SqmExpression visitConcatenationExpression(HqlParser.ConcatenationExpressionContext ctx) { if ( ctx.getChildCount() != 3 ) { - throw new ParsingException( "Expecting two operands to the concat operator" ); + throw new SyntaxException( "Expecting two operands to the '||' operator" ); } return getFunctionDescriptor( "concat" ).generateSqmExpression( asList( @@ -2976,7 +2977,7 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem @Override public Object visitAdditionExpression(HqlParser.AdditionExpressionContext ctx) { if ( ctx.getChildCount() != 3 ) { - throw new ParsingException( "Expecting two operands to the additive operator" ); + throw new SyntaxException( "Expecting two operands to the additive operator" ); } final SqmExpression left = (SqmExpression) ctx.expression(0).accept(this); @@ -2997,7 +2998,7 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem @Override public Object visitMultiplicationExpression(HqlParser.MultiplicationExpressionContext ctx) { if ( ctx.getChildCount() != 3 ) { - throw new ParsingException( "Expecting two operands to the multiplicative operator" ); + throw new SyntaxException( "Expecting two operands to the multiplicative operator" ); } final SqmExpression left = (SqmExpression) ctx.expression(0).accept( this ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlExpressionResolver.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlExpressionResolver.java index cc823f7df9..8b9f1d90bf 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlExpressionResolver.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlExpressionResolver.java @@ -24,15 +24,15 @@ import static java.util.Locale.ROOT; /** * Resolution of a SqlSelection reference for a given SqlSelectable. Some * SqlSelectable are required to be qualified (e.g. a Column) - this is indicated - * by the QualifiableSqlSelectable sub-type. The NonQualifiableSqlSelectable - * sub-type indicates a SqlSelectable that does not require qualification (e.g. a + * by the QualifiableSqlSelectable subtype. The NonQualifiableSqlSelectable + * subtype indicates a SqlSelectable that does not require qualification (e.g. a * literal). *

* The point of this contract is to allow "unique-ing" of SqlSelectable references * in a query to a single SqlSelection reference - effectively a caching of * SqlSelection instances keyed by the SqlSelectable (+ qualifier when applicable) * that it refers to. - * + *

* Note also that the returns can be a specialized Expression represented by * {@link org.hibernate.sql.ast.tree.expression.SqlSelectionExpression} * diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/WindowFunctionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/WindowFunctionTest.java index 4a854910f5..365a16ba90 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/WindowFunctionTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/WindowFunctionTest.java @@ -10,7 +10,7 @@ import java.util.Date; import java.util.List; import org.hibernate.internal.util.ExceptionHelper; -import org.hibernate.query.sqm.ParsingException; +import org.hibernate.query.SyntaxException; import org.hibernate.testing.orm.domain.StandardDomainModel; import org.hibernate.testing.orm.domain.gambit.EntityOfBasics; @@ -202,9 +202,9 @@ public class WindowFunctionTest { } catch (Exception e) { final Throwable rootCause = ExceptionHelper.getRootCause( e ); - assertInstanceOf( ParsingException.class, rootCause ); + assertInstanceOf( SyntaxException.class, rootCause ); assertEquals( - "Position based order-by is not allowed in OVER or WITHIN GROUP clauses", + "Position based 'order by' is not allowed in 'over' or 'within group' clauses", rootCause.getMessage() ); }