From 38fc97feb300fda87a137310d4f667b016333c6f Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 10 Jan 2022 01:19:19 +0100 Subject: [PATCH] sort out some confusion regarding elements() vs value() + indices() vs index() Strictly, elements() and indices() don't make sense as select items, but we have tests for this, and users who reported bugs and sent in patches, etc, etc, so I'm going to go ahead and keep accepting them in the select clause as a blessed misuse. I'm not however going to allow them to be dereferenced because no, that's why. --- .../org/hibernate/grammars/hql/HqlParser.g4 | 28 ++++--- .../hql/internal/SemanticQueryBuilder.java | 80 ++++++++++--------- 2 files changed, 60 insertions(+), 48 deletions(-) diff --git a/hibernate-core/src/main/antlr/org/hibernate/grammars/hql/HqlParser.g4 b/hibernate-core/src/main/antlr/org/hibernate/grammars/hql/HqlParser.g4 index 5f21d5c862..a270684d8a 100644 --- a/hibernate-core/src/main/antlr/org/hibernate/grammars/hql/HqlParser.g4 +++ b/hibernate-core/src/main/antlr/org/hibernate/grammars/hql/HqlParser.g4 @@ -385,8 +385,7 @@ pathContinuation */ syntacticDomainPath : treatedNavigablePath - | collectionElementNavigablePath - | collectionIndexNavigablePath + | collectionValueNavigablePath | mapKeyNavigablePath | simplePath indexedPathAccessFragment ; @@ -419,17 +418,10 @@ treatedNavigablePath ; /** - * A 'values()' or 'elements()' function that "breaks" a path expression + * A 'value()' function that "breaks" a path expression */ -collectionElementNavigablePath - : (VALUE | ELEMENTS) LEFT_PAREN path RIGHT_PAREN pathContinuation? - ; - -/** - * An 'indices()' function that "breaks" a path expression - */ -collectionIndexNavigablePath - : INDICES LEFT_PAREN path RIGHT_PAREN +collectionValueNavigablePath + : VALUE LEFT_PAREN path RIGHT_PAREN pathContinuation? ; /** @@ -958,6 +950,7 @@ function | jpaCollectionFunction | hqlCollectionFunction | jpaNonstandardFunction + | collectionFunctionMisuse | genericFunction ; @@ -1018,6 +1011,17 @@ hqlCollectionFunction | MINELEMENT LEFT_PAREN path RIGHT_PAREN # MinElementFunction ; +/** + * To accommodate the misuse of elements() and indices() in the select clause + * + * (At some stage in the history of HQL, someone mixed them up with value() and index(), + * and so we have tests that insist they're interchangeable. Ugh.) + */ +collectionFunctionMisuse + : ELEMENTS LEFT_PAREN path RIGHT_PAREN + | INDICES LEFT_PAREN path RIGHT_PAREN + ; + /** * The special `every()`, `all()`, `any()` and `some()` functions defined by HQL * 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 907bccef25..235850ece9 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 @@ -198,11 +198,7 @@ import static java.time.format.DateTimeFormatter.ISO_LOCAL_TIME; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; -import static org.hibernate.grammars.hql.HqlParser.EXCEPT; -import static org.hibernate.grammars.hql.HqlParser.IDENTIFIER; -import static org.hibernate.grammars.hql.HqlParser.INTERSECT; -import static org.hibernate.grammars.hql.HqlParser.PLUS; -import static org.hibernate.grammars.hql.HqlParser.UNION; +import static org.hibernate.grammars.hql.HqlParser.*; import static org.hibernate.query.TemporalUnit.DATE; import static org.hibernate.query.TemporalUnit.DAY_OF_MONTH; import static org.hibernate.query.TemporalUnit.DAY_OF_WEEK; @@ -3932,6 +3928,46 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem return path.getReferencedPathSource() instanceof PluralPersistentAttribute; } + @Override + public SqmPath visitCollectionFunctionMisuse(HqlParser.CollectionFunctionMisuseContext ctx) { + + // Note: this is a total misuse of the elements() and indices() functions, + // which are supposed to be a shortcut way to write a subquery! + // used this way, they're just a worse way to write value()/index() + + if ( getCreationOptions().useStrictJpaCompliance() ) { + throw new StrictJpaComplianceViolation( StrictJpaComplianceViolation.Type.HQL_COLLECTION_FUNCTION ); + } + + final SqmPath pluralAttributePath = consumeDomainPath( (HqlParser.PathContext) ctx.getChild( 2 ) ); + final SqmPathSource referencedPathSource = pluralAttributePath.getReferencedPathSource(); + final TerminalNode firstNode = (TerminalNode) ctx.getChild( 0 ); + + if ( !(referencedPathSource instanceof PluralPersistentAttribute ) ) { + throw new PathException( + String.format( + "Argument of '%s' is not a plural path '%s'", + firstNode.getSymbol().getText(), + pluralAttributePath.getNavigablePath() + ) + ); + } + + CollectionPart.Nature nature; + switch ( firstNode.getSymbol().getType() ) { + case ELEMENTS: + nature = CollectionPart.Nature.ELEMENT; + break; + case INDICES: + nature = CollectionPart.Nature.INDEX; + break; + default: + throw new ParsingException("Impossible symbol"); + } + + return pluralAttributePath.resolvePathPart( nature.getName(), true, this ); + } + @Override public SqmMaxElementPath visitMaxElementFunction(HqlParser.MaxElementFunctionContext ctx) { if ( creationOptions.useStrictJpaCompliance() ) { @@ -4069,11 +4105,8 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem if ( firstChild instanceof HqlParser.TreatedNavigablePathContext ) { return visitTreatedNavigablePath( (HqlParser.TreatedNavigablePathContext) firstChild ); } - else if ( firstChild instanceof HqlParser.CollectionElementNavigablePathContext ) { - return visitCollectionElementNavigablePath( (HqlParser.CollectionElementNavigablePathContext) firstChild ); - } - else if ( firstChild instanceof HqlParser.CollectionIndexNavigablePathContext ) { - return visitCollectionIndexNavigablePath( (HqlParser.CollectionIndexNavigablePathContext) firstChild ); + else if ( firstChild instanceof HqlParser.CollectionValueNavigablePathContext ) { + return visitCollectionValueNavigablePath( (HqlParser.CollectionValueNavigablePathContext) firstChild ); } else if ( firstChild instanceof HqlParser.MapKeyNavigablePathContext ) { return visitMapKeyNavigablePath( (HqlParser.MapKeyNavigablePathContext) firstChild ); @@ -4190,7 +4223,7 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem } @Override - public SqmPath visitCollectionElementNavigablePath(HqlParser.CollectionElementNavigablePathContext ctx) { + public SqmPath visitCollectionValueNavigablePath(HqlParser.CollectionValueNavigablePathContext ctx) { final SqmPath pluralAttributePath = consumeDomainPath( (HqlParser.PathContext) ctx.getChild( 2 ) ); final SqmPathSource referencedPathSource = pluralAttributePath.getReferencedPathSource(); final TerminalNode firstNode = (TerminalNode) ctx.getChild( 0 ); @@ -4211,9 +4244,6 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem if ( attribute.getCollectionClassification() != CollectionClassification.MAP ) { throw new StrictJpaComplianceViolation( StrictJpaComplianceViolation.Type.VALUE_FUNCTION_ON_NON_MAP ); } - if ( firstNode.getSymbol().getType() == HqlParser.ELEMENTS ) { - throw new StrictJpaComplianceViolation( StrictJpaComplianceViolation.Type.HQL_COLLECTION_FUNCTION ); - } } SqmPath result = pluralAttributePath.resolvePathPart( CollectionPart.Nature.ELEMENT.getName(), true, this ); @@ -4226,28 +4256,6 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem } - @Override - public SqmPath visitCollectionIndexNavigablePath(HqlParser.CollectionIndexNavigablePathContext ctx) { - if ( getCreationOptions().useStrictJpaCompliance() ) { - throw new StrictJpaComplianceViolation( StrictJpaComplianceViolation.Type.HQL_COLLECTION_FUNCTION ); - } - final SqmPath pluralAttributePath = consumeDomainPath( (HqlParser.PathContext) ctx.getChild( 2 ) ); - final SqmPathSource referencedPathSource = pluralAttributePath.getReferencedPathSource(); - - if ( !(referencedPathSource instanceof PluralPersistentAttribute ) ) { - final TerminalNode firstNode = (TerminalNode) ctx.getChild( 0 ); - throw new PathException( - String.format( - "Argument of '%s' is not a plural path '%s'", - firstNode.getSymbol().getText(), - pluralAttributePath.getNavigablePath() - ) - ); - } - - return pluralAttributePath.resolvePathPart( CollectionPart.Nature.INDEX.getName(), true, this ); - } - @Override @SuppressWarnings("rawtypes") public SqmPath visitMapKeyNavigablePath(HqlParser.MapKeyNavigablePathContext ctx) {