From 6b21d436ce291d8a0371ec61ac50a3e74867d97d Mon Sep 17 00:00:00 2001 From: Jan Schatteman Date: Fri, 14 Apr 2023 17:08:41 +0200 Subject: [PATCH] HHH-16438 - fix for issue - move the discriminator condition from the where clause to the join clause - add another test to JoinWithSingleTableInheritanceTest Signed-off-by: Jan Schatteman --- .../sqm/sql/BaseSqmToSqlAstConverter.java | 17 ++- .../JoinWithSingleTableInheritanceTest.java | 138 ++++++++++++++++-- 2 files changed, 139 insertions(+), 16 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index f77a05418a..2df43588de 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -3166,6 +3166,7 @@ public abstract class BaseSqmToSqlAstConverter extends Base } private TableGroup consumeEntityJoin(SqmEntityJoin sqmJoin, TableGroup lhsTableGroup, boolean transitive) { + final List predicates = new ArrayList<>(); final EntityPersister entityDescriptor = resolveEntityPersister( sqmJoin.getReferencedPathSource() ); final SqlAstJoinType correspondingSqlJoinType = sqmJoin.getSqmJoinType().getCorrespondingSqlJoinType(); @@ -3174,16 +3175,26 @@ public abstract class BaseSqmToSqlAstConverter extends Base sqmJoin.getNavigablePath(), sqmJoin.getExplicitAlias(), null, - () -> (predicate) -> additionalRestrictions = combinePredicates( additionalRestrictions, predicate ), + () -> (predicate) -> { + switch ( correspondingSqlJoinType ) { + case INNER: + case LEFT: + case FULL: + predicates.add( predicate ); + break; + default: + additionalRestrictions = combinePredicates( additionalRestrictions, predicate ); + } + }, this - ); + ); getFromClauseIndex().register( sqmJoin, tableGroup ); final TableGroupJoin tableGroupJoin = new TableGroupJoin( sqmJoin.getNavigablePath(), correspondingSqlJoinType, tableGroup, - null + predicates.size() == 1 ? predicates.get(0) : null ); // add any additional join restrictions diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/join/JoinWithSingleTableInheritanceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/join/JoinWithSingleTableInheritanceTest.java index 67296cb46c..68d19a1a0c 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/join/JoinWithSingleTableInheritanceTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/join/JoinWithSingleTableInheritanceTest.java @@ -8,15 +8,17 @@ package org.hibernate.orm.test.join; import java.util.List; +import jakarta.persistence.Column; import jakarta.persistence.DiscriminatorColumn; import jakarta.persistence.DiscriminatorType; import jakarta.persistence.DiscriminatorValue; import jakarta.persistence.Entity; -import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.orm.junit.DialectFeatureChecks; import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.RequiresDialectFeature; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; @@ -45,8 +47,8 @@ public class JoinWithSingleTableInheritanceTest { public void cleanup( SessionFactoryScope scope ) { scope.inTransaction( s -> { - s.createMutationQuery( "delete from AbstractSuperClass" ).executeUpdate(); s.createMutationQuery( "delete from RootOne" ).executeUpdate(); + s.createMutationQuery( "delete from AbstractSuperClass" ).executeUpdate(); } ); } @@ -54,7 +56,7 @@ public class JoinWithSingleTableInheritanceTest { @Test public void testLeftJoinOnSingleTableInheritance(SessionFactoryScope scope) { scope.inTransaction( - s -> s.persist( new RootOne() ) + s -> s.persist( new RootOne(1) ) ); scope.inTransaction( @@ -71,10 +73,10 @@ public class JoinWithSingleTableInheritanceTest { s -> { s.persist(new ChildEntityA( 11 )); s.persist(new ChildEntityB( 21 )); - RootOne r1 = new RootOne(); + RootOne r1 = new RootOne(1); r1.setSomeOtherId( 11 ); s.persist( r1 ); - RootOne r2 = new RootOne(); + RootOne r2 = new RootOne(2); r2.setSomeOtherId( 21 ); s.persist( r2 ); } @@ -82,28 +84,132 @@ public class JoinWithSingleTableInheritanceTest { scope.inTransaction( s -> { - List l = s.createSelectionQuery( "select r.id, r.someOtherId, ce.id from RootOne r left join ChildEntityA ce on ce.id = r.someOtherId order by r.id", Object[].class ).list(); + List l = s.createSelectionQuery( "select r.id, r.someOtherId, ce.id, ce.disc_col from RootOne r left join ChildEntityA ce on ce.id = r.someOtherId order by r.id", Object[].class ).list(); assertEquals( 2, l.size() ); Object[] r1 = l.get( 0 ); - assertEquals( (int) r1[0], 1 ); - assertEquals( (int) r1[1], 11 ); - assertEquals( (int) r1[2], 11 ); + assertEquals( 1, (int) r1[0] ); + assertEquals( 11, (int) r1[1] ); + assertEquals( 11, (int) r1[2] ); + assertEquals( 1, (int) r1[3] ); // r2 has to be there, but shouldn't have any data w/ respect to ChildEntityB Object[] r2 = l.get( 1 ); - assertEquals( (int) r2[0], 2 ); - assertEquals( (int) r2[1], 21 ); + assertEquals( 2, (int) r2[0] ); + assertEquals( 21, (int) r2[1] ); assertNull( r2[2] ); + assertNull( r2[3] ); + } + ); + } + @Test + public void testCrossJoinOnSingleTableInheritance(SessionFactoryScope scope) { + 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 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) { + 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( 11 ); + s.persist( r2 ); + RootOne r3 = new RootOne(3); + r3.setSomeOtherId( 21 ); + s.persist( r3 ); + } + ); + + scope.inTransaction( + s -> { + List l = s.createSelectionQuery( "select r.id, r.someOtherId, ce.id, ce.disc_col from RootOne r right join ChildEntityA ce on ce.id = r.someOtherId order by ce.id, r.id", Object[].class ).list(); + assertEquals( 3, l.size() ); + Object[] r1 = l.get( 0 ); + assertEquals( 1, (int) r1[0] ); + assertEquals( 11, (int) r1[1] ); + assertEquals( 11, (int) r1[2] ); + assertEquals( 1, (int) r1[3] ); + Object[] r2 = l.get( 1 ); + assertEquals( 2, (int) r2[0] ); + assertEquals( 11, (int) r2[1] ); + assertEquals( 11, (int) r2[2] ); + assertEquals( 1, (int) r2[3] ); + Object[] r3 = l.get( 2 ); + assertNull( r3[0] ); + assertNull( r3[1] ); + assertEquals( 12, (int) r3[2] ); + assertEquals( 1, (int) r3[3] ); + } + ); + } + + @Test + @RequiresDialectFeature(feature = DialectFeatureChecks.SupportsFullJoin.class) + public void testFullJoinOnSingleTableInheritance(SessionFactoryScope scope) { + scope.inTransaction( + s -> { + s.persist(new ChildEntityA( 11 )); + s.persist(new ChildEntityA( 12 )); + s.persist(new ChildEntityB( 21 )); + s.persist(new ChildEntityB( 22 )); + RootOne r1 = new RootOne(1); + r1.setSomeOtherId( 11 ); + s.persist( r1 ); + RootOne r2 = new RootOne(2); + r2.setSomeOtherId( 11 ); + s.persist( r2 ); + RootOne r3 = new RootOne(3); + r3.setSomeOtherId( 21 ); + s.persist( r3 ); + RootOne r4 = new RootOne(4); + s.persist( r4 ); + } + ); + + scope.inTransaction( + s -> { + List l = s.createSelectionQuery( "select r.id, r.someOtherId from RootOne r full join ChildEntityA ce order by ce.id, r.id", Object[].class ).list(); + assertEquals( 10, l.size() ); } ); } @Entity(name = "AbstractSuperClass") - @DiscriminatorColumn(name = "DISC_COL", discriminatorType = DiscriminatorType.INTEGER) + @DiscriminatorColumn(name = "disc_col", discriminatorType = DiscriminatorType.INTEGER) public static abstract class AbstractSuperClass { @Id private Integer id; + @Column(insertable = false, updatable = false) + private Integer disc_col; + public AbstractSuperClass(Integer id) { this.id = id; } @@ -128,10 +234,16 @@ public class JoinWithSingleTableInheritanceTest { @Entity(name = "RootOne") public static class RootOne { @Id - @GeneratedValue Integer id; Integer someOtherId; + public RootOne() { + } + + public RootOne(Integer id) { + this.id = id; + } + public Integer getSomeOtherId() { return someOtherId; }