Merge pull request #3145 from FOCONIS/m2m-where-formula

FIX: Query on formula m2o-property uses wrong alias:
ebean-orm · Aug 14, 2023 · 9097466 · 9097466
2 parents 99604d8 + 5720ee6
commit 9097466
Show file tree

Hide file tree

Showing 6 changed files with 135 additions and 33 deletions.
diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java
@@ -456,7 +456,8 @@ public int fetchPreference() {
   @Override
   public void appendFrom(DbSqlContext ctx, SqlJoinType joinType, String manyWhere) {
     if (formula && sqlFormulaJoin != null) {
-      ctx.appendFormulaJoin(sqlFormulaJoin, joinType, manyWhere);
+      String alias = ctx.tableAliasManyWhere(manyWhere);
+      ctx.appendFormulaJoin(sqlFormulaJoin, joinType, alias);
     } else if (secondaryTableJoin != null) {
       String relativePrefix = ctx.relativePrefix(secondaryTableJoinPrefix);
       secondaryTableJoin.addJoin(joinType, relativePrefix, ctx);

diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java
@@ -613,12 +613,24 @@ public void appendSelect(DbSqlContext ctx, boolean subQuery) {
     }
   }
 
+  /**
+   * Add table join with explicit table alias.
+   */
+  @Override
+  public SqlJoinType addJoin(SqlJoinType joinType, String a1, String a2, DbSqlContext ctx) {
+    if (sqlFormulaJoin != null) {
+      ctx.appendFormulaJoin(sqlFormulaJoin, joinType, a1);
+    }
+    return super.addJoin(joinType, a1, a2, ctx);
+  }
+
   @Override
   public void appendFrom(DbSqlContext ctx, SqlJoinType joinType, String manyWhere) {
     if (!isTransient && !primaryKeyExport) {
       localHelp.appendFrom(ctx, joinType);
       if (sqlFormulaJoin != null) {
-        ctx.appendFormulaJoin(sqlFormulaJoin, joinType, manyWhere);
+        String alias = ctx.tableAliasManyWhere(manyWhere);
+        ctx.appendFormulaJoin(sqlFormulaJoin, joinType, alias);
       }
     }
   }

diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbSqlContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbSqlContext.java
@@ -73,7 +73,7 @@ public interface DbSqlContext {
    * Append a Sql Formula join. This converts the "${ta}" keyword to the current
    * table alias.
    */
-  void appendFormulaJoin(String sqlFormulaJoin, SqlJoinType joinType, String manyWhere);
+  void appendFormulaJoin(String sqlFormulaJoin, SqlJoinType joinType, String tableAlias);
 
   /**
    * Return the current content length.

diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultDbSqlContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultDbSqlContext.java
@@ -197,7 +197,7 @@ public String tableAlias(String prefix) {
 
   @Override
   public String tableAliasManyWhere(String prefix) {
-    return alias.tableAliasManyWhere(prefix);
+    return prefix == null ? tableAliasStack.peek() : alias.tableAliasManyWhere(prefix);
   }
 
   @Override
@@ -225,9 +225,8 @@ public DefaultDbSqlContext append(String s) {
   }
 
   @Override
-  public void appendFormulaJoin(String sqlFormulaJoin, SqlJoinType joinType, String manyWhere) {
+  public void appendFormulaJoin(String sqlFormulaJoin, SqlJoinType joinType, String tableAlias) {
     // replace ${ta} placeholder with the real table alias...
-    String tableAlias = manyWhere == null ? tableAliasStack.peek() : tableAliasManyWhere(manyWhere);
     String converted = sqlFormulaJoin.replace(tableAliasPlaceHolder, tableAlias);
     if (formulaJoins == null) {
       formulaJoins = new HashSet<>();
@@ -240,11 +239,17 @@ public void appendFormulaJoin(String sqlFormulaJoin, SqlJoinType joinType, Strin
     formulaJoins.add(converted);
     sb.append(" ");
     if (joinType == SqlJoinType.OUTER) {
-      if ("join".equalsIgnoreCase(sqlFormulaJoin.substring(0, 4))) {
+      if ("join".equalsIgnoreCase(converted.substring(0, 4))) {
         // prepend left as we are in the 'many' part
         sb.append("left ");
       }
     }
+    if (joinType == SqlJoinType.INNER) {
+      if ("left join".equalsIgnoreCase(converted.substring(0, 9))) {
+        // remove left as we do not need it
+        converted = converted.substring(5);
+      }
+    }
     sb.append(converted);
   }
 

diff --git a/ebean-test/src/test/java/org/tests/model/basic/TreeNode.java b/ebean-test/src/test/java/org/tests/model/basic/TreeNode.java
@@ -0,0 +1,31 @@
+package org.tests.model.basic;
+
+import io.ebean.annotation.Formula;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.ManyToOne;
+import javax.persistence.OneToMany;
+import java.util.List;
+
+/**
+ * @author Roland Praml, FOCONIS AG
+ */
+@Entity
+public class TreeNode {
+
+  @Id
+  private int id;
+
+  @ManyToOne
+  private TreeNode parent;
+  @OneToMany
+  private List<TreeNode> children;
+
+  private int softRef;
+
+  @Formula(select = "${ta}_ref.id", join = "left join e_basic ${ta}_ref on ${ta}_ref.id = ${ta}.soft_ref")
+  @ManyToOne
+  private EBasic ref;
+
+}
diff --git a/ebean-test/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java b/ebean-test/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java
@@ -1,14 +1,15 @@
 package org.tests.query.joins;
 
-import io.ebean.xtest.BaseTestCase;
 import io.ebean.DB;
 import io.ebean.Query;
 import io.ebean.test.LoggedSql;
+import io.ebean.xtest.BaseTestCase;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.tests.model.basic.Order;
 import org.tests.model.basic.OrderShipment;
 import org.tests.model.basic.ResetBasicData;
+import org.tests.model.basic.TreeNode;
 import org.tests.model.family.ChildPerson;
 import org.tests.model.family.ParentPerson;
 
@@ -31,8 +32,8 @@ public void test_OrderFindIds() {
     LoggedSql.start();
 
     List<Integer> orderIds = DB.find(Order.class)
-        .where().eq("totalItems", 3)
-        .findIds();
+      .where().eq("totalItems", 3)
+      .findIds();
     assertThat(orderIds).hasSize(2);
 
     List<String> loggedSql = LoggedSql.stop();
@@ -46,8 +47,8 @@ public void test_OrderFindList() {
     LoggedSql.start();
 
     List<Order> orders = DB.find(Order.class)
-        .where().eq("totalItems", 3)
-        .findList();
+      .where().eq("totalItems", 3)
+      .findList();
     assertThat(orders).hasSize(2);
 
     List<String> loggedSql = LoggedSql.stop();
@@ -122,11 +123,11 @@ public void testWhereOnChainedFormulaManyWhere() {
     }
     assertThat(shipQuery.getGeneratedSql()).contains(
       "from or_order_ship t0 " +
-      "join o_order u1 on u1.id = t0.order_id " +
-      "join or_order_ship u2 on u2.order_id = u1.id " +
-      "join o_order u3 on u3.id = u2.order_id " +
-      "left join (select order_id, count(*) as total_items, sum(order_qty*unit_price) as total_amount from o_order_detail group by order_id) z_bu3 on z_bu3.order_id = u3.id " +
-      "where z_bu3.total_amount is not null");
+        "join o_order u1 on u1.id = t0.order_id " +
+        "join or_order_ship u2 on u2.order_id = u1.id " +
+        "join o_order u3 on u3.id = u2.order_id " +
+        "left join (select order_id, count(*) as total_items, sum(order_qty*unit_price) as total_amount from o_order_detail group by order_id) z_bu3 on z_bu3.order_id = u3.id " +
+        "where z_bu3.total_amount is not null");
   }
 
   @Test
@@ -188,9 +189,9 @@ public void test_OrderFindSingleAttributeList() {
     LoggedSql.start();
 
     List<Date> orderDates = DB.find(Order.class)
-        .select("orderDate")
-        .where().eq("totalItems", 3)
-        .findSingleAttributeList();
+      .select("orderDate")
+      .where().eq("totalItems", 3)
+      .findSingleAttributeList();
     assertThat(orderDates).hasSize(2);
 
     List<String> sql = LoggedSql.stop();
@@ -205,11 +206,11 @@ public void test_OrderFindOne() {
     LoggedSql.start();
 
     Order order = DB.find(Order.class)
-        .select("totalItems")
-        .where().eq("totalItems", 3)
-        .setMaxRows(1)
-        .orderById(true)
-        .findOne();
+      .select("totalItems")
+      .where().eq("totalItems", 3)
+      .setMaxRows(1)
+      .orderById(true)
+      .findOne();
 
     assertThat(order.getTotalItems()).isEqualTo(3);
 
@@ -229,8 +230,8 @@ public void test_ParentPersonFindIds() {
     LoggedSql.start();
 
     List<ParentPerson> orderIds = DB.find(ParentPerson.class)
-        .where().eq("totalAge", 3)
-        .findIds();
+      .where().eq("totalAge", 3)
+      .findIds();
 
     List<String> loggedSql = LoggedSql.stop();
     assertEquals(1, loggedSql.size());
@@ -243,10 +244,10 @@ public void test_ParentPersonFindList() {
     LoggedSql.start();
 
     DB.find(ParentPerson.class)
-        .select("identifier")
-        //.where().eq("totalAge", 3)
-        .where().eq("familyName", "foo")
-        .findList();
+      .select("identifier")
+      //.where().eq("totalAge", 3)
+      .where().eq("familyName", "foo")
+      .findList();
 
     List<String> sql = LoggedSql.stop();
     assertEquals(1, sql.size());
@@ -323,12 +324,64 @@ public void test_ChildPersonParentFindCount() {
     LoggedSql.start();
 
     DB.find(ChildPerson.class)
-        .where().eq("parent.totalAge", 3)
-        .findCount();
+      .where().eq("parent.totalAge", 3)
+      .findCount();
 
     List<String> loggedSql = LoggedSql.stop();
     assertEquals(1, loggedSql.size());
     assertThat(loggedSql.get(0)).contains("select count(*) from child_person t0 left join parent_person t1 on t1.identifier = t0.parent_identifier");
     assertThat(loggedSql.get(0)).contains("where coalesce(f2.child_age, 0) = ?");
   }
+
+  @Test
+  public void test_softRef() {
+
+    LoggedSql.start();
+
+    DB.find(TreeNode.class)
+      .where().isNotNull("ref.id")
+      .findList();
+
+    List<String> loggedSql = LoggedSql.stop();
+    assertThat(loggedSql.get(0)).contains("select t0.id, t0.soft_ref, t0.parent_id, t0_ref.id "
+      + "from tree_node t0 "
+      + "left join e_basic t0_ref on t0_ref.id = t0.soft_ref "
+      + "left join e_basic t1 on t1.id = t0_ref.id where t1.id is not null");
+  }
+
+  @Test
+  public void test_softRefChildren() {
+
+    LoggedSql.start();
+
+    DB.find(TreeNode.class).select("id")
+      .where().isNotNull("children.ref.id")
+      .findList();
+
+    List<String> loggedSql = LoggedSql.stop();
+    if (isPostgresCompatible()) {
+      // TBD
+    } else {
+      assertThat(loggedSql.get(0)).contains("select distinct t0.id "
+        + "from tree_node t0 "
+        + "join tree_node u1 on u1.parent_id = t0.id "
+        + "join e_basic u1_ref on u1_ref.id = u1.soft_ref "
+        + "join e_basic u2 on u2.id = u1_ref.id where u2.id is not null");
+    }
+    LoggedSql.start();
+    DB.find(TreeNode.class).select("id")
+      .where().isNull("children.ref.id")
+      .findList();
+
+    loggedSql = LoggedSql.stop();
+    if (isPostgresCompatible()) {
+      // TBD
+    } else {
+      assertThat(loggedSql.get(0)).contains("select distinct t0.id "
+        + "from tree_node t0 "
+        + "left join tree_node u1 on u1.parent_id = t0.id "
+        + "left join e_basic u1_ref on u1_ref.id = u1.soft_ref "
+        + "left join e_basic u2 on u2.id = u1_ref.id where u2.id is null");
+    }
+  }
 }