Merge pull request #3396 from ebean-orm/feature/limitOffset-with-labe…

…lComment Include inline sql hint and comment in limit/offset sql
ebean-orm · Apr 30, 2024 · b99d4f5 · b99d4f5
2 parents e51d9ba + 7d18942
commit b99d4f5
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 72 deletions.
diff --git a/ebean-api/src/main/java/io/ebean/config/dbplatform/AnsiSqlRowsLimiter.java b/ebean-api/src/main/java/io/ebean/config/dbplatform/AnsiSqlRowsLimiter.java
@@ -4,22 +4,8 @@ public final class AnsiSqlRowsLimiter implements SqlLimiter {
 
   @Override
   public SqlLimitResponse limit(SqlLimitRequest request) {
-    String dbSql = request.getDbSql();
-    StringBuilder sb = new StringBuilder(50 + dbSql.length());
-    sb.append("select ");
-    if (request.isDistinct()) {
-      sb.append("distinct ");
-    }
-    sb.append(dbSql);
-    int firstRow = request.getFirstRow();
-    if (firstRow > 0) {
-      sb.append(" offset ").append(firstRow).append(" rows");
-    }
-    int maxRows = request.getMaxRows();
-    if (maxRows > 0) {
-      sb.append(" fetch next ").append(maxRows).append(" rows only");
-    }
-    String sql = request.getDbPlatform().completeSql(sb.toString(), request.getOrmQuery());
+    final var ansiSql = request.ansiOffsetRows();
+    final var sql = request.getDbPlatform().completeSql(ansiSql, request.getOrmQuery());
     return new SqlLimitResponse(sql);
   }
 }
diff --git a/ebean-api/src/main/java/io/ebean/config/dbplatform/LimitOffsetSqlLimiter.java b/ebean-api/src/main/java/io/ebean/config/dbplatform/LimitOffsetSqlLimiter.java
@@ -7,23 +7,16 @@ public final class LimitOffsetSqlLimiter implements SqlLimiter {
 
   @Override
   public SqlLimitResponse limit(SqlLimitRequest request) {
-    String dbSql = request.getDbSql();
-    StringBuilder sb = new StringBuilder(50 + dbSql.length());
-    sb.append("select ");
-    if (request.isDistinct()) {
-      sb.append("distinct ");
-    }
-    sb.append(dbSql);
+    final var buffer = request.selectDistinctOnSql();
     int maxRows = request.getMaxRows();
     if (maxRows > 0) {
-      sb.append(" limit ").append(maxRows);
+      buffer.append(" limit ").append(maxRows);
     }
     int firstRow = request.getFirstRow();
     if (firstRow > 0) {
-      sb.append(" offset ").append(firstRow);
+      buffer.append(" offset ").append(firstRow);
     }
-    String sql = request.getDbPlatform().completeSql(sb.toString(), request.getOrmQuery());
-    return new SqlLimitResponse(sql);
+    return new SqlLimitResponse(request.getDbPlatform().completeSql(buffer.toString(), request.getOrmQuery()));
   }
 
 }
diff --git a/ebean-api/src/main/java/io/ebean/config/dbplatform/SqlLimitRequest.java b/ebean-api/src/main/java/io/ebean/config/dbplatform/SqlLimitRequest.java
@@ -8,6 +8,21 @@
  */
 public interface SqlLimitRequest {
 
+  /**
+   * Return ANSI SQL Offset and next rows.
+   */
+  String ansiOffsetRows();
+
+  /**
+   * Create and return buffer including the select distinct clause.
+   */
+  StringBuilder selectDistinct();
+
+  /**
+   * Create and return buffer including the select distinct on clause.
+   */
+  StringBuilder selectDistinctOnSql();
+
   /**
    * Return true if the query uses distinct.
    */

diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java
@@ -533,7 +533,7 @@ private class BuildReq {
     private final boolean distinct;
     private final boolean countSingleAttribute;
     private final String dbOrderBy;
-    private boolean useSqlLimiter;
+    private final boolean useSqlLimiter;
     private boolean hasWhere;
 
     private BuildReq(String selectClause, OrmQueryRequest<?> request, CQueryPredicates predicates, SqlTree select) {
@@ -550,13 +550,15 @@ private BuildReq(String selectClause, OrmQueryRequest<?> request, CQueryPredicat
       this.distinct = query.isDistinct() || select.isSqlDistinct();
       this.dbOrderBy = predicates.dbOrderBy();
       this.countSingleAttribute = query.isCountDistinct() && query.isSingleAttribute();
+      this.useSqlLimiter = selectClause == null
+        && query.hasMaxRowsOrFirstRow()
+        && (select.manyProperty() == null || query.isSingleAttribute());
     }
 
     private void appendSelect() {
       if (selectClause != null) {
         sb.append(selectClause);
       } else {
-        useSqlLimiter = (query.hasMaxRowsOrFirstRow() && (select.manyProperty() == null || query.isSingleAttribute()));
         if (!useSqlLimiter) {
           appendSelectDistinct();
         }
@@ -733,7 +735,7 @@ private SqlLimitResponse buildSql() {
       }
       if (useSqlLimiter) {
         // use LIMIT/OFFSET, ROW_NUMBER() or rownum type SQL query limitation
-        SqlLimitRequest r = new OrmQueryLimitRequest(sb.toString(), dbOrderBy, query, dbPlatform, distinct);
+        var r = new OrmQueryLimitRequest(sb.toString(), dbOrderBy, query, dbPlatform, distinct, select.distinctOn(), hint(), inlineSqlComment());
         return sqlLimiter.limit(r);
       } else {
         if (updateStatement) {

diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryLimitRequest.java b/ebean-core/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryLimitRequest.java
@@ -11,13 +11,67 @@ public final class OrmQueryLimitRequest implements SqlLimitRequest {
   private final String sql;
   private final String sqlOrderBy;
   private final boolean distinct;
+  private final String distinctOn;
+  private final String hint;
+  private final String label;
 
   public OrmQueryLimitRequest(String sql, String sqlOrderBy, SpiQuery<?> ormQuery, DatabasePlatform dbPlatform, boolean distinct) {
+    this(sql, sqlOrderBy, ormQuery, dbPlatform, distinct, null, "", "");
+  }
+
+  public OrmQueryLimitRequest(String sql, String sqlOrderBy, SpiQuery<?> ormQuery, DatabasePlatform dbPlatform,
+                              boolean distinct, String distinctOn, String hint, String label) {
     this.sql = sql;
     this.sqlOrderBy = sqlOrderBy;
     this.ormQuery = ormQuery;
     this.dbPlatform = dbPlatform;
     this.distinct = distinct;
+    this.distinctOn = distinctOn;
+    this.hint = hint;
+    this.label  = label;
+  }
+
+  private StringBuilder newBuffer() {
+    return new StringBuilder(50 + sql.length());
+  }
+
+  @Override
+  public String ansiOffsetRows() {
+    final var buffer = selectDistinct();
+    buffer.append(sql);
+    int firstRow = getFirstRow();
+    if (firstRow > 0) {
+      buffer.append(" offset ").append(firstRow).append(" rows");
+    }
+    int maxRows = getMaxRows();
+    if (maxRows > 0) {
+      buffer.append(" fetch next ").append(maxRows).append(" rows only");
+    }
+    return buffer.toString();
+  }
+
+  @Override
+  public StringBuilder selectDistinct() {
+    final var buffer = newBuffer();
+    buffer.append("select ").append(hint).append(label);
+    if (distinct) {
+      buffer.append("distinct ");
+    }
+    return buffer;
+  }
+
+  @Override
+  public StringBuilder selectDistinctOnSql() {
+    var buffer = newBuffer();
+    buffer.append("select ").append(hint).append(label);
+    if (distinct) {
+      buffer.append("distinct ");
+      if (distinctOn != null) {
+        buffer.append("on (").append(distinctOn).append(") ");
+      }
+    }
+    buffer.append(sql);
+    return buffer;
   }
 
   @Override

diff --git a/ebean-querybean/src/test/java/org/querytest/QCustomerTest.java b/ebean-querybean/src/test/java/org/querytest/QCustomerTest.java
@@ -314,7 +314,7 @@ void filterManySeparateQuery() {
       .query();
 
     q.findList();
-    assertThat(q.getGeneratedSql()).isEqualTo("select t0.id, t0.name from be_customer t0 limit 10");
+    assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterManySeparateQuery */ t0.id, t0.name from be_customer t0 limit 10");
   }
 
   @Test

diff --git a/ebean-test/src/test/java/io/ebean/xtest/base/DtoQueryFromOrmTest.java b/ebean-test/src/test/java/io/ebean/xtest/base/DtoQueryFromOrmTest.java
@@ -222,8 +222,12 @@ public void asDto_withoutExplicitId() {
     LoggedSql.start();
 
     DtoQuery<ContactDto> query = DB.find(Contact.class)
-      .select("email, " + concat("lastName", ", ", "firstName") + " as fullName").where().isNotNull("email")
-      .isNotNull("lastName").order().asc("lastName").asDto(ContactDto.class);
+      .select("email, " + concat("lastName", ", ", "firstName") + " as fullName")
+      .where()
+      .isNotNull("email")
+      .isNotNull("lastName")
+      .order().asc("lastName")
+      .asDto(ContactDto.class);
 
     List<ContactDto> dtos = query.findList();
 
@@ -246,9 +250,15 @@ public void example() {
 
     LoggedSql.start();
 
-    List<ContactDto> contactDtos = DB.find(Contact.class).setLabel("emailFullName")
-      .select("email, " + concat("lastName", ", ", "firstName") + " as fullName").where().isNotNull("email")
-      .isNotNull("lastName").order().asc("lastName").setMaxRows(10).asDto(ContactDto.class).findList();
+    List<ContactDto> contactDtos = DB.find(Contact.class)
+      .setLabel("emailFullName")
+      .select("email, " + concat("lastName", ", ", "firstName") + " as fullName")
+      .where().isNotNull("email")
+      .isNotNull("lastName")
+      .orderBy().asc("lastName")
+      .setMaxRows(10)
+      .asDto(ContactDto.class)
+      .findList();
 
     assertThat(contactDtos).isNotEmpty();
 
@@ -260,11 +270,11 @@ public void example() {
     List<String> sql = LoggedSql.stop();
 
     if (isSqlServer()) {
-      assertSql(sql.get(0)).contains("select top 10 t0.email, " + concat("t0.last_name", ", ", "t0.first_name")
+      assertSql(sql.get(0)).contains("select /* emailFullName */ top 10 t0.email, " + concat("t0.last_name", ", ", "t0.first_name")
         + " fullName from contact t0 where t0.email is not null and t0.last_name is not null order by t0.last_name");
 
     } else {
-      assertSql(sql.get(0)).contains("select t0.email, " + concat("t0.last_name", ", ", "t0.first_name")
+      assertSql(sql.get(0)).contains("select /* emailFullName */ t0.email, " + concat("t0.last_name", ", ", "t0.first_name")
         + " fullName from contact t0 where t0.email is not null and t0.last_name is not null order by t0.last_name");
     }
   }

diff --git a/platforms/oracle/src/main/java/io/ebean/platform/oracle/OracleAnsiSqlRowsLimiter.java b/platforms/oracle/src/main/java/io/ebean/platform/oracle/OracleAnsiSqlRowsLimiter.java
@@ -11,23 +11,8 @@ final class OracleAnsiSqlRowsLimiter implements SqlLimiter {
 
   @Override
   public SqlLimitResponse limit(SqlLimitRequest request) {
-    String dbSql = request.getDbSql();
-    StringBuilder sb = new StringBuilder(50 + dbSql.length());
-    sb.append("select ");
-    if (request.isDistinct()) {
-      sb.append("distinct ");
-    }
-    sb.append(dbSql);
-    int firstRow = request.getFirstRow();
-    if (firstRow > 0) {
-      sb.append(" offset ").append(firstRow).append(" rows");
-    }
-    int maxRows = request.getMaxRows();
-    if (maxRows > 0) {
-      sb.append(" fetch next ").append(maxRows).append(" rows only");
-    }
     // Oracle does not support FOR UPDATE clause with limit offset
-    return new SqlLimitResponse(sb.toString());
+    return new SqlLimitResponse(request.ansiOffsetRows());
   }
 
 }
diff --git a/platforms/sqlserver/src/main/java/io/ebean/platform/sqlserver/SqlServerSqlLimiter.java b/platforms/sqlserver/src/main/java/io/ebean/platform/sqlserver/SqlServerSqlLimiter.java
@@ -11,31 +11,23 @@ final class SqlServerSqlLimiter implements SqlLimiter {
 
   @Override
   public SqlLimitResponse limit(SqlLimitRequest request) {
-    String dbSql = request.getDbSql();
-    StringBuilder sb = new StringBuilder(50 + dbSql.length());
     int firstRow = request.getFirstRow();
     int maxRows = request.getMaxRows();
     if (firstRow < 1) {
       // just use top n
-      sb.append("select ");
-      if (request.isDistinct()) {
-        sb.append("distinct ");
-      }
-      sb.append("top ").append(maxRows).append(' ');
-      sb.append(dbSql);
-      return new SqlLimitResponse(sb.toString());
+      final var buffer = request.selectDistinct();
+      buffer.append("top ").append(maxRows).append(' ');
+      buffer.append(request.getDbSql());
+      return new SqlLimitResponse(buffer.toString());
     }
-    sb.append("select ");
-    if (request.isDistinct()) {
-      sb.append("distinct ");
-    }
-    sb.append(dbSql);
-    sb.append(' ').append("offset");
-    sb.append(' ').append(firstRow).append(" rows");
+    final var buffer = request.selectDistinct();
+    buffer.append(request.getDbSql());
+    buffer.append(' ').append("offset");
+    buffer.append(' ').append(firstRow).append(" rows");
     if (maxRows > 0) {
-      sb.append(" fetch next ").append(maxRows).append(" rows only");
+      buffer.append(" fetch next ").append(maxRows).append(" rows only");
     }
-    return new SqlLimitResponse(sb.toString());
+    return new SqlLimitResponse(buffer.toString());
   }
 
 }