From 3df127e2e07ba5bb2559408715f5d610ef53060a Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Wed, 26 Aug 2015 14:08:15 +0100 Subject: [PATCH 1/2] SQM-1 - Created a unified alias registry --- .../parser/internal/hql/antlr/HqlParser.g4 | 16 ++++++---- .../query/parser/AliasCollisionException.java | 20 +++++++++++++ .../query/parser/internal/AliasRegistry.java | 30 +++++++++++++++++++ .../query/parser/internal/ParsingContext.java | 5 ++++ .../hql/AbstractHqlParseTreeVisitor.java | 15 ++++++++-- .../hql/phase1/FromClauseProcessor.java | 14 +++++---- .../hql/SimpleSemanticQueryBuilderTest.java | 28 +++++++++++++++++ 7 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/AliasCollisionException.java create mode 100644 hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/AliasRegistry.java diff --git a/hibernate-query-interpreter/src/main/antlr/org/hibernate/query/parser/internal/hql/antlr/HqlParser.g4 b/hibernate-query-interpreter/src/main/antlr/org/hibernate/query/parser/internal/hql/antlr/HqlParser.g4 index 315ce05..81166cb 100644 --- a/hibernate-query-interpreter/src/main/antlr/org/hibernate/query/parser/internal/hql/antlr/HqlParser.g4 +++ b/hibernate-query-interpreter/src/main/antlr/org/hibernate/query/parser/internal/hql/antlr/HqlParser.g4 @@ -139,7 +139,11 @@ selection // I have noticed that without this predicate, Antlr will sometimes // interpret `select a.b from Something ...` as `from` being the // select-expression alias - : selectExpression (asKeyword? {!doesUpcomingTokenMatchAny("from")}? IDENTIFIER)? + : selectExpression (asKeyword? {!doesUpcomingTokenMatchAny("from")}? alias)? + ; + +alias + : IDENTIFIER ; selectExpression @@ -172,7 +176,7 @@ dynamicInstantiationArgs ; dynamicInstantiationArg - : dynamicInstantiationArgExpression (asKeyword? IDENTIFIER)? + : dynamicInstantiationArgExpression (asKeyword? alias)? ; dynamicInstantiationArgExpression @@ -181,7 +185,7 @@ dynamicInstantiationArgExpression ; jpaSelectObjectSyntax - : objectKeyword LEFT_PAREN IDENTIFIER RIGHT_PAREN + : objectKeyword LEFT_PAREN alias RIGHT_PAREN ; @@ -201,7 +205,7 @@ fromElementSpaceRoot ; mainEntityPersisterReference - : dotIdentifierSequence (asKeyword? {!doesUpcomingTokenMatchAny("where","join")}? IDENTIFIER)? + : dotIdentifierSequence (asKeyword? {!doesUpcomingTokenMatchAny("where","join")}? alias)? ; crossJoin @@ -209,7 +213,7 @@ crossJoin ; jpaCollectionJoin - : inKeyword LEFT_PAREN path RIGHT_PAREN (asKeyword? IDENTIFIER)? + : inKeyword LEFT_PAREN path RIGHT_PAREN (asKeyword? alias)? ; qualifiedJoin @@ -217,7 +221,7 @@ qualifiedJoin ; qualifiedJoinRhs - : path (asKeyword? IDENTIFIER)? + : path (asKeyword? alias)? ; qualifiedJoinPredicate diff --git a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/AliasCollisionException.java b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/AliasCollisionException.java new file mode 100644 index 0000000..c5b83ba --- /dev/null +++ b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/AliasCollisionException.java @@ -0,0 +1,20 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.query.parser; + +/** + * @author Andrea Boriero + */ +public class AliasCollisionException extends SemanticException { + public AliasCollisionException(String message) { + super( message ); + } + + public AliasCollisionException(String message, Throwable cause) { + super( message, cause ); + } +} diff --git a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/AliasRegistry.java b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/AliasRegistry.java new file mode 100644 index 0000000..ebb17a7 --- /dev/null +++ b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/AliasRegistry.java @@ -0,0 +1,30 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.query.parser.internal; + +import java.util.HashSet; +import java.util.Set; + +import org.hibernate.query.parser.AliasCollisionException; + +/** + * @author Andrea Boriero + */ +public class AliasRegistry { + Set aliases; + + public AliasRegistry() { + this.aliases = new HashSet(); + } + + public void registerAlias(String alias) { + if ( aliases.contains( alias ) ) { + throw new AliasCollisionException( "Alias collision, alias " + alias + " is used in a different clause" ); + } + aliases.add( alias ); + } +} diff --git a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/ParsingContext.java b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/ParsingContext.java index 0b48413..3778f18 100644 --- a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/ParsingContext.java +++ b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/ParsingContext.java @@ -16,6 +16,7 @@ public class ParsingContext { private final ConsumerContext consumerContext; private final ImplicitAliasGenerator aliasGenerator = new ImplicitAliasGenerator(); + private final AliasRegistry aliasRegistry = new AliasRegistry(); public ParsingContext(ConsumerContext consumerContext) { this.consumerContext = consumerContext; @@ -28,4 +29,8 @@ public ConsumerContext getConsumerContext() { public ImplicitAliasGenerator getImplicitAliasGenerator() { return aliasGenerator; } + + public AliasRegistry getAliasRegistry(){ + return aliasRegistry; + } } diff --git a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/AbstractHqlParseTreeVisitor.java b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/AbstractHqlParseTreeVisitor.java index 3314a23..bbc3c82 100644 --- a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/AbstractHqlParseTreeVisitor.java +++ b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/AbstractHqlParseTreeVisitor.java @@ -243,10 +243,19 @@ public SelectClause visitSelectClause(HqlParser.SelectClauseContext ctx) { public Selection visitSelection(HqlParser.SelectionContext ctx) { return new Selection( visitSelectExpression( ctx.selectExpression() ), - ctx.IDENTIFIER() == null ? null : ctx.IDENTIFIER().getText() + visitAlias( ctx.alias() ) ); } + @Override + public String visitAlias(HqlParser.AliasContext ctx) { + if(ctx == null){ + return null; + } + parsingContext.getAliasRegistry().registerAlias( ctx.IDENTIFIER().getText() ); + return ctx.IDENTIFIER().getText(); + } + @Override public Expression visitSelectExpression(HqlParser.SelectExpressionContext ctx) { if ( ctx.dynamicInstantiation() != null ) { @@ -286,7 +295,7 @@ public DynamicInstantiation visitDynamicInstantiation(HqlParser.DynamicInstantia public DynamicInstantiationArgument visitDynamicInstantiationArg(HqlParser.DynamicInstantiationArgContext ctx) { return new DynamicInstantiationArgument( visitDynamicInstantiationArgExpression( ctx.dynamicInstantiationArgExpression() ), - ctx.IDENTIFIER() == null ? null : ctx.IDENTIFIER().getText() + visitAlias( ctx.alias() ) ); } @@ -304,7 +313,7 @@ else if ( ctx.expression() != null ) { @Override public FromElementReferenceExpression visitJpaSelectObjectSyntax(HqlParser.JpaSelectObjectSyntaxContext ctx) { - final String alias = ctx.IDENTIFIER().getText(); + final String alias = visitAlias( ctx.alias() ); final FromElement fromElement = fromClauseIndex.findFromElementByAlias( alias ); if ( fromElement == null ) { throw new SemanticException( "Unable to resolve alias [" + alias + "] in selection [" + ctx.getText() + "]" ); diff --git a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/phase1/FromClauseProcessor.java b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/phase1/FromClauseProcessor.java index fc4bf79..24be2e9 100644 --- a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/phase1/FromClauseProcessor.java +++ b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/phase1/FromClauseProcessor.java @@ -172,7 +172,7 @@ public void enterFromElementSpaceRoot(HqlParser.FromElementSpaceRootContext ctx) final RootEntityFromElement rootEntityFromElement = fromElementBuilder.makeRootEntityFromElement( currentFromElementSpace, resolveEntityReference( ctx.mainEntityPersisterReference().dotIdentifierSequence() ), - interpretAlias( ctx.mainEntityPersisterReference().IDENTIFIER() ) + interpretAlias( ctx.mainEntityPersisterReference().alias() ) ); fromElementMap.put( ctx.getText(), rootEntityFromElement ); } @@ -188,11 +188,13 @@ private EntityTypeDescriptor resolveEntityReference(HqlParser.DotIdentifierSeque return entityTypeDescriptor; } - private String interpretAlias(TerminalNode aliasNode) { - if ( aliasNode == null ) { + private String interpretAlias(HqlParser.AliasContext ctx) { + if ( ctx == null ) { return parsingContext.getImplicitAliasGenerator().buildUniqueImplicitAlias(); } + final TerminalNode aliasNode = ctx.IDENTIFIER(); assert aliasNode.getSymbol().getType() == HqlParser.IDENTIFIER; + parsingContext.getAliasRegistry().registerAlias( aliasNode.getText() ); return aliasNode.getText(); } @@ -212,7 +214,7 @@ public void enterCrossJoin(HqlParser.CrossJoinContext ctx) { final CrossJoinedFromElement join = fromElementBuilder.makeCrossJoinedFromElement( currentFromElementSpace, entityTypeDescriptor, - interpretAlias( ctx.mainEntityPersisterReference().IDENTIFIER() ) + interpretAlias( ctx.mainEntityPersisterReference().alias() ) ); fromElementMap.put( ctx.getText(), join ); } @@ -226,7 +228,7 @@ public void enterJpaCollectionJoin(HqlParser.JpaCollectionJoinContext ctx) { currentFromElementSpace, currentFromClauseStackNode, JoinType.INNER, - interpretAlias( ctx.IDENTIFIER() ), + interpretAlias( ctx.alias() ), false ); @@ -259,7 +261,7 @@ public void enterQualifiedJoin(HqlParser.QualifiedJoinContext ctx) { currentFromElementSpace, currentFromClauseStackNode, joinType, - interpretAlias( ctx.qualifiedJoinRhs().IDENTIFIER() ), + interpretAlias( ctx.qualifiedJoinRhs().alias() ), ctx.fetchKeyword() != null ); diff --git a/hibernate-query-interpreter/src/test/java/org/hibernate/query/parser/hql/SimpleSemanticQueryBuilderTest.java b/hibernate-query-interpreter/src/test/java/org/hibernate/query/parser/hql/SimpleSemanticQueryBuilderTest.java index 51a047a..bfc7672 100644 --- a/hibernate-query-interpreter/src/test/java/org/hibernate/query/parser/hql/SimpleSemanticQueryBuilderTest.java +++ b/hibernate-query-interpreter/src/test/java/org/hibernate/query/parser/hql/SimpleSemanticQueryBuilderTest.java @@ -8,6 +8,7 @@ import java.util.Collection; +import org.hibernate.query.parser.AliasCollisionException; import org.hibernate.query.parser.SemanticException; import org.hibernate.query.parser.SemanticQueryInterpreter; import org.hibernate.query.parser.internal.hql.HqlParseTreeBuilder; @@ -325,6 +326,33 @@ public void testSimpleDynamicInstantiation() throws Exception { assertNotNull( querySpec ); } + @Test(expected = AliasCollisionException.class) + public void testAliasCollision(){ + final String query = "select a.address as a from Anything as a"; + final SelectStatement selectStatement = (SelectStatement) SemanticQueryInterpreter.interpret( + query, + new ConsumerContextTestingImpl() + ); + } + + @Test(expected = AliasCollisionException.class) + public void testAliasCollisionInSubquery() throws Exception { + final String query = "Select a from Something a where a.b in ( select a from SomethingElse a where a.basic = 5) and a.c in ( select c from SomethingElse2 c where c.basic1 = 6)"; + final SelectStatement selectStatement = (SelectStatement) SemanticQueryInterpreter.interpret( + query, + new ConsumerContextTestingImpl() + ); + } + + @Test(expected = AliasCollisionException.class) + public void testAliasCollisionInJoin() throws Exception { + final String query = "select a from Something a left outer join a.entity a on a.basic1 > 5 and a.basic2 < 20"; + final SelectStatement selectStatement = (SelectStatement) SemanticQueryInterpreter.interpret( + query, + new ConsumerContextTestingImpl() + ); + } + private static class DTO { } } From 268d7ba460ce8a48e9303e770d9cc415b0ab9767 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Thu, 27 Aug 2015 10:08:31 +0100 Subject: [PATCH 2/2] SQM-1 - Fix based on Steve code review --- .../hibernate/query/parser/internal/hql/antlr/HqlParser.g4 | 4 ++-- .../org/hibernate/query/parser/AliasCollisionException.java | 4 ++-- .../org/hibernate/query/parser/internal/AliasRegistry.java | 4 ++-- .../parser/internal/hql/AbstractHqlParseTreeVisitor.java | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/hibernate-query-interpreter/src/main/antlr/org/hibernate/query/parser/internal/hql/antlr/HqlParser.g4 b/hibernate-query-interpreter/src/main/antlr/org/hibernate/query/parser/internal/hql/antlr/HqlParser.g4 index 81166cb..42b26cb 100644 --- a/hibernate-query-interpreter/src/main/antlr/org/hibernate/query/parser/internal/hql/antlr/HqlParser.g4 +++ b/hibernate-query-interpreter/src/main/antlr/org/hibernate/query/parser/internal/hql/antlr/HqlParser.g4 @@ -176,7 +176,7 @@ dynamicInstantiationArgs ; dynamicInstantiationArg - : dynamicInstantiationArgExpression (asKeyword? alias)? + : dynamicInstantiationArgExpression (asKeyword? IDENTIFIER)? ; dynamicInstantiationArgExpression @@ -185,7 +185,7 @@ dynamicInstantiationArgExpression ; jpaSelectObjectSyntax - : objectKeyword LEFT_PAREN alias RIGHT_PAREN + : objectKeyword LEFT_PAREN IDENTIFIER RIGHT_PAREN ; diff --git a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/AliasCollisionException.java b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/AliasCollisionException.java index c5b83ba..e944855 100644 --- a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/AliasCollisionException.java +++ b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/AliasCollisionException.java @@ -1,8 +1,8 @@ /* * Hibernate, Relational Persistence for Idiomatic Java * - * License: GNU Lesser General Public License (LGPL), version 2.1 or later. - * See the lgpl.txt file in the root directory or . + * License: Apache License, Version 2.0 + * See the LICENSE file in the root directory or visit http://www.apache.org/licenses/LICENSE-2.0 */ package org.hibernate.query.parser; diff --git a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/AliasRegistry.java b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/AliasRegistry.java index ebb17a7..d2a3d51 100644 --- a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/AliasRegistry.java +++ b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/AliasRegistry.java @@ -1,8 +1,8 @@ /* * Hibernate, Relational Persistence for Idiomatic Java * - * License: GNU Lesser General Public License (LGPL), version 2.1 or later. - * See the lgpl.txt file in the root directory or . + * License: Apache License, Version 2.0 + * See the LICENSE file in the root directory or visit http://www.apache.org/licenses/LICENSE-2.0 */ package org.hibernate.query.parser.internal; diff --git a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/AbstractHqlParseTreeVisitor.java b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/AbstractHqlParseTreeVisitor.java index bbc3c82..37cda41 100644 --- a/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/AbstractHqlParseTreeVisitor.java +++ b/hibernate-query-interpreter/src/main/java/org/hibernate/query/parser/internal/hql/AbstractHqlParseTreeVisitor.java @@ -295,7 +295,7 @@ public DynamicInstantiation visitDynamicInstantiation(HqlParser.DynamicInstantia public DynamicInstantiationArgument visitDynamicInstantiationArg(HqlParser.DynamicInstantiationArgContext ctx) { return new DynamicInstantiationArgument( visitDynamicInstantiationArgExpression( ctx.dynamicInstantiationArgExpression() ), - visitAlias( ctx.alias() ) + ctx.IDENTIFIER() == null ? null : ctx.IDENTIFIER().getText() ); } @@ -313,7 +313,7 @@ else if ( ctx.expression() != null ) { @Override public FromElementReferenceExpression visitJpaSelectObjectSyntax(HqlParser.JpaSelectObjectSyntaxContext ctx) { - final String alias = visitAlias( ctx.alias() ); + final String alias = ctx.IDENTIFIER().getText(); final FromElement fromElement = fromClauseIndex.findFromElementByAlias( alias ); if ( fromElement == null ) { throw new SemanticException( "Unable to resolve alias [" + alias + "] in selection [" + ctx.getText() + "]" );