HHH-16438 - Apply some suggestions from Christian's code review

Co-authored-by: Christian Beikov <christian.beikov@gmail.com>
This commit is contained in:
Jan Schatteman 2023-04-18 17:24:54 +02:00
parent 6b21d436ce
commit 85a636c856
2 changed files with 9 additions and 42 deletions

View File

@ -49,6 +49,7 @@ import org.hibernate.id.CompositeNestedGeneratedValueGenerator;
import org.hibernate.id.OptimizableGenerator; import org.hibernate.id.OptimizableGenerator;
import org.hibernate.id.enhanced.Optimizer; import org.hibernate.id.enhanced.Optimizer;
import org.hibernate.internal.FilterHelper; import org.hibernate.internal.FilterHelper;
import org.hibernate.internal.util.MutableObject;
import org.hibernate.internal.util.collections.CollectionHelper; import org.hibernate.internal.util.collections.CollectionHelper;
import org.hibernate.internal.util.collections.Stack; import org.hibernate.internal.util.collections.Stack;
import org.hibernate.internal.util.collections.StandardStack; import org.hibernate.internal.util.collections.StandardStack;
@ -3166,7 +3167,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
} }
private TableGroup consumeEntityJoin(SqmEntityJoin<?> sqmJoin, TableGroup lhsTableGroup, boolean transitive) { private TableGroup consumeEntityJoin(SqmEntityJoin<?> sqmJoin, TableGroup lhsTableGroup, boolean transitive) {
final List<Predicate> predicates = new ArrayList<>(); final MutableObject<Predicate> predicate = new MutableObject<>();
final EntityPersister entityDescriptor = resolveEntityPersister( sqmJoin.getReferencedPathSource() ); final EntityPersister entityDescriptor = resolveEntityPersister( sqmJoin.getReferencedPathSource() );
final SqlAstJoinType correspondingSqlJoinType = sqmJoin.getSqmJoinType().getCorrespondingSqlJoinType(); final SqlAstJoinType correspondingSqlJoinType = sqmJoin.getSqmJoinType().getCorrespondingSqlJoinType();
@ -3175,17 +3176,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
sqmJoin.getNavigablePath(), sqmJoin.getNavigablePath(),
sqmJoin.getExplicitAlias(), sqmJoin.getExplicitAlias(),
null, null,
() -> (predicate) -> { () -> p -> predicate.set( combinePredicates( predicate.get(), p ) ),
switch ( correspondingSqlJoinType ) {
case INNER:
case LEFT:
case FULL:
predicates.add( predicate );
break;
default:
additionalRestrictions = combinePredicates( additionalRestrictions, predicate );
}
},
this this
); );
getFromClauseIndex().register( sqmJoin, tableGroup ); getFromClauseIndex().register( sqmJoin, tableGroup );
@ -3194,7 +3185,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
sqmJoin.getNavigablePath(), sqmJoin.getNavigablePath(),
correspondingSqlJoinType, correspondingSqlJoinType,
tableGroup, tableGroup,
predicates.size() == 1 ? predicates.get(0) : null predicate.get()
); );
// add any additional join restrictions // add any additional join restrictions

View File

@ -23,6 +23,7 @@ import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertEquals;
@ -102,33 +103,7 @@ public class JoinWithSingleTableInheritanceTest {
} }
@Test @Test
public void testCrossJoinOnSingleTableInheritance(SessionFactoryScope scope) { @Disabled(value = "HHH-16494")
scope.inTransaction(
s -> {
s.persist(new ChildEntityA( 11 ));
s.persist(new ChildEntityA( 12 ));
s.persist(new ChildEntityB( 21 ));
RootOne r1 = new RootOne(1);
r1.setSomeOtherId( 11 );
s.persist( r1 );
RootOne r2 = new RootOne(2);
r2.setSomeOtherId( 12 );
s.persist( r2 );
RootOne r3 = new RootOne(3);
r3.setSomeOtherId( 21 );
s.persist( r3 );
}
);
scope.inTransaction(
s -> {
List<Object[]> l = s.createSelectionQuery( "select r.id, r.someOtherId from RootOne r join ChildEntityA", Object[].class ).list();
assertEquals( 6, l.size() );
}
);
}
@Test
public void testRightJoinOnSingleTableInheritance(SessionFactoryScope scope) { public void testRightJoinOnSingleTableInheritance(SessionFactoryScope scope) {
scope.inTransaction( scope.inTransaction(
s -> { s -> {
@ -172,6 +147,7 @@ public class JoinWithSingleTableInheritanceTest {
@Test @Test
@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsFullJoin.class) @RequiresDialectFeature(feature = DialectFeatureChecks.SupportsFullJoin.class)
@Disabled(value = "HHH-16494")
public void testFullJoinOnSingleTableInheritance(SessionFactoryScope scope) { public void testFullJoinOnSingleTableInheritance(SessionFactoryScope scope) {
scope.inTransaction( scope.inTransaction(
s -> { s -> {
@ -195,8 +171,8 @@ public class JoinWithSingleTableInheritanceTest {
scope.inTransaction( scope.inTransaction(
s -> { s -> {
List<Object[]> l = s.createSelectionQuery( "select r.id, r.someOtherId from RootOne r full join ChildEntityA ce order by ce.id, r.id", Object[].class ).list(); List<Object[]> l = s.createSelectionQuery( "select r.id, r.someOtherId from RootOne r full join ChildEntityA ce on ce.id = r.someOtherId order by ce.id, r.id", Object[].class ).list();
assertEquals( 10, l.size() ); assertEquals( 7, l.size() );
} }
); );
} }