From bc58df9a78c7ec7be00324219c7207e16cd1bf2d Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Wed, 29 Apr 2015 16:02:51 -0700 Subject: [PATCH] HHH-9758 : Broken SQL generated for dynamic batch fetching entities with a composite ID --- .../hibernate/internal/util/StringHelper.java | 12 +- .../java/org/hibernate/test/batchfetch/A.java | 64 ++++++++++ .../java/org/hibernate/test/batchfetch/B.java | 68 +++++++++++ .../org/hibernate/test/batchfetch/BId.java | 51 ++++++++ .../batchfetch/DynamicBatchFetchTest.java | 115 ++++++++++++++++++ 5 files changed, 304 insertions(+), 6 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/batchfetch/A.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/batchfetch/B.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/batchfetch/BId.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/batchfetch/DynamicBatchFetchTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java index 326e37bc7d..0016bdc82f 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java @@ -71,13 +71,13 @@ public final class StringHelper { return buf.toString(); } - public static String joinWithQualifier(String[] values, String qualifier, String deliminator) { + public static String joinWithQualifierAndSuffix(String[] values, String qualifier, String suffix, String deliminator) { int length = values.length; if ( length == 0 ) return ""; - StringBuilder buf = new StringBuilder( length * values[0].length() ) - .append( qualify( qualifier, values[0] ) ); + StringBuilder buf = new StringBuilder( length * ( values[0].length() + suffix.length() ) ) + .append( qualify( qualifier, values[0] ) ).append( suffix ); for ( int i = 1; i < length; i++ ) { - buf.append( deliminator ).append( qualify( qualifier, values[i] ) ); + buf.append( deliminator ).append( qualify( qualifier, values[i] ) ).append( suffix ); } return buf.toString(); } @@ -755,11 +755,11 @@ public final class StringHelper { else { // composite if ( dialect.supportsRowValueConstructorSyntaxInInList() ) { - final String tuple = "(" + StringHelper.repeat( "?", keyColumnNames.length, "," ); + final String tuple = "(" + StringHelper.repeat( "?", keyColumnNames.length, "," ) + ")"; return StringHelper.replace( sql, BATCH_ID_PLACEHOLDER, repeat( tuple, ids.length, "," ) ); } else { - final String keyCheck = joinWithQualifier( keyColumnNames, alias, " and " ); + final String keyCheck = "(" + joinWithQualifierAndSuffix( keyColumnNames, alias, " = ?", " and " ) + ")"; return replace( sql, BATCH_ID_PLACEHOLDER, repeat( keyCheck, ids.length, " or " ) ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/batchfetch/A.java b/hibernate-core/src/test/java/org/hibernate/test/batchfetch/A.java new file mode 100644 index 0000000000..f57a87549f --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/batchfetch/A.java @@ -0,0 +1,64 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2006-2011, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.batchfetch; + +import javax.persistence.*; + +import org.hibernate.annotations.BatchSize; + +@Entity +public class A { + @Id + @GeneratedValue + private Integer id; + + private String otherProperty; + + @OneToOne(fetch = FetchType.LAZY) + private B b; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getOtherProperty() { + return otherProperty; + } + + public void setOtherProperty(String otherProperty) { + this.otherProperty = otherProperty; + } + + public B getB() { + return b; + } + + public void setB(B b) { + this.b = b; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/batchfetch/B.java b/hibernate-core/src/test/java/org/hibernate/test/batchfetch/B.java new file mode 100644 index 0000000000..af17edd126 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/batchfetch/B.java @@ -0,0 +1,68 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2006-2011, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.batchfetch; + +import org.hibernate.annotations.BatchSize; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.IdClass; + +@Entity +@IdClass(BId.class) +@BatchSize(size = 1000) +public class B { + + @Id + private Integer idPart1; + + @Id + private Integer idPart2; + + private String otherProperty; + + public Integer getIdPart1() { + return idPart1; + } + + public void setIdPart1(Integer idPart1) { + this.idPart1 = idPart1; + } + + public Integer getIdPart2() { + return idPart2; + } + + public void setIdPart2(Integer idPart2) { + this.idPart2 = idPart2; + } + + public String getOtherProperty() { + return otherProperty; + } + + public void setOtherProperty(String otherProperty) { + this.otherProperty = otherProperty; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/batchfetch/BId.java b/hibernate-core/src/test/java/org/hibernate/test/batchfetch/BId.java new file mode 100644 index 0000000000..34c9c09ebf --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/batchfetch/BId.java @@ -0,0 +1,51 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2006-2011, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.batchfetch; + +import java.io.Serializable; + +public class BId + implements Serializable { + private static final long serialVersionUID = 1L; + + private Integer idPart1; + private Integer idPart2; + + public Integer getIdPart1() { + return idPart1; + } + + public void setIdPart1(Integer idPart1) { + this.idPart1 = idPart1; + } + + public Integer getIdPart2() { + return idPart2; + } + + public void setIdPart2(Integer idPart2) { + this.idPart2 = idPart2; + } + +} \ No newline at end of file diff --git a/hibernate-core/src/test/java/org/hibernate/test/batchfetch/DynamicBatchFetchTest.java b/hibernate-core/src/test/java/org/hibernate/test/batchfetch/DynamicBatchFetchTest.java new file mode 100644 index 0000000000..9c762b2650 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/batchfetch/DynamicBatchFetchTest.java @@ -0,0 +1,115 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2006-2011, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.batchfetch; + +import java.util.List; + +import org.junit.Test; + +import org.hibernate.Hibernate; +import org.hibernate.Session; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.cfg.Configuration; +import org.hibernate.engine.spi.EntityKey; +import org.hibernate.engine.spi.SessionImplementor; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class DynamicBatchFetchTest extends BaseCoreFunctionalTestCase { + private static int currentId = 1; + + @Override + protected void configure(Configuration configuration) { + super.configure( configuration ); + configuration.setProperty( AvailableSettings.BATCH_FETCH_STYLE, "DYNAMIC" ); + super.configure( configuration ); + configuration.setProperty( AvailableSettings.USE_SECOND_LEVEL_CACHE, "false" ); + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { A.class, B.class }; + } + + @Test + public void testDynamicBatchFetch() { + Integer aId1 = createAAndB(); + Integer aId2 = createAAndB(); + Session s = openSession(); + s.getTransaction().begin(); + List resultList = s.createQuery("from A where id in (" + aId1 + "," + aId2 + ") order by id" ).list(); + A a1 = (A) resultList.get(0); + A a2 = (A) resultList.get( 1 ); + assertEquals( aId1, a1.getId() ); + assertEquals( aId2, a2.getId() ); + assertFalse( Hibernate.isInitialized( a1.getB() ) ); + assertFalse( Hibernate.isInitialized( a2.getB() ) ); + assertEquals( "foo", a1.getB().getOtherProperty() ); + assertTrue( Hibernate.isInitialized( a1.getB() ) ); + // a2.getB() is still uninitialized + assertFalse( Hibernate.isInitialized( a2.getB() ) ); + // the B entity has been loaded, but is has not been made the target of a2.getB() yet. + assertTrue( ( (SessionImplementor) session ).getPersistenceContext().containsEntity( + new EntityKey( + ( (SessionImplementor) session ).getContextEntityIdentifier( a2.getB() ), + ( (SessionImplementor) session ).getFactory().getEntityPersister( B.class.getName() ) + ) + ) + ); + // a2.getB() is still uninitialized; getting the ID for a2.getB() did not initialize it. + assertFalse( Hibernate.isInitialized( a2.getB() ) ); + assertEquals( "foo", a2.getB().getOtherProperty() ); + // now it's initialized. + assertTrue( Hibernate.isInitialized( a2.getB() ) ); + s.getTransaction().commit(); + s.close(); + + } + + private int createAAndB() { + Session s = openSession(); + s.getTransaction().begin(); + B b = new B(); + b.setIdPart1( currentId ); + b.setIdPart2( currentId); + b.setOtherProperty("foo"); + s.save( b ); + + A a = new A(); + a.setId( currentId ); + a.setB( b ); + + s.save( a ); + + s.getTransaction().commit(); + s.close(); + + currentId++; + + return currentId - 1; + } +}