Bug fix and change filterMany to put predicates into JOIN clause, Fix for #3626

ebean-core/src/main/java/io/ebeaninternal/api/SpiQueryManyJoin.java

@@ -15,8 +15,4 @@ public interface SpiQueryManyJoin {
    */
   String fetchOrderBy();
 
-  /**
-   * Wrap the filter many expression with a condition allowing lEFT JOIN null matching row.
-   */
-  String idNullOr(String filterManyExpression);
 }

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

@@ -36,11 +36,6 @@ public String toString() {
     return "prefix:" + prefix + " name:" + name + " dbColumn:" + dbColumn + " ph:" + placeHolder;
   }
 
-  @Override
-  public String idNullOr(String filterManyExpression) {
-    throw new UnsupportedOperationException();
-  }
-
   @Override
   public boolean isAggregation() {
     return false;

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

@@ -513,11 +513,6 @@ public boolean isAssignableFrom(Class<?> type) {
     return owningType.isAssignableFrom(type);
   }
 
-  @Override
-  public String idNullOr(String filterManyExpression) {
-    throw new UnsupportedOperationException();
-  }
-
   @Override
   public void loadIgnore(DbReadContext ctx) {
     ctx.dataReader().incrementPos(1);

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

@@ -634,11 +634,6 @@ public String fetchOrderBy() {
     return fetchOrderBy;
   }
 
-  @Override
-  public String idNullOr(String filterManyExpression) {
-    return targetIdBinder.idNullOr(name, filterManyExpression);
-  }
-
   /**
    * Return the order by for use when lazy loading the associated collection.
    */

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

@@ -154,4 +154,9 @@ public interface DbSqlContext {
    * as it was already added to the query.
    */
   boolean joinAdded();
+
+  /**
+   * Include the filter many predicates if specified into the JOIN clause.
+   */
+  void includeFilterMany();
 }

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

@@ -27,11 +27,6 @@ public interface IdBinder {
    */
   void initialise();
 
-  /**
-   * Wrap the filter many expression with a condition allowing lEFT JOIN null matching row.
-   */
-  String idNullOr(String name, String filterManyExpression);
-
   String idSelect();
 
   /**

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

@@ -45,20 +45,6 @@ public void initialise() {
     this.idInValueSql = idInExpandedForm ? idInExpanded() : idInCompressed();
   }
 
-  @Override
-  public String idNullOr(String prefix, String filterManyExpression) {
-    StringBuilder sb = new StringBuilder(100);
-    sb.append("((");
-    for (int i = 0; i < props.length; i++) {
-      if (i > 0) {
-        sb.append(" and ");
-      }
-      sb.append("${").append(prefix).append('}').append(props[i].dbColumn()).append(" is null");
-    }
-    sb.append(") or (").append(filterManyExpression).append("))");
-    return sb.toString();
-  }
-
   @Override
   public String idSelect() {
     return embIdProperty.name();

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

@@ -28,11 +28,6 @@ public IdBinderEmpty() {
   public void initialise() {
   }
 
-  @Override
-  public String idNullOr(String name, String filterManyExpression) {
-    throw new UnsupportedOperationException();
-  }
-
   @Override
   public String idSelect() {
     return "";

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

@@ -261,9 +261,4 @@ public String cacheKeyFromBean(EntityBean bean) {
     final Object value = idProperty.getValue(bean);
     return scalarType.format(value);
   }
-
-  @Override
-  public String idNullOr(String prefix, String filterManyExpression) {
-    return "(${" + prefix + "}" + idProperty.dbColumn() + " is null or (" + filterManyExpression + "))";
-  }
 }

ebean-core/src/main/java/io/ebeaninternal/server/el/ElPropertyChain.java

@@ -286,8 +286,4 @@ public void pathSet(Object bean, Object value) {
     }
   }
 
-  @Override
-  public String idNullOr(String filterManyExpression) {
-    throw new UnsupportedOperationException();
-  }
 }

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

@@ -715,7 +715,7 @@ private SqlLimitResponse buildSql() {
       appendHistoryAsOfPredicate();
       appendFindId();
       appendToWhere(predicates.dbWhere());
-      appendToWhere(predicates.dbFilterMany());
+      appendToWhere(predicates.dbFilterManyWhere());
       if (!query.isIncludeSoftDeletes()) {
         appendSoftDelete();
       }

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

@@ -39,6 +39,7 @@ public final class CQueryPredicates {
   private final Object idValue;
   private final BindParams bindParams;
   private DefaultExpressionRequest filterMany;
+  private boolean filterManyJoin;
   /**
    * Bind values from the where expressions.
    */
@@ -106,6 +107,9 @@ public String bind(DataBind dataBind) throws SQLException {
         dataBind.append(", ");
       }
     }
+    if (filterManyJoin) {
+      filterMany.bind(dataBind);
+    }
     if (idValue != null) {
       // this is a find by id type query...
       request.descriptor().bindId(dataBind, idValue);
@@ -119,7 +123,7 @@ public String bind(DataBind dataBind) throws SQLException {
     if (where != null) {
       where.bind(dataBind);
     }
-    if (filterMany != null) {
+    if (!filterManyJoin && filterMany != null) {
       filterMany.bind(dataBind);
     }
     if (having != null) {
@@ -189,9 +193,10 @@ public void prepare(boolean buildSql) {
       if (chunk != null) {
         SpiExpressionList<?> filterManyExpr = chunk.getFilterMany();
         if (filterManyExpr != null) {
-          this.filterMany = new DefaultExpressionRequest(request, deployParser, binder, filterManyExpr);
+          filterManyJoin = chunk.isFilterManyJoin();
+          filterMany = new DefaultExpressionRequest(request, deployParser, binder, filterManyExpr);
           if (buildSql) {
-            dbFilterMany = manyProperty.idNullOr(filterMany.buildSql());
+            dbFilterMany = filterMany.buildSql();
           }
         }
       }
@@ -339,10 +344,17 @@ String dbWhere() {
   }
 
   /**
-   * Return a db filter for filtering many fetch joins.
+   * Return a db filter to be included in the WHERE.
+   */
+  String dbFilterManyWhere() {
+    return filterManyJoin ? null : dbFilterMany;
+  }
+
+  /**
+   * Return a db filter to be included in the JOIN.
    */
-  String dbFilterMany() {
-    return dbFilterMany;
+  String dbFilterManyJoin() {
+    return filterManyJoin ? dbFilterMany : null;
   }
 
   /**

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

@@ -22,6 +22,7 @@ final class DefaultDbSqlContext implements DbSqlContext {
   private final ArrayStack<String> joinStack = new ArrayStack<>();
   private final ArrayStack<String> prefixStack = new ArrayStack<>();
   private final String fromForUpdate;
+  private final String dbFilterManyJoin;
   private boolean useColumnAlias;
   private int columnIndex;
   private int asOfTableCount;
@@ -41,14 +42,22 @@ final class DefaultDbSqlContext implements DbSqlContext {
   private boolean joinSuppressed;
 
   DefaultDbSqlContext(SqlTreeAlias alias, String columnAliasPrefix, CQueryHistorySupport historySupport,
-                      CQueryDraftSupport draftSupport, String fromForUpdate) {
+                      CQueryDraftSupport draftSupport, String fromForUpdate, String dbFilterManyJoin) {
     this.alias = alias;
     this.columnAliasPrefix = columnAliasPrefix;
     this.useColumnAlias = columnAliasPrefix != null;
     this.draftSupport = draftSupport;
     this.historySupport = historySupport;
     this.historyQuery = (historySupport != null);
     this.fromForUpdate = fromForUpdate;
+    this.dbFilterManyJoin = dbFilterManyJoin;
+  }
+
+  @Override
+  public void includeFilterMany() {
+    if (dbFilterManyJoin != null) {
+      sb.append(" and ").append(dbFilterManyJoin);
+    }
   }
 
   @Override

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

@@ -107,7 +107,7 @@ public final class SqlTreeBuilder {
     CQueryHistorySupport historySupport = builder.historySupport(query);
     CQueryDraftSupport draftSupport = builder.draftSupport(query);
     String colAlias = subQuery || rootNode.isSingleProperty() ? null : columnAliasPrefix;
-    this.ctx = new DefaultDbSqlContext(alias, colAlias, historySupport, draftSupport, fromForUpdate);
+    this.ctx = new DefaultDbSqlContext(alias, colAlias, historySupport, draftSupport, fromForUpdate, predicates.dbFilterManyJoin());
   }
 
   /**

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

@@ -45,5 +45,6 @@ protected void appendExtraWhere(DbSqlContext ctx) {
   @Override
   public void appendFrom(DbSqlContext ctx, SqlJoinType joinType) {
     super.appendFrom(ctx, joinType.autoToOuter());
+    ctx.includeFilterMany();
   }
 }

ebean-core/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java

@@ -171,8 +171,9 @@ public <T> SpiExpressionList<T> filterMany(Query<T> rootQuery) {
     if (filterMany == null) {
       FilterExprPath exprPath = new FilterExprPath(path);
       SpiExpressionFactory queryEf = (SpiExpressionFactory) rootQuery.getExpressionFactory();
-      ExpressionFactory filterEf = queryEf.createExpressionFactory();// exprPath);
+      ExpressionFactory filterEf = queryEf.createExpressionFactory();
       filterMany = new FilterExpressionList(exprPath, filterEf, rootQuery);
+      // assuming conditions supported in JOIN, not setting markForQueryJoin = true
     }
     return filterMany;
   }
@@ -187,6 +188,13 @@ private SpiExpressionList<?> getFilterManyTrimPath(int trimPath) {
     return filterMany.trimPath(trimPath);
   }
 
+  /**
+   * Return true if there is a filterMany and it should be included in the JOIN.
+   */
+  public boolean isFilterManyJoin() {
+    return filterMany != null && !markForQueryJoin;
+  }
+
   /**
    * Adjust filterMany expressions for inclusion in main query.
    */

ebean-querybean/src/test/java/org/querytest/QCustomerTest.java

@@ -328,7 +328,7 @@ void filterMany() {
       .query();
 
     q.findList();
-    assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterMany */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where (t1.id is null or (t1.first_name like ? escape'|' and t1.email is not null)) order by t0.id");
+    assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterMany */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id and t1.first_name like ? escape'|' and t1.email is not null order by t0.id");
   }
 
   @Test
@@ -340,7 +340,7 @@ void filterManySingle() {
       .query();
 
     q.findList();
-    assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterManySingle */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where (t1.id is null or (t1.first_name like ? escape'|')) order by t0.id");
+    assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterManySingle */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id and t1.first_name like ? escape'|' order by t0.id");
   }
 
   @Test
@@ -375,7 +375,7 @@ void filterManySingleQuery() {
       .query();
 
     q.findList();
-    assertThat(q.getGeneratedSql()).contains(" from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where (t1.id is null or (t1.first_name like ? and t1.first_name like ? escape'|')) order by t0.id");
+    assertThat(q.getGeneratedSql()).contains(" from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id and t1.first_name like ? and t1.first_name like ? escape'|' order by t0.id");
   }
 
   @Test
@@ -389,7 +389,7 @@ void filterManyOr() {
       .query();
 
     q.findList();
-    assertThat(q.getGeneratedSql()).contains(" from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where (t1.id is null or ((t1.first_name like ? escape'|' or t1.last_name like ? escape'|'))) order by t0.id");
+    assertThat(q.getGeneratedSql()).contains(" from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id and (t1.first_name like ? escape'|' or t1.last_name like ? escape'|') order by t0.id");
   }
 
   @Test
@@ -406,7 +406,7 @@ void filterManyBeforeSelectFetchGroup_expect_filterManyExpressionRetained() {
       .query();
 
     q.findList();
-    assertThat(q.getGeneratedSql()).contains(" t0.id, t0.name, t1.id, t1.email from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where (t1.id is null or (t1.first_name like ? escape'|')) order by t0.id");
+    assertThat(q.getGeneratedSql()).contains(" t0.id, t0.name, t1.id, t1.email from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id and t1.first_name like ? escape'|' order by t0.id");
   }
 
   @Test

ebean-querybean/src/test/java/org/querytest/QDataWithFormulaMainTest.java

@@ -22,7 +22,7 @@ void testFilterMany() {
 
     List<String> sql = LoggedSql.stop();
     assertThat(sql).hasSize(1);
-    assertThat(sql.get(0)).contains(" where ((t1.main_id is null and t1.meta_key is null and t1.value_index is null) or (t1.meta_key = ?)) order by t0.id");
+    assertThat(sql.get(0)).contains(" from data_with_formula_main t0 left join data_with_formula t1 on t1.main_id = t0.id and t1.meta_key = ? order by t0.id");
   }
 
   @Test
@@ -36,6 +36,6 @@ void testFilterManyComposite() {
 
     List<String> sql = LoggedSql.stop();
     assertThat(sql).hasSize(1);
-    assertThat(sql.get(0)).contains(" where ((t1.main_id is null and t1.meta_key is null and t1.value_index is null) or (t1.meta_key = ?)) order by t0.id");
+    assertThat(sql.get(0)).contains(" left join data_with_formula t1 on t1.main_id = t0.id and t1.meta_key = ? order by t0.id");
   }
 }

ebean-test/src/test/java/org/tests/model/basic/Building.java

@@ -0,0 +1,27 @@
+package org.tests.model.basic;
+
+import io.ebean.Model;
+import jakarta.persistence.*;
+
+@Entity
+public class Building extends Model {
+  public static final String CAFE = "cafe";
+  public static final String HOUSE = "house";
+  public static final String STORE = "store";
+
+  @Id
+  public int id;
+  @Column(nullable = false)
+  public String type;
+  @Column(name = "lvl")
+  public int level;
+  public final String name;
+  @ManyToOne(optional = false)
+  public final Clan clan;
+
+  public Building(Clan clan, String type, String name) {
+    this.clan = clan;
+    this.type = type;
+    this.name = name;
+  }
+}

ebean-test/src/test/java/org/tests/model/basic/Clan.java

@@ -0,0 +1,17 @@
+package org.tests.model.basic;
+
+import io.ebean.Model;
+import jakarta.persistence.*;
+
+import java.util.ArrayList;
+import java.util.List;
+
+@Entity
+public class Clan extends Model {
+
+  @Id
+  public int id;
+
+  @OneToMany(cascade = CascadeType.ALL)
+  public List<Building> buildings = new ArrayList<>();
+}

ebean-test/src/test/java/org/tests/model/basic/ClanQuest.java

@@ -0,0 +1,18 @@
+package org.tests.model.basic;
+
+import io.ebean.Model;
+import jakarta.persistence.*;
+
+@Entity
+public class ClanQuest extends Model {
+
+  @Id
+  public int id;
+
+  @ManyToOne(optional = false)
+  public final Clan clan;
+
+  public ClanQuest(Clan clan) {
+    this.clan = clan;
+  }
+}