#3466 - Duplicated entities in findMany when a ManyToOne relation is …

…fetched (Postgresql)

Internally in OrmQueryDetail.markQueryJoins() the SpiQueryManyJoin is determined and returned. This change uses this and effectively removes the code from OrmQueryRequest.determineMany() and BeanDescriptor.manyProperty() which was the source of this issue (performing a similar task but not correctly taking the full path into account and hence the source of this bug).

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

@@ -580,7 +580,12 @@ public static TemporalMode of(SpiQuery<?> query) {
   /**
    * Convert joins as necessary to query joins etc.
    */
-  SpiQuerySecondary convertJoins();
+  SpiQueryManyJoin convertJoins();
+
+  /**
+   * Return secondary queries if required.
+   */
+  SpiQuerySecondary secondaryQuery();
 
   /**
    * Return the TransactionContext.

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

@@ -0,0 +1,17 @@
+package io.ebeaninternal.api;
+
+/**
+ * A SQL Join for a ToMany property included in the query.
+ */
+public interface SpiQueryManyJoin {
+
+  /**
+   * The full path of the many property to include in the query via join.
+   */
+  String path();
+
+  /**
+   * Order by clause defined via mapping on the ToMany property.
+   */
+  String fetchOrderBy();
+}

ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java

@@ -41,9 +41,9 @@ public final class OrmQueryRequest<T> extends BeanRequest implements SpiOrmQuery
   private CQueryPlanKey queryPlanKey;
   private SpiQuerySecondary secondaryQueries;
   private List<T> cacheBeans;
-  private BeanPropertyAssocMany<?> manyProperty;
   private boolean inlineCountDistinct;
   private Set<String> dependentTables;
+  private SpiQueryManyJoin manyJoin;
 
   public OrmQueryRequest(SpiEbeanServer server, OrmQueryEngine queryEngine, SpiQuery<T> query, SpiTransaction t) {
     super(server, t);
@@ -168,7 +168,8 @@ private void adapterPreQuery() {
    */
   @Override
   public void prepareQuery() {
-    secondaryQueries = query.convertJoins();
+    manyJoin = query.convertJoins();
+    secondaryQueries = query.secondaryQuery();
     beanDescriptor.prepareQuery(query);
     adapterPreQuery();
     queryPlanKey = query.prepare(this);
@@ -446,19 +447,19 @@ public SpiQuery<T> query() {
     return query;
   }
 
-  /**
-   * Determine and return the ToMany property that is included in the query.
-   */
-  public BeanPropertyAssocMany<?> determineMany() {
-    manyProperty = beanDescriptor.manyProperty(query);
-    return manyProperty;
+  public SpiQueryManyJoin manyJoin() {
+    return manyJoin;
   }
 
   /**
-   * Return the many property that is fetched in the query or null if there is not one.
+   * Return true if there is a manyJoin that should be included with this query.
    */
-  public BeanPropertyAssocMany<?> manyPropertyForOrderBy() {
-    return query.isSingleAttribute() ? null : manyProperty;
+  public boolean includeManyJoin() {
+    if (manyJoin == null || query.isSingleAttribute()) {
+      return false;
+    }
+    final Type type = query.type();
+    return type != Type.SQ_EX && type != Type.COUNT;
   }
 
   /**

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

@@ -1601,19 +1601,6 @@ private BeanProperty findWhenModifiedProperty() {
     return null;
   }
 
-  /**
-   * Return the many property included in the query or null if one is not.
-   */
-  public BeanPropertyAssocMany<?> manyProperty(SpiQuery<?> query) {
-    OrmQueryDetail detail = query.detail();
-    for (BeanPropertyAssocMany<?> many : propertiesMany) {
-      if (detail.includesPath(many.name())) {
-        return many;
-      }
-    }
-    return null;
-  }
-
   /**
    * Return a raw expression for 'where parent id in ...' clause.
    */

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

@@ -1,5 +1,6 @@
 package io.ebeaninternal.server.el;
 
+import io.ebeaninternal.api.SpiQueryManyJoin;
 import io.ebeaninternal.server.deploy.BeanProperty;
 
 /**
@@ -10,7 +11,7 @@
  * joins and set place holders for table alias'.
  * </p>
  */
-public interface ElPropertyDeploy {
+public interface ElPropertyDeploy extends SpiQueryManyJoin {
 
   /**
    * This is the elPrefix for all root level properties.
@@ -80,4 +81,14 @@ public interface ElPropertyDeploy {
    * is left as a 'join' and which get converted to query join.
    */
   int fetchPreference();
+
+  @Override
+  default String path() {
+    return elName();
+  }
+
+  @Override
+  default String fetchOrderBy() {
+    return beanProperty().fetchOrderBy();
+  }
 }

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

@@ -1,13 +1,9 @@
 package io.ebeaninternal.server.query;
 
 import io.ebean.OrderBy;
-import io.ebeaninternal.api.BindParams;
-import io.ebeaninternal.api.CoreLog;
-import io.ebeaninternal.api.SpiExpressionList;
-import io.ebeaninternal.api.SpiQuery;
+import io.ebeaninternal.api.*;
 import io.ebeaninternal.server.bind.DataBind;
 import io.ebeaninternal.server.core.OrmQueryRequest;
-import io.ebeaninternal.server.deploy.BeanPropertyAssocMany;
 import io.ebeaninternal.server.deploy.DeployParser;
 import io.ebeaninternal.server.expression.DefaultExpressionRequest;
 import io.ebeaninternal.server.persist.Binder;
@@ -175,9 +171,8 @@ public void prepare(boolean buildSql) {
     buildUpdateClause(buildSql, deployParser);
     buildBindWhereRawSql(buildSql);
 
-    BeanPropertyAssocMany<?> manyProperty = request.determineMany();
     if (buildSql) {
-      dbOrderBy = deriveOrderByWithMany(deployParser, request.manyPropertyForOrderBy());
+      dbOrderBy = deriveOrderByWithMany(deployParser);
       // create a copy of the includes required to support the orderBy
       orderByIncludes = new HashSet<>(deployParser.includes());
     }
@@ -188,13 +183,16 @@ public void prepare(boolean buildSql) {
         dbWhere = where.buildSql();
       }
     }
+    SpiQueryManyJoin manyProperty = request.manyJoin();
     if (manyProperty != null) {
-      OrmQueryProperties chunk = query.detail().getChunk(manyProperty.name(), false);
-      SpiExpressionList<?> filterManyExpr = chunk.getFilterMany();
-      if (filterManyExpr != null) {
-        this.filterMany = new DefaultExpressionRequest(request, deployParser, binder, filterManyExpr);
-        if (buildSql) {
-          dbFilterMany = filterMany.buildSql();
+      OrmQueryProperties chunk = query.detail().getChunk(manyProperty.path(), false);
+      if (chunk != null) {
+        SpiExpressionList<?> filterManyExpr = chunk.getFilterMany();
+        if (filterManyExpr != null) {
+          this.filterMany = new DefaultExpressionRequest(request, deployParser, binder, filterManyExpr);
+          if (buildSql) {
+            dbFilterMany = filterMany.buildSql();
+          }
         }
       }
     }
@@ -248,19 +246,20 @@ private String parseOrderBy(DeployParser parser) {
   /**
    * There is a many property we need to make sure the ordering is appropriate.
    */
-  private String deriveOrderByWithMany(DeployParser parser, BeanPropertyAssocMany<?> manyProp) {
+  private String deriveOrderByWithMany(DeployParser parser) {
     String orderBy = parseOrderBy(parser);
-    if (manyProp == null) {
+    if (!request.includeManyJoin()) {
       return orderBy;
     }
     String orderById = parser.parse(request.descriptor().defaultOrderBy());
     if (orderBy == null) {
       orderBy = orderById;
     }
     // check for default ordering on the many property...
+    SpiQueryManyJoin manyProp = request.manyJoin();
     String manyOrderBy = manyProp.fetchOrderBy();
     if (manyOrderBy != null) {
-      orderBy = orderBy + ", " + parser.parse(CQueryBuilder.prefixOrderByFields(manyProp.name(), manyOrderBy));
+      orderBy = orderBy + ", " + parser.parse(CQueryBuilder.prefixOrderByFields(manyProp.path(), manyOrderBy));
     }
     if (request.isFindById()) {
       // only one master bean so should be fine...
@@ -273,7 +272,7 @@ private String deriveOrderByWithMany(DeployParser parser, BeanPropertyAssocMany<
     if (idPos == 0) {
       return orderBy;
     }
-    int manyPos = orderBy.indexOf("${" + manyProp.name() + "}");
+    int manyPos = orderBy.indexOf("${" + manyProp.path() + "}");
     if (manyPos == -1) {
       // no ordering of the many
       if (idPos == -1) {
@@ -286,10 +285,10 @@ private String deriveOrderByWithMany(DeployParser parser, BeanPropertyAssocMany<
     if (idPos == -1 || idPos >= manyPos) {
       if (idPos > manyPos) {
         // there was an error with the order by...
-        String msg = "A Query on [" + request.descriptor() + "] includes a join to a 'many' association [" + manyProp.name()
-        + "] with an incorrect orderBy [" + orderBy + "]. The id property [" + orderById
-        + "] must come before the many property [" + manyProp.name() + "] in the orderBy."
-        + " Ebean has automatically modified the orderBy clause to do this.";
+        String msg = "A Query on [" + request.descriptor() + "] includes a join to a 'many' association [" + manyProp.path()
+          + "] with an incorrect orderBy [" + orderBy + "]. The id property [" + orderById
+          + "] must come before the many property [" + manyProp.path() + "] in the orderBy."
+          + " Ebean has automatically modified the orderBy clause to do this.";
         CoreLog.log.log(WARNING, msg);
       }
       // the id needs to come before the manyPropName

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

@@ -504,22 +504,26 @@ public final void setLazyLoadManyPath(String lazyLoadManyPath) {
   }
 
   @Override
-  public final SpiQuerySecondary convertJoins() {
+  public final SpiQueryManyJoin convertJoins() {
     if (!useDocStore) {
       createExtraJoinsToSupportManyWhereClause();
     }
-    markQueryJoins();
+    return markQueryJoins();
+  }
+
+  @Override
+  public SpiQuerySecondary secondaryQuery() {
     return new OrmQuerySecondary(removeQueryJoins(), removeLazyJoins());
   }
 
   /**
    * Limit the number of fetch joins to Many properties, mark as query joins as needed.
+   *
+   * @return The query join many property or null.
    */
-  private void markQueryJoins() {
-    if (distinctOn == null) {
-      // no automatic join to query join conversion when distinctOn is used
-      detail.markQueryJoins(beanDescriptor, lazyLoadManyPath, isAllowOneManyFetch(), type.defaultSelect());
-    }
+  private SpiQueryManyJoin markQueryJoins() {
+    // no automatic join to query join conversion when distinctOn is used
+    return distinctOn != null ? null : detail.markQueryJoins(beanDescriptor, lazyLoadManyPath, isAllowOneManyFetch(), type.defaultSelect());
   }
 
   private boolean isAllowOneManyFetch() {

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

@@ -3,6 +3,7 @@
 import io.ebean.FetchConfig;
 import io.ebean.event.BeanQueryRequest;
 import io.ebean.util.SplitName;
+import io.ebeaninternal.api.SpiQueryManyJoin;
 import io.ebeaninternal.server.deploy.BeanDescriptor;
 import io.ebeaninternal.server.deploy.BeanPropertyAssoc;
 import io.ebeaninternal.server.el.ElPropertyDeploy;
@@ -327,12 +328,15 @@ private void sortFetchPaths(BeanDescriptor<?> d, OrmQueryProperties p, LinkedHas
 
   /**
    * Mark 'fetch joins' to 'many' properties over to 'query joins' where needed.
+   *
+   * @return The fetch join many property or null
    */
-  void markQueryJoins(BeanDescriptor<?> beanDescriptor, String lazyLoadManyPath, boolean allowOne, boolean addIds) {
+  SpiQueryManyJoin markQueryJoins(BeanDescriptor<?> beanDescriptor, String lazyLoadManyPath, boolean allowOne, boolean addIds) {
     if (fetchPaths.isEmpty()) {
-      return;
+      return null;
     }
 
+    ElPropertyDeploy many = null;
     // the name of the many fetch property if there is one
     String manyFetchProperty = null;
     // flag that is set once the many fetch property is chosen
@@ -353,13 +357,15 @@ void markQueryJoins(BeanDescriptor<?> beanDescriptor, String lazyLoadManyPath, b
             fetchJoinFirstMany = false;
             manyFetchProperty = pair.getPath();
             chunk.filterManyInline();
+            many = elProp;
           } else {
             // convert this one over to a 'query join'
             chunk.markForQueryJoin();
           }
         }
       }
     }
+    return many;
   }
 
   /**