Bug: Wrong join in query with @Inheritance and @DbForeignKey

ebean-core/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelpRefInherit.java

@@ -3,6 +3,7 @@ package io.ebeaninternal.server.deploy;
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.PersistenceContext;
 import io.ebeaninternal.server.query.SqlJoinType;
+import io.ebeaninternal.server.query.SqlTreeJoin;
 
 import java.sql.SQLException;
 
@@ -60,11 +61,28 @@ final class AssocOneHelpRefInherit extends AssocOneHelp {
     return ref;
   }
 
-  @Override
   void appendFrom(DbSqlContext ctx, SqlJoinType joinType) {
     // add join to support the discriminator column
     String relativePrefix = ctx.getRelativePrefix(property.name);
-    property.tableJoin.addJoin(joinType, relativePrefix, ctx);
+    ctx.addExtraJoin(new Extra(relativePrefix, joinType));
+  }
+
+  /**
+   * Extra join to support the discriminator column.
+   */
+  final class Extra implements SqlTreeJoin {
+    final String relativePrefix;
+    final SqlJoinType joinType;
+    Extra(String relativePrefix, SqlJoinType joinType) {
+      this.relativePrefix = relativePrefix;
+      this.joinType = joinType;
+    }
+
+    @Override
+    public void addJoin(DbSqlContext ctx) {
+      // add join to support the discriminator column *IF* join is not already present
+      property.tableJoin.addJoin(joinType, relativePrefix, ctx);
+    }
   }
 
   /**

ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbSqlContext.java

@@ -1,6 +1,7 @@
 package io.ebeaninternal.server.deploy;
 
 import io.ebeaninternal.server.query.SqlJoinType;
+import io.ebeaninternal.server.query.SqlTreeJoin;
 
 /**
  * Used to provide context during sql construction.
@@ -133,4 +134,13 @@ public interface DbSqlContext {
    */
   void appendFromForUpdate();
 
+  /**
+   * Delay adding an extra join to support inheritance discriminator in projection (IF required).
+   */
+  void addExtraJoin(SqlTreeJoin treeJoin);
+
+  /**
+   * Add extra joins *IF* required to support inheritance discriminator in projection.
+   */
+  void flushExtraJoins();
 }

ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultDbSqlContext.java

@@ -7,6 +7,7 @@ import io.ebeaninternal.server.util.ArrayStack;
 
 import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 
 final class DefaultDbSqlContext implements DbSqlContext {
 
@@ -43,7 +44,9 @@ final class DefaultDbSqlContext implements DbSqlContext {
 
   private String currentPrefix;
 
-  private ArrayList<BeanProperty> encryptedProps;
+  private List<BeanProperty> encryptedProps;
+
+  private List<SqlTreeJoin> extraJoins;
 
   private final CQueryDraftSupport draftSupport;
 
@@ -70,6 +73,24 @@ final class DefaultDbSqlContext implements DbSqlContext {
     return alias.isIncludeSoftDelete();
   }
 
+  @Override
+  public void addExtraJoin(SqlTreeJoin treeJoin) {
+    if (extraJoins == null) {
+      extraJoins = new ArrayList<>();
+    }
+    extraJoins.add(treeJoin);
+  }
+
+  @Override
+  public void flushExtraJoins() {
+    if (extraJoins != null) {
+      for (SqlTreeJoin extra : extraJoins) {
+        extra.addJoin(this);
+      }
+      extraJoins = null;
+    }
+  }
+
   @Override
   public void appendFromForUpdate() {
     if (fromForUpdate != null) {

ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java

@@ -256,7 +256,7 @@ public final class SqlTreeBuilder {
       }
       addManyWhereJoins(myJoinList);
     }
-    extraProps.forEach(props::add);
+    extraProps.forEach(props::add); //addExtra
 
     if (!rawSql && manyWhereJoins.isFormulaWithJoin(prefix)) {
       for (String property : manyWhereJoins.getFormulaJoinProperties(prefix)) {

ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeJoin.java

@@ -0,0 +1,15 @@
+package io.ebeaninternal.server.query;
+
+import io.ebeaninternal.server.deploy.DbSqlContext;
+
+/**
+ * Extra Sql joins conditionally added if required (after children are joined).
+ */
+public interface SqlTreeJoin {
+
+  /**
+   * Add the extra join if required.
+   */
+  void addJoin(DbSqlContext ctx);
+
+}

ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java

@@ -279,18 +279,16 @@ class SqlTreeNodeBean implements SqlTreeNode {
     ctx.pushTableAlias(prefix);
 
     baseTableAlias = ctx.getTableAlias(prefix);
-
     // join and return SqlJoinType to use for child joins
     joinType = appendFromBaseTable(ctx, joinType);
-
     for (STreeProperty property : properties) {
       // usually nothing... except for 1-1 Exported
       property.appendFrom(ctx, joinType, null);
     }
-
     for (SqlTreeNode child : children) {
       child.appendFrom(ctx, joinType);
     }
+    ctx.flushExtraJoins();
     ctx.popTableAlias();
     ctx.popJoin();
   }

ebean-test/src/test/java/org/tests/model/carwheeltruck/TestMTOInheritNoDiscriminator.java

@@ -9,10 +9,10 @@ import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-public class TestMTOInheritNoDiscriminator extends BaseTestCase {
+class TestMTOInheritNoDiscriminator extends BaseTestCase {
 
   @Test
-  public void test() {
+  void test() {
 
     final TTruckHolder holder = setup();
 
@@ -31,7 +31,7 @@ public class TestMTOInheritNoDiscriminator extends BaseTestCase {
 
     assertThat(sql).hasSize(1);
     if (isH2() || isPostgresCompatible()) {
-      assertSql(sql.get(0)).contains("select t0.id, t0.name, t0.version, t2.type, t0.truck_plate_no, t0.basic_id, t1.id, t1.some_uid, t1.foo, t1.owner_id from ttruck_holder t0 join tcar t2 on t2.plate_no = t0.truck_plate_no left join ttruck_holder_item t1 on t1.owner_id = t0.id where t0.id = ? order by t0.id");
+      assertSql(sql.get(0)).contains("select t0.id, t0.name, t0.version, t2.type, t0.truck_plate_no, t0.basic_id, t1.id, t1.some_uid, t1.foo, t1.owner_id from ttruck_holder t0 left join ttruck_holder_item t1 on t1.owner_id = t0.id join tcar t2 on t2.plate_no = t0.truck_plate_no where t0.id = ? order by t0.id");
     }
   }
 

ebean-test/src/test/java/org/tests/model/join/AccountAccess.java

@@ -0,0 +1,10 @@
+package org.tests.model.join;
+
+import javax.persistence.DiscriminatorValue;
+import javax.persistence.Entity;
+
+@DiscriminatorValue("A")
+@Entity
+public class AccountAccess extends HAccess {
+
+}

ebean-test/src/test/java/org/tests/model/join/BankAccount.java

@@ -0,0 +1,10 @@
+package org.tests.model.join;
+
+import javax.persistence.DiscriminatorValue;
+import javax.persistence.Entity;
+
+@DiscriminatorValue("B")
+@Entity
+public class BankAccount extends HAccount {
+
+}

ebean-test/src/test/java/org/tests/model/join/CustomerAccess.java

@@ -0,0 +1,10 @@
+package org.tests.model.join;
+
+import javax.persistence.DiscriminatorValue;
+import javax.persistence.Entity;
+
+@DiscriminatorValue("C")
+@Entity
+public class CustomerAccess extends HAccess {
+
+}

ebean-test/src/test/java/org/tests/model/join/HAccess.java

@@ -0,0 +1,68 @@
+package org.tests.model.join;
+
+import java.util.UUID;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.Inheritance;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+
+import io.ebean.annotation.DbForeignKey;
+
+@Inheritance
+@Entity
+public abstract class HAccess {
+
+  @Id
+  UUID id;
+
+  @ManyToOne
+  @DbForeignKey(noConstraint = true)
+  @JoinColumn(name = "accessor_id")
+  HCustomer accessor;
+
+  @ManyToOne
+  @DbForeignKey(noConstraint = true)
+  @JoinColumn(name = "principal_id")
+  HCustomer principal;
+
+  @ManyToOne
+  @DbForeignKey(noConstraint = true)
+  @JoinColumn(name = "access_account_number")
+  HAccount account;
+
+  public UUID getId() {
+    return id;
+  }
+
+  public void setId(UUID id) {
+    this.id = id;
+  }
+
+  public HCustomer getAccessor() {
+    return accessor;
+  }
+
+  public void setAccessor(HCustomer accessor) {
+    this.accessor = accessor;
+  }
+
+  public HCustomer getPrincipal() {
+    return principal;
+  }
+
+  public void setPrincipal(HCustomer principal) {
+    this.principal = principal;
+  }
+
+  public HAccount getAccount() {
+    return account;
+  }
+
+  public void setAccount(HAccount account) {
+    this.account = account;
+    setPrincipal(account.getOwner());
+  }
+
+}

ebean-test/src/test/java/org/tests/model/join/HAccount.java

@@ -0,0 +1,32 @@
+package org.tests.model.join;
+
+import javax.persistence.*;
+
+@Entity
+@Inheritance
+public abstract class HAccount {
+
+  @Id
+  String accountNumber;
+
+  @ManyToOne
+  @JoinColumn(name = "owner_id")
+  HCustomer owner;
+
+  public String getAccountNumber() {
+    return accountNumber;
+  }
+
+  public void setAccountNumber(String accountNumber) {
+    this.accountNumber = accountNumber;
+  }
+
+  public HCustomer getOwner() {
+    return owner;
+  }
+
+  public void setOwner(HCustomer owner) {
+    this.owner = owner;
+  }
+
+}

ebean-test/src/test/java/org/tests/model/join/HCustomer.java

@@ -0,0 +1,20 @@
+package org.tests.model.join;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+@Entity
+public class HCustomer {
+
+  @Id
+  final String cid;
+
+  final String name;
+
+  String status = "A";
+
+  public HCustomer(String cid, String name) {
+    this.cid = cid;
+    this.name = name;
+  }
+}

ebean-test/src/test/java/org/tests/query/TestQueryMultiJoinFetchPath.java

@@ -0,0 +1,69 @@
+package org.tests.query;
+
+import io.ebean.BaseTestCase;
+import io.ebean.DB;
+import io.ebean.Query;
+import org.junit.jupiter.api.Test;
+import org.tests.model.join.*;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class TestQueryMultiJoinFetchPath extends BaseTestCase {
+
+  @Test
+  void test() {
+
+    HCustomer c1 = new HCustomer("c1", "c1");
+    DB.save(c1);
+
+    HCustomer c2 = new HCustomer("c2", "c2");
+    DB.save(c2);
+
+    HCustomer c3 = new HCustomer("c3", "c3");
+    DB.save(c3);
+
+    HAccount a1 = new BankAccount();
+    a1.setAccountNumber("a1");
+    a1.setOwner(c1);
+    DB.save(a1);
+
+    CustomerAccess ca = new CustomerAccess();
+    ca.setAccessor(c3);
+    ca.setPrincipal(c1);
+    DB.save(ca);
+
+    AccountAccess aa = new AccountAccess();
+    aa.setAccessor(c2);
+    aa.setAccount(a1);
+    DB.save(aa);
+
+    List<Object> ids = DB.find(HAccess.class)
+      .where()
+      .eq("principal.status", "A")
+      .eq("accessor.status", "A")
+      .findIds();
+
+    assertThat(ids).hasSize(2);
+
+    Query<HAccess> query = DB.find(HAccess.class)
+      .fetch("account","accountNumber")
+      .fetch("accessor","name")
+      .where()
+      .eq("accessor.status", "A")
+      .eq("principal.status", "A")
+      .idIn(ids)
+      .query();
+
+    List<HAccess> accesses = query.findList();
+
+    assertThat(accesses).hasSize(2);
+    if (isH2()) {
+      assertThat(query.getGeneratedSql()).isEqualTo("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t0.accessor_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id in (?,?,?,?,?)");
+    } else {
+      assertThat(query.getGeneratedSql()).contains("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t0.accessor_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id ");
+    }
+  }
+
+}