#1795 - Refactor bind padding for Id IN expressions (based on internal multiValueSupport and simple id)

src/main/java/io/ebean/event/BeanQueryRequest.java

@@ -24,6 +24,11 @@ public interface BeanQueryRequest<T> {
    */
   Query<T> getQuery();
 
+  /**
+   * Return true if an Id IN expression should have the bind parameters padded.
+   */
+  boolean isPadInExpression();
+
   /**
    * Return true if multi-value binding using Array or Table Values is supported.
    */

src/main/java/io/ebeaninternal/api/LoadBeanRequest.java

@@ -4,8 +4,6 @@ import io.ebean.bean.EntityBean;
 import io.ebean.bean.EntityBeanIntercept;
 import io.ebeaninternal.server.core.OrmQueryRequest;
 import io.ebeaninternal.server.deploy.BeanDescriptor;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -17,8 +15,6 @@ import java.util.Set;
  */
 public class LoadBeanRequest extends LoadRequest {
 
-  private static final Logger logger = LoggerFactory.getLogger(LoadBeanRequest.class);
-
   private final List<EntityBeanIntercept> batch;
 
   private final LoadBeanBuffer loadBuffer;
@@ -56,7 +52,7 @@ public class LoadBeanRequest extends LoadRequest {
     return loadBuffer.getBeanDescriptor().getBeanType();
   }
 
-  public boolean isLoadCache() {
+  private boolean isLoadCache() {
     return loadCache;
   }
 
@@ -75,17 +71,10 @@ public class LoadBeanRequest extends LoadRequest {
   /**
    * Return the load context.
    */
-  public LoadBeanBuffer getLoadContext() {
+  private LoadBeanBuffer getLoadContext() {
     return loadBuffer;
   }
 
-  /**
-   * Return the property that invoked the lazy loading.
-   */
-  public String getLazyLoadProperty() {
-    return lazyLoadProperty;
-  }
-
   public int getBatchSize() {
     return getLoadContext().getBatchSize();
   }
@@ -93,30 +82,14 @@ public class LoadBeanRequest extends LoadRequest {
   /**
    * Return the list of Id values for the beans in the lazy load buffer.
    */
-  public List<Object> getIdList(int batchSize) {
+  public List<Object> getIdList() {
 
-    List<Object> idList = new ArrayList<>(batchSize);
+    List<Object> idList = new ArrayList<>();
 
     BeanDescriptor<?> desc = loadBuffer.getBeanDescriptor();
     for (EntityBeanIntercept ebi : batch) {
-      EntityBean bean = ebi.getOwner();
-      idList.add(desc.getId(bean));
+      idList.add(desc.getId(ebi.getOwner()));
     }
-
-
-    if (!desc.isMultiValueIdSupported() && !idList.isEmpty()) {
-      int extraIds = batchSize - batch.size();
-      if (extraIds > 0) {
-        // for performance make up the Id's to the batch size
-        // so we get the same query (for Ebean and the db)
-        Object firstId = idList.get(0);
-        for (int i = 0; i < extraIds; i++) {
-          // just add the first Id again
-          idList.add(firstId);
-        }
-      }
-    }
-
     return idList;
   }
 

src/main/java/io/ebeaninternal/api/LoadManyRequest.java

@@ -3,6 +3,7 @@ package io.ebeaninternal.api;
 import io.ebean.bean.BeanCollection;
 import io.ebean.bean.EntityBean;
 import io.ebean.util.StringHelper;
+import io.ebeaninternal.server.core.BindPadding;
 import io.ebeaninternal.server.core.OrmQueryRequest;
 import io.ebeaninternal.server.deploy.BeanDescriptor;
 import io.ebeaninternal.server.deploy.BeanPropertyAssocMany;
@@ -65,13 +66,6 @@ public class LoadManyRequest extends LoadRequest {
     return batch;
   }
 
-  /**
-   * Return the load context.
-   */
-  public LoadManyBuffer getLoadContext() {
-    return loadContext;
-  }
-
   /**
    * Return true if lazy loading should only load the id values.
    * <p>
@@ -80,14 +74,14 @@ public class LoadManyRequest extends LoadRequest {
    * used.
    * </p>
    */
-  public boolean isOnlyIds() {
+  private boolean isOnlyIds() {
     return onlyIds;
   }
 
   /**
    * Return true if we should load the Collection ids into the cache.
    */
-  public boolean isLoadCache() {
+  private boolean isLoadCache() {
     return loadCache;
   }
 
@@ -98,22 +92,16 @@ public class LoadManyRequest extends LoadRequest {
     return loadContext.getBatchSize();
   }
 
-  private List<Object> getParentIdList(int batchSize) {
+  private List<Object> getParentIdList() {
 
-    ArrayList<Object> idList = new ArrayList<>(batchSize);
+    List<Object> idList = new ArrayList<>();
 
     BeanPropertyAssocMany<?> many = getMany();
     for (BeanCollection<?> bc : batch) {
       idList.add(many.getParentId(bc.getOwnerBean()));
     }
-    if (!many.getTargetDescriptor().isMultiValueIdSupported()) {
-      int extraIds = batchSize - batch.size();
-      if (extraIds > 0) {
-        Object firstId = idList.get(0);
-        for (int i = 0; i < extraIds; i++) {
-          idList.add(firstId);
-        }
-      }
+    if (many.getTargetDescriptor().isPadInExpression()) {
+      BindPadding.padIds(idList);
     }
 
     return idList;
@@ -123,7 +111,7 @@ public class LoadManyRequest extends LoadRequest {
     return loadContext.getBeanProperty();
   }
 
-  public SpiQuery<?> createQuery(SpiEbeanServer server, int batchSize) {
+  public SpiQuery<?> createQuery(SpiEbeanServer server) {
 
     BeanPropertyAssocMany<?> many = getMany();
 
@@ -142,10 +130,7 @@ public class LoadManyRequest extends LoadRequest {
     }
 
     query.setLazyLoadForParents(many);
-
-    List<Object> idList = getParentIdList(batchSize);
-    many.addWhereParentIdIn(query, idList, loadContext.isUseDocStore());
-
+    many.addWhereParentIdIn(query, getParentIdList(), loadContext.isUseDocStore());
     query.setPersistenceContext(loadContext.getPersistenceContext());
 
     String mode = isLazy() ? "+lazy" : "+query";

src/main/java/io/ebeaninternal/server/core/BindPadding.java

@@ -0,0 +1,57 @@
+package io.ebeaninternal.server.core;
+
+import java.util.List;
+
+/**
+ * Supports padding bind parameters for IN expressions.
+ * <p>
+ * We do this in order to get better hit ratio on DB query plans.
+ * </p>
+ */
+public final class BindPadding {
+
+  /**
+   * Pad out the Ids into common bucket sizes.
+   *
+   * @param idCollection The collection of Ids values being bound.
+   */
+  public static void padIds(List<Object> idCollection) {
+
+    int extraIds = padding(idCollection.size());
+    if (extraIds > 0) {
+      // for performance make up the Id's to the batch size
+      // so we get the same query (for Ebean and the db)
+      Object firstId = idCollection.get(0);
+      for (int i = 0; i < extraIds; i++) {
+        // just add the first Id again
+        idCollection.add(firstId);
+      }
+    }
+  }
+
+  /**
+   * Extra padding on binding id's in order to get better hit ratio on DB prepared statements / query plans.
+   */
+  static int padding(int size) {
+    if (size == 1) {
+      return 0;
+    }
+    if (size <= 5) {
+      return 5 - size;
+    }
+    if (size <= 10) {
+      return 10 - size;
+    }
+    if (size <= 20) {
+      return 20 - size;
+    }
+    if (size <= 40) {
+      return 40 - size;
+    }
+    if (size <= 50) {
+      return 50 - size;
+    }
+    return size <= 100 ? 100 - size : 0;
+  }
+
+}

src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java

@@ -38,54 +38,14 @@ class DefaultBeanLoader {
     this.onIterateUseExtraTxn = server.getDatabasePlatform().useExtraTransactionOnIterateSecondaryQueries();
   }
 
-  /**
-   * Return a batch size that might be less than the requestedBatchSize.
-   * <p>
-   * This means we can have large and variable requestedBatchSizes.
-   * </p>
-   * <p>
-   * We want to restrict the number of different batch sizes as we want to
-   * re-use the query plan cache and get DB statement re-use.
-   * </p>
-   */
-  private int getBatchSize(int batchSize) {
-
-    if (batchSize == 1) {
-      // there is only one bean/collection to load
-      return 1;
-    }
-    if (batchSize <= 5) {
-      // anything less than 5 becomes 5
-      return 5;
-    }
-    if (batchSize <= 10) {
-      return 10;
-    }
-    if (batchSize <= 20) {
-      return 20;
-    }
-    if (batchSize <= 50) {
-      return 50;
-    }
-    if (batchSize <= 100) {
-      return 100;
-    }
-    return batchSize;
-  }
-
   void refreshMany(EntityBean parentBean, String propertyName) {
     refreshMany(parentBean, propertyName, null);
   }
 
   void loadMany(LoadManyRequest loadRequest) {
 
-    List<BeanCollection<?>> batch = loadRequest.getBatch();
-
-    int batchSize = getBatchSize(batch.size());
-
-    SpiQuery<?> query = loadRequest.createQuery(server, batchSize);
+    SpiQuery<?> query = loadRequest.createQuery(server);
     executeQuery(loadRequest, query);
-
     loadRequest.postLoad();
   }
 
@@ -97,7 +57,7 @@ class DefaultBeanLoader {
     loadManyInternal(parentBean, propertyName, null, false, onlyIds);
   }
 
-  void refreshMany(EntityBean parentBean, String propertyName, Transaction t) {
+  private void refreshMany(EntityBean parentBean, String propertyName, Transaction t) {
     loadManyInternal(parentBean, propertyName, t, true, false);
   }
 
@@ -147,8 +107,7 @@ class DefaultBeanLoader {
       query.setLoadDescription("+lazy", null);
     }
 
-    String idProperty = parentDesc.getIdBinder().getIdProperty();
-    query.select(idProperty);
+    query.select(parentDesc.getIdBinder().getIdProperty());
 
     if (onlyIds) {
       query.fetch(many.getName(), many.getTargetIdProperty());
@@ -192,9 +151,7 @@ class DefaultBeanLoader {
       throw new RuntimeException("Nothing in batch?");
     }
 
-    int batchSize = getBatchSize(batch.size());
-
-    List<Object> idList = loadRequest.getIdList(batchSize);
+    List<Object> idList = loadRequest.getIdList();
     if (idList.isEmpty()) {
       // everything was loaded from cache
       return;
@@ -315,6 +272,5 @@ class DefaultBeanLoader {
     }
 
     desc.resetManyProperties(dbBean);
-
   }
 }

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

@@ -115,6 +115,11 @@ public final class OrmQueryRequest<T> extends BeanRequest implements SpiOrmQuery
     }
   }
 
+  @Override
+  public boolean isPadInExpression() {
+    return beanDescriptor.isPadInExpression();
+  }
+
   @Override
   public boolean isMultiValueIdSupported() {
     return beanDescriptor.isMultiValueIdSupported();

src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java

@@ -1835,6 +1835,13 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType {
     return multiValueSupported && isSimpleId();
   }
 
+  /**
+   * Return true if Id IN expression should have bind parameters padded.
+   */
+  public boolean isPadInExpression() {
+    return !multiValueSupported && isSimpleId();
+  }
+
   /**
    * Return the sql for binding an id. This is the columns with table alias that
    * make up the id.

src/main/java/io/ebeaninternal/server/expression/IdInExpression.java

@@ -5,6 +5,7 @@ import io.ebeaninternal.api.ManyWhereJoins;
 import io.ebeaninternal.api.SpiExpression;
 import io.ebeaninternal.api.SpiExpressionRequest;
 import io.ebeaninternal.api.SpiExpressionValidation;
+import io.ebeaninternal.server.core.BindPadding;
 import io.ebeaninternal.server.deploy.BeanDescriptor;
 import io.ebeaninternal.server.deploy.id.IdBinder;
 
@@ -46,6 +47,10 @@ public class IdInExpression extends NonPrepareExpression {
   @Override
   public void prepareExpression(BeanQueryRequest<?> request) {
     multiValueIdSupported = request.isMultiValueIdSupported();
+    if (!multiValueIdSupported && !idCollection.isEmpty() && request.isPadInExpression()) {
+      // pad out the ids for better hit ratio on DB query plans
+      BindPadding.padIds(idCollection);
+    }
   }
 
   @Override

src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java

@@ -1,5 +1,6 @@
 package io.ebeaninternal.server.loadcontext;
 
+import io.ebean.CacheMode;
 import io.ebean.bean.BeanLoader;
 import io.ebean.bean.EntityBeanIntercept;
 import io.ebean.bean.PersistenceContext;
@@ -20,6 +21,8 @@ import java.util.Set;
  */
 class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext {
 
+  private final boolean cache;
+
   private List<LoadBuffer> bufferList;
 
   private LoadBuffer currentBuffer;
@@ -29,6 +32,7 @@ class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext {
     // bufferList only required when using query joins (queryFetch)
     this.bufferList = (!queryFetch) ? null : new ArrayList<>();
     this.currentBuffer = createBuffer(firstBatchSize);
+    this.cache = (queryProps == null) ? false : queryProps.isCache();
   }
 
   /**
@@ -43,6 +47,9 @@ class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext {
 
   private void configureQuery(SpiQuery<?> query, String lazyLoadProperty) {
 
+    if (cache) {
+      query.setBeanCacheMode(CacheMode.ON);
+    }
     setLabel(query);
     parent.propagateQueryState(query, desc.isDocStoreMapped());
     query.setParentNode(objectGraphNode);

src/test/java/io/ebeaninternal/server/core/BindPaddingTest.java

@@ -0,0 +1,51 @@
+package io.ebeaninternal.server.core;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+
+public class BindPaddingTest {
+
+  @Test
+  public void padIds() {
+
+    final List<Object> input = asList(1, 2);
+    BindPadding.padIds(input);
+    assertThat(input).contains(1,2,1,1,1);
+    assertThat(input).hasSize(5);
+  }
+
+  private List<Object> asList(int... id) {
+    List<Object> list = new ArrayList<>();
+    for (int i : id) {
+      list.add(i);
+    }
+    return list;
+  }
+
+  @Test
+  public void padding() {
+
+    assertEquals(0, BindPadding.padding(1));
+    assertEquals(3, BindPadding.padding(2));
+    assertEquals(2, BindPadding.padding(3));
+    assertEquals(1, BindPadding.padding(4));
+    assertEquals(0, BindPadding.padding(5));
+    assertEquals(4, BindPadding.padding(6));
+    assertEquals(1, BindPadding.padding(9));
+    assertEquals(0, BindPadding.padding(10));
+    assertEquals(9, BindPadding.padding(11));
+    assertEquals(0, BindPadding.padding(20));
+    assertEquals(19, BindPadding.padding(21));
+    assertEquals(0, BindPadding.padding(40));
+    assertEquals(9, BindPadding.padding(41));
+    assertEquals(0, BindPadding.padding(50));
+    assertEquals(49, BindPadding.padding(51));
+    assertEquals(0, BindPadding.padding(100));
+    assertEquals(0, BindPadding.padding(101));
+  }
+}

src/test/java/io/ebeaninternal/server/expression/BaseExpressionTest.java

@@ -20,18 +20,18 @@ public abstract class BaseExpressionTest extends BaseTestCase {
   }
 
   protected String hash(SpiExpression expression) {
-    StringBuilder sb  = new StringBuilder();
+    StringBuilder sb = new StringBuilder();
     if (expression != null) {
       expression.queryPlanHash(sb);
     }
     return sb.toString();
   }
 
-  protected void same(SpiExpression one, SpiExpression two){
+  protected void same(SpiExpression one, SpiExpression two) {
     assertThat(hash(one)).isEqualTo(hash(two));
   }
 
-  protected void different(SpiExpression one, SpiExpression two){
+  protected void different(SpiExpression one, SpiExpression two) {
     assertThat(hash(one)).isNotEqualTo(hash(two));
   }
 
@@ -50,7 +50,7 @@ public abstract class BaseExpressionTest extends BaseTestCase {
   }
 
 
-  private static final TDQueryRequest<Customer> MULTI_VALUE= new TDQueryRequest<>(true);
+  private static final TDQueryRequest<Customer> MULTI_VALUE = new TDQueryRequest<>(true);
   private static final TDQueryRequest<Customer> NO_MULTI_VALUE = new TDQueryRequest<>(false);
 
   static class TDQueryRequest<T> implements BeanQueryRequest<T> {
@@ -76,6 +76,11 @@ public abstract class BaseExpressionTest extends BaseTestCase {
       return null;
     }
 
+    @Override
+    public boolean isPadInExpression() {
+      return supported;
+    }
+
     @Override
     public boolean isMultiValueIdSupported() {
       return supported;

src/test/java/org/tests/cache/TestBeanCache.java

@@ -76,7 +76,7 @@ public class TestBeanCache extends BaseTestCase {
     List<String> sql = LoggedSqlCollector.current();
     assertThat(sql).hasSize(1);
     if (isH2()) {
-      assertThat(sql.get(0)).contains("from o_cached_bean t0 where t0.id in (?,?,?)");
+      assertThat(sql.get(0)).contains("from o_cached_bean t0 where t0.id in (?,?,?,?,?)");
     }
 
     log.info("All hits (3 of 3) ...");
@@ -124,7 +124,7 @@ public class TestBeanCache extends BaseTestCase {
     assertThat(sql).hasSize(1);
     if (isH2()) {
       // fetch the misses from DB
-      assertThat(sql.get(0)).contains("from o_cached_bean t0 where t0.id in (?,?)");
+      assertThat(sql.get(0)).contains("from o_cached_bean t0 where t0.id in (?,?,?,?,?)");
     }
   }
 

src/test/java/org/tests/cache/TestBeanFetchJoinCache.java

@@ -0,0 +1,45 @@
+package org.tests.cache;
+
+import io.ebean.BaseTestCase;
+import io.ebean.DB;
+import org.junit.Test;
+import org.tests.model.basic.Customer;
+import org.tests.model.basic.Order;
+import org.tests.model.basic.ResetBasicData;
+
+import java.util.List;
+
+import static io.ebean.CacheMode.ON;
+
+public class TestBeanFetchJoinCache extends BaseTestCase {
+
+  @Test
+  public void test() {
+
+    ResetBasicData.reset();
+
+//    DB.find(Customer.class)
+//      .setBeanCacheMode(ON)
+//      .findList();
+
+
+    DB.find(Customer.class)
+      .where().idIn(1,2,3)
+      //.setBeanCacheMode(ON)
+      .findList();
+
+    List<Order> orders = DB.find(Order.class)
+      .fetchQuery("customer", "name")
+      .findList();
+
+
+    orders = DB.find(Order.class)
+      .fetchQuery("customer", "+cache, name")
+      .findList();
+
+    for (Order order : orders) {
+      order.getCustomer().getName();
+    }
+
+  }
+}

src/test/resources/logback-test.xml

@@ -80,10 +80,10 @@
 <!--  <logger name="io.ebean.DDL" level="DEBUG"/>-->
 
   <logger name="io.ebean.SQL" level="TRACE"/>
-<!--  <logger name="io.ebean.TXN" level="TRACE"/>-->
-<!--  <logger name="io.ebean.SUM" level="TRACE"/>-->
+  <logger name="io.ebean.TXN" level="TRACE"/>
+  <logger name="io.ebean.SUM" level="TRACE"/>
 
-<!--  <logger name="io.ebean.cache" level="TRACE"/>-->
+  <logger name="io.ebean.cache" level="TRACE"/>
 <!--  <logger name="io.ebean.cache.REGION" level="TRACE"/>-->
 <!--  <logger name="io.ebean.cache.TABLEMOD" level="TRACE"/>-->