HHH-9758 : Broken SQL generated for dynamic batch fetching entities with a composite ID

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() )
+		StringBuilder buf = new StringBuilder( length * ( values[0].length() + suffix.length() ) )
-				.append( qualify( qualifier, values[0] ) );
+				.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 " ) );
 			}
 		}

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;
+  }
+}

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;
+  }
+}

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;
+  }
+
+}

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;
+	}
+}