From 62e691c38aa66f4523cba12ece4d59671f022055 Mon Sep 17 00:00:00 2001 From: Jonathan Bregler Date: Mon, 18 Sep 2017 15:02:35 +0200 Subject: [PATCH] HHH-11816 - JoinProcessor considers table names with colons dynamic filter parameters --- .../hql/internal/ast/HqlSqlWalker.java | 6 + .../hql/internal/ast/util/JoinProcessor.java | 26 +- .../EntityWithUnusualTableNameJoinTest.java | 302 ++++++++++++++++++ 3 files changed, 327 insertions(+), 7 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/hql/EntityWithUnusualTableNameJoinTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java index 3b24bdaf5c..463691c9d8 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java @@ -19,8 +19,10 @@ import java.util.Map; import java.util.Set; import org.hibernate.QueryException; +import org.hibernate.dialect.Dialect; import org.hibernate.engine.internal.JoinSequence; import org.hibernate.engine.internal.ParameterBinder; +import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.hql.internal.CollectionProperties; import org.hibernate.hql.internal.antlr.HqlSqlBaseWalker; @@ -1420,6 +1422,10 @@ public class HqlSqlWalker extends HqlSqlBaseWalker implements ErrorReporter, Par return hqlParser.getTreatMap().get( path ); } + public Dialect getDialect() { + return sessionFactoryHelper.getFactory().getServiceRegistry().getService( JdbcServices.class ).getDialect(); + } + public static void panic() { throw new QueryException( "TreeWalker: panic" ); } diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/util/JoinProcessor.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/util/JoinProcessor.java index fae211bb45..175a3510ad 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/util/JoinProcessor.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/util/JoinProcessor.java @@ -12,6 +12,8 @@ import java.util.Iterator; import java.util.List; import java.util.ListIterator; import java.util.StringTokenizer; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.hibernate.AssertionFailure; import org.hibernate.dialect.Dialect; @@ -46,6 +48,9 @@ import org.hibernate.type.Type; public class JoinProcessor implements SqlTokenTypes { private static final CoreMessageLogger LOG = CoreLogging.messageLogger( JoinProcessor.class ); + private static final Pattern DYNAMIC_FILTER_PATTERN = Pattern.compile(":(\\w+\\S*)\\s"); + private static final String LITERAL_DELIMITER = "'"; + private final HqlSqlWalker walker; private final SyntheticAndFactory syntheticAndFactory; @@ -98,7 +103,7 @@ public class JoinProcessor implements SqlTokenTypes { // found it easiest to simply reorder the FromElements here into ascending order // in terms of injecting them into the resulting sql ast in orders relative to those // expected by the old parser; this is definitely another of those "only needed - // for regression purposes". The SyntheticAndFactory, then, simply injects them as it + // for regression purposes". The SyntheticAndFactory, then, simply injects them as it // encounters them. fromElements = new ArrayList(); ListIterator liter = fromClause.getFromElements().listIterator( fromClause.getFromElements().size() ); @@ -118,7 +123,7 @@ public class JoinProcessor implements SqlTokenTypes { && fromElement.getOrigin().getWithClauseFragment().contains( fromElement.getTableAlias() ) ) { fromElement.getOrigin().getJoinSequence().addJoin( (ImpliedFromElement) fromElement ); // This from element will be rendered as part of the origins join sequence - fromElement.setText(""); + fromElement.setText( "" ); } else { fromElements.add( fromElement ); @@ -203,7 +208,7 @@ public class JoinProcessor implements SqlTokenTypes { private String processFromFragment(String frag, JoinSequence join) { String fromFragment = frag.trim(); - // The FROM fragment will probably begin with ', '. Remove this if it is present. + // The FROM fragment will probably begin with ', '. Remove this if it is present. if ( fromFragment.startsWith( ", " ) ) { fromFragment = fromFragment.substring( 2 ); } @@ -215,12 +220,12 @@ public class JoinProcessor implements SqlTokenTypes { final ParameterContainer container, final HqlSqlWalker walker) { if ( walker.getEnabledFilters().isEmpty() - && ( !hasDynamicFilterParam( sqlFragment ) ) + && ( !hasDynamicFilterParam( walker, sqlFragment ) ) && ( !( hasCollectionFilterParam( sqlFragment ) ) ) ) { return; } - Dialect dialect = walker.getSessionFactoryHelper().getFactory().getDialect(); + Dialect dialect = walker.getDialect(); String symbols = ParserHelper.HQL_SEPARATORS + dialect.openQuote() + dialect.closeQuote(); StringTokenizer tokens = new StringTokenizer( sqlFragment, symbols, true ); StringBuilder result = new StringBuilder(); @@ -261,8 +266,15 @@ public class JoinProcessor implements SqlTokenTypes { container.setText( result.toString() ); } - private static boolean hasDynamicFilterParam(String sqlFragment) { - return !sqlFragment.contains( ParserHelper.HQL_VARIABLE_PREFIX ); + private static boolean hasDynamicFilterParam(HqlSqlWalker walker, String sqlFragment) { + String closeQuote = String.valueOf( walker.getDialect().closeQuote() ); + + Matcher matcher = DYNAMIC_FILTER_PATTERN.matcher( sqlFragment ); + if ( matcher.find() && matcher.groupCount() > 0 ) { + String match = matcher.group( 1 ); + return match.endsWith( closeQuote ) || match.endsWith( LITERAL_DELIMITER ); + } + return true; } private static boolean hasCollectionFilterParam(String sqlFragment) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/hql/EntityWithUnusualTableNameJoinTest.java b/hibernate-core/src/test/java/org/hibernate/test/hql/EntityWithUnusualTableNameJoinTest.java new file mode 100644 index 0000000000..2bfaa3ece0 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/hql/EntityWithUnusualTableNameJoinTest.java @@ -0,0 +1,302 @@ +/* + * 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.test.hql; + +import static org.hamcrest.core.Is.is; +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; +import static org.junit.Assert.assertThat; + +import java.util.List; + +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.Table; + +import org.hibernate.annotations.NaturalId; +import org.hibernate.testing.Skip; +import org.hibernate.testing.TestForIssue; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * @author Jonathan Bregler + */ +public class EntityWithUnusualTableNameJoinTest extends EntityJoinTest { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[]{ FinancialRecord.class, User.class, Customer.class, Account.class }; + } + + @Override + @Before + public void prepare() { + createTestData(); + } + + @Override + @After + public void cleanup() { + deleteTestData(); + } + + @Test + @TestForIssue(jiraKey = "HHH-11816") + public void testInnerEntityJoinsWithVariable() { + doInHibernate( this::sessionFactory, session -> { + + // this should get financial records which have a lastUpdateBy user set + List result = session.createQuery( + "select r.id, c.name, u.id, u.username " + + "from FinancialRecord r " + + " inner join r.customer c " + + " inner join User u on r.lastUpdateBy = u.username and u.username=:username" ) + .setParameter( "username", "steve" ).list(); + + assertThat( Integer.valueOf( result.size() ), is( Integer.valueOf( 1 ) ) ); + Object[] steveAndAcme = result.get( 0 ); + assertThat( steveAndAcme[0], is( Integer.valueOf( 1 ) ) ); + assertThat( steveAndAcme[1], is( "Acme" ) ); + assertThat( steveAndAcme[3], is( "steve" ) ); + + } ); + } + + @Test + @TestForIssue(jiraKey = "HHH-11816") + public void testInnerEntityJoinsWithVariableSingleQuoted() { + doInHibernate( this::sessionFactory, session -> { + + // this should get financial records which have a lastUpdateBy user set + List result = session.createQuery( + "select r.id, c.name, a.id, a.accountname, r.lastUpdateBy " + + "from FinancialRecord r " + + " inner join r.customer c " + + " inner join Account a on a.customer = c and a.accountname!='test:account' and a.accountname=:accountname and r.lastUpdateBy != null" ) + .setParameter( "accountname", "DEBIT" ).list(); + + assertThat( Integer.valueOf( result.size() ), is( Integer.valueOf( 1 ) ) ); + Object[] steveAndAcmeAndDebit = result.get( 0 ); + assertThat( steveAndAcmeAndDebit[0], is( Integer.valueOf( 1 ) ) ); + assertThat( steveAndAcmeAndDebit[1], is( "Acme" ) ); + assertThat( steveAndAcmeAndDebit[3], is( "DEBIT" ) ); + assertThat( steveAndAcmeAndDebit[4], is( "steve" ) ); + } ); + } + + @Override + @Skip(message = "The superclass test checks for the table name which is different in this test case and causes the test to fail", condition = Skip.AlwaysSkip.class) + public void testNoImpliedJoinGeneratedForEqualityComparison() { + } + + private void createTestData() { + doInHibernate( this::sessionFactory, session -> { + + final Customer customer = new Customer( Integer.valueOf( 1 ), "Acme" ); + session.save( customer ); + session.save( new User( Integer.valueOf( 1 ), "steve", customer ) ); + session.save( new User( Integer.valueOf( 2 ), "jane" ) ); + session.save( new FinancialRecord( Integer.valueOf( 1 ), customer, "steve" ) ); + session.save( new FinancialRecord( Integer.valueOf( 2 ), customer, null ) ); + session.save( new Account( Integer.valueOf( 1 ), "DEBIT", customer ) ); + session.save( new Account( Integer.valueOf( 2 ), "CREDIT" ) ); + } ); + } + + private void deleteTestData() { + doInHibernate( this::sessionFactory, session -> { + session.createQuery( "delete FinancialRecord" ).executeUpdate(); + session.createQuery( "delete User" ).executeUpdate(); + session.createQuery( "delete Account" ).executeUpdate(); + session.createQuery( "delete Customer" ).executeUpdate(); + + } ); + } + + @Entity(name = "Customer") + @Table(name = "`my::customer`") + public static class Customer { + + private Integer id; + private String name; + + public Customer() { + } + + public Customer(Integer id, String name) { + this.id = id; + this.name = name; + } + + @Id + public Integer getId() { + return this.id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return this.name; + } + + public void setName(String name) { + this.name = name; + } + } + + @Entity(name = "FinancialRecord") + @Table(name = "`financial?record`") + public static class FinancialRecord { + + private Integer id; + private Customer customer; + private String lastUpdateBy; + + public FinancialRecord() { + } + + public FinancialRecord(Integer id, Customer customer, String lastUpdateBy) { + this.id = id; + this.customer = customer; + this.lastUpdateBy = lastUpdateBy; + } + + @Id + public Integer getId() { + return this.id; + } + + public void setId(Integer id) { + this.id = id; + } + + @ManyToOne + @JoinColumn + public Customer getCustomer() { + return this.customer; + } + + public void setCustomer(Customer customer) { + this.customer = customer; + } + + public String getLastUpdateBy() { + return this.lastUpdateBy; + } + + public void setLastUpdateBy(String lastUpdateBy) { + this.lastUpdateBy = lastUpdateBy; + } + } + + @Entity(name = "User") + @Table(name = "`my::user`") + public static class User { + + private Integer id; + private String username; + private Customer customer; + + public User() { + } + + public User(Integer id, String username) { + this.id = id; + this.username = username; + } + + public User(Integer id, String username, Customer customer) { + this.id = id; + this.username = username; + this.customer = customer; + } + + @Id + public Integer getId() { + return this.id; + } + + public void setId(Integer id) { + this.id = id; + } + + @NaturalId + public String getUsername() { + return this.username; + } + + public void setUsername(String username) { + this.username = username; + } + + @ManyToOne(fetch = FetchType.LAZY) + public Customer getCustomer() { + return this.customer; + } + + public void setCustomer(Customer customer) { + this.customer = customer; + } + } + + @Entity(name = "Account") + @Table(name = "`account`") + public static class Account { + + private Integer id; + private String accountname; + private Customer customer; + + public Account() { + } + + public Account(Integer id, String accountname) { + this.id = id; + this.accountname = accountname; + } + + public Account(Integer id, String accountname, Customer customer) { + this.id = id; + this.accountname = accountname; + this.customer = customer; + } + + @Id + public Integer getId() { + return this.id; + } + + public void setId(Integer id) { + this.id = id; + } + + @NaturalId + public String getAccountname() { + return this.accountname; + } + + public void setAccountname(String accountname) { + this.accountname = accountname; + } + + @ManyToOne(fetch = FetchType.LAZY) + public Customer getCustomer() { + return this.customer; + } + + public void setCustomer(Customer customer) { + this.customer = customer; + } + } + +}