diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/AbstractSqmPath.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/AbstractSqmPath.java index a10674e4fe64..faf3131949f8 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/AbstractSqmPath.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/AbstractSqmPath.java @@ -6,10 +6,10 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import org.hibernate.AssertionFailure; @@ -33,7 +33,6 @@ import jakarta.persistence.metamodel.PluralAttribute; import jakarta.persistence.metamodel.SingularAttribute; -import static java.util.Collections.emptyList; import static org.hibernate.internal.util.NullnessUtil.castNonNull; /** @@ -49,7 +48,7 @@ public abstract class AbstractSqmPath extends AbstractSqmExpression implem * E.g., given {@code p.mate.mate} the {@code SqmRoot} identified by {@code p} would * have a reusable path for the {@code p.mate} path. */ - private Map> reusablePaths; + private final Map> reusablePaths = new ConcurrentHashMap<>( 0 ); protected AbstractSqmPath( NavigablePath navigablePath, @@ -103,24 +102,19 @@ public SqmPath getLhs() { @Override public List> getReusablePaths() { - return reusablePaths == null ? emptyList() : new ArrayList<>( reusablePaths.values() ); + return new ArrayList<>( reusablePaths.values() ); } @Override public void visitReusablePaths(Consumer> consumer) { - if ( reusablePaths != null ) { - reusablePaths.values().forEach( consumer ); - } + reusablePaths.values().forEach( consumer ); } @Override public void registerReusablePath(SqmPath path) { assert path.getLhs() == this; - if ( reusablePaths == null ) { - reusablePaths = new HashMap<>(); - } final String relativeName = path.getNavigablePath().getLocalName(); - final SqmPath previous = reusablePaths.put( relativeName, path ); + final SqmPath previous = reusablePaths.putIfAbsent( relativeName, path ); if ( previous != null && previous != path ) { throw new IllegalStateException( "Implicit-join path registration unexpectedly overrode previous registration - " + relativeName ); } @@ -128,7 +122,7 @@ public void registerReusablePath(SqmPath path) { @Override public SqmPath getReusablePath(String name) { - return reusablePaths == null ? null : reusablePaths.get( name ); + return reusablePaths.get( name ); } @Override @@ -206,18 +200,10 @@ protected SqmPath resolvePath(PersistentAttribute attribute) { protected SqmPath resolvePath(String attributeName, SqmPathSource pathSource) { final SqmPathSource intermediatePathSource = getResolvedModel().getIntermediatePathSource( pathSource ); - if ( reusablePaths == null ) { - reusablePaths = new HashMap<>(); - final SqmPath path = pathSource.createSqmPath( this, intermediatePathSource ); - reusablePaths.put( attributeName, path ); - return path; - } - else { - //noinspection unchecked - return (SqmPath) - reusablePaths.computeIfAbsent( attributeName, - name -> pathSource.createSqmPath( this, intermediatePathSource ) ); - } + //noinspection unchecked + return (SqmPath) + reusablePaths.computeIfAbsent( attributeName, + name -> pathSource.createSqmPath( this, intermediatePathSource ) ); } protected SqmTreatedPath getTreatedPath(ManagedDomainType treatTarget) { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sqm/ConcurrentQueryCreationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sqm/ConcurrentQueryCreationTest.java new file mode 100644 index 000000000000..7a272d867a24 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sqm/ConcurrentQueryCreationTest.java @@ -0,0 +1,195 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.sqm; + +import java.util.List; +import java.util.concurrent.CompletionService; +import java.util.concurrent.ExecutorCompletionService; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; + +import org.hibernate.cfg.Configuration; +import org.hibernate.cfg.Environment; +import org.hibernate.engine.spi.SessionImplementor; + +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.hibernate.testing.orm.junit.JiraKey; +import org.junit.Test; + +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.Inheritance; +import jakarta.persistence.InheritanceType; +import jakarta.persistence.Version; + +import static org.junit.Assert.assertEquals; + +@JiraKey(value = "HHH-19429") +public class ConcurrentQueryCreationTest extends BaseCoreFunctionalTestCase { + + private static final int NUM_THREADS = 32; + + @Override + protected void configure(Configuration configuration) { + super.configure( configuration ); + configuration.setProperty( Environment.POOL_SIZE, Integer.toString( NUM_THREADS ) ); + } + + @Test + public void versionedUpdate() throws Exception { + Consumer action = session -> { + SimpleEntity entity = new SimpleEntity( "jack" ); + session.persist( entity ); + session.createMutationQuery( "UPDATE VERSIONED SimpleEntity e SET e.name = :name WHERE e.id = :id" ) + .setParameter( "id", entity.getId() ) + .setParameter( "name", "new name" ) + .executeUpdate(); + }; + + runConcurrently( action ); + } + + @Test + public void queryWithTreat() throws Exception { + Consumer action = session -> { + SpecificEntity specificEntity = new SpecificEntity( "some name" ); + session.persist( specificEntity ); + session.persist( new OtherSpecificEntity( "some name" ) ); + + List results = session.createQuery( + """ + SELECT COUNT(*) FROM ParentEntity e WHERE e.id = :id + AND ( + TREAT(e as SpecificEntity).name = :name + OR + TREAT(e as OtherSpecificEntity).name = :name + )""", Long.class + ) + .setParameter( "id", specificEntity.getId() ) + .setParameter( "name", "some name" ) + .getResultList(); + + assertEquals( 1, results.size() ); + assertEquals( 1L, results.get( 0 ).longValue() ); + }; + + runConcurrently( action ); + } + + private void runConcurrently(Consumer action) throws Exception { + ExecutorService executor = Executors.newFixedThreadPool( NUM_THREADS ); + try { + CompletionService completionService = new ExecutorCompletionService<>( executor ); + + for ( int round = 0; round < 100; round++ ) { + for ( int i = 0; i < NUM_THREADS; i++ ) { + completionService.submit( () -> { + inTransaction( action ); + return null; + } ); + } + + for ( int i = 0; i < NUM_THREADS; i++ ) { + completionService.take().get( 1, TimeUnit.MINUTES ); + } + + rebuildSessionFactory(); + } + } + finally { + executor.shutdown(); + executor.awaitTermination( 1, TimeUnit.MINUTES ); + } + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { + SimpleEntity.class, + ParentEntity.class, + SpecificEntity.class, + OtherSpecificEntity.class + }; + } + + @Override + protected boolean isCleanupTestDataRequired() { + return true; + } + + @Entity(name = "SimpleEntity") + public static class SimpleEntity { + @Id + @GeneratedValue + private Integer id; + + @Version + private Integer version; + + private String name; + + SimpleEntity() { + } + + SimpleEntity(String name) { + this.name = name; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + @Entity(name = "ParentEntity") + @Inheritance(strategy = InheritanceType.JOINED) + public static class ParentEntity { + @Id + @GeneratedValue + private Integer id; + + public Integer getId() { + return id; + } + } + + @Entity(name = "SpecificEntity") + public static class SpecificEntity extends ParentEntity { + private String name; + + public SpecificEntity() { + } + + public SpecificEntity(String name) { + this.name = name; + } + } + + @Entity(name = "OtherSpecificEntity") + public static class OtherSpecificEntity extends ParentEntity { + private String name; + + public OtherSpecificEntity() { + } + + public OtherSpecificEntity(String name) { + this.name = name; + } + } +}