diff --git a/src/main/java/io/ebeaninternal/api/SpiQuery.java b/src/main/java/io/ebeaninternal/api/SpiQuery.java index 24d625f47..881cae5e3 100644 --- a/src/main/java/io/ebeaninternal/api/SpiQuery.java +++ b/src/main/java/io/ebeaninternal/api/SpiQuery.java @@ -749,23 +749,6 @@ public interface SpiQuery extends Query, TxnProfileEventCodes { */ boolean isDisableLazyLoading(); - /** - * Internally set by Ebean when this query must use the DISTINCT keyword. - *

- * This does not exclude/remove the use of the id property. - */ - void setSqlDistinct(boolean sqlDistinct); - - /** - * Return true if this query has been specified by a user or internally by Ebean to use DISTINCT. - */ - boolean isDistinctQuery(); - - /** - * Return true if this was internally set to sql distinct (ie. many where predicate). - */ - boolean isSqlDistinct(); - /** * Return true if this query has been specified by a user to use DISTINCT. */ diff --git a/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java b/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java index eeacc87df..8b2e52f46 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java +++ b/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java @@ -280,13 +280,8 @@ class CQueryBuilder { if (sqlTree.isSingleProperty()) { request.setInlineCountDistinct(); } - } else { - if (hasMany) { - // need to count distinct id's ... - query.setSqlDistinct(true); - } else { - sqlSelect = "select count(*)"; - } + } else if (!hasMany) { + sqlSelect = "select count(*)"; } SqlLimitResponse s = buildSql(sqlSelect, request, predicates, sqlTree); @@ -556,6 +551,7 @@ class CQueryBuilder { return rawSqlHandler.buildSql(request, predicates, query.getRawSql().getSql()); } + boolean distinct = query.isDistinct() || select.isSqlDistinct(); boolean useSqlLimiter = false; StringBuilder sb = new StringBuilder(500); String dbOrderBy = predicates.getDbOrderBy(); @@ -568,7 +564,7 @@ class CQueryBuilder { if (!useSqlLimiter) { sb.append("select "); - if (query.isDistinctQuery()) { + if (distinct) { if (request.isInlineCountDistinct()) { sb.append("count("); } @@ -590,7 +586,7 @@ class CQueryBuilder { if (request.isInlineCountDistinct()) { sb.append(")"); } - if (query.isDistinctQuery() && dbOrderBy != null && !query.isSingleAttribute()) { + if (distinct && dbOrderBy != null && !query.isSingleAttribute()) { // add the orderBy columns to the select clause (due to distinct) sb.append(", ").append(DbOrderByTrim.trim(dbOrderBy)); } @@ -692,7 +688,7 @@ class CQueryBuilder { if (useSqlLimiter) { // use LIMIT/OFFSET, ROW_NUMBER() or rownum type SQL query limitation - SqlLimitRequest r = new OrmQueryLimitRequest(sb.toString(), dbOrderBy, query, dbPlatform); + SqlLimitRequest r = new OrmQueryLimitRequest(sb.toString(), dbOrderBy, query, dbPlatform, distinct); return sqlLimiter.limit(r); } else { diff --git a/src/main/java/io/ebeaninternal/server/query/CQueryBuilderRawSql.java b/src/main/java/io/ebeaninternal/server/query/CQueryBuilderRawSql.java index 0bbee778d..0d37f2344 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQueryBuilderRawSql.java +++ b/src/main/java/io/ebeaninternal/server/query/CQueryBuilderRawSql.java @@ -1,6 +1,5 @@ package io.ebeaninternal.server.query; -import io.ebeaninternal.server.rawsql.SpiRawSql; import io.ebean.config.dbplatform.DatabasePlatform; import io.ebean.config.dbplatform.SqlLimitResponse; import io.ebean.config.dbplatform.SqlLimiter; @@ -9,6 +8,7 @@ import io.ebeaninternal.api.SpiQuery; import io.ebeaninternal.server.core.OrmQueryRequest; import io.ebeaninternal.server.deploy.BeanDescriptor; import io.ebeaninternal.server.querydefn.OrmQueryLimitRequest; +import io.ebeaninternal.server.rawsql.SpiRawSql; import io.ebeaninternal.server.util.BindParamsParser; class CQueryBuilderRawSql { @@ -50,7 +50,7 @@ class CQueryBuilderRawSql { SpiQuery query = request.getQuery(); if (query.hasMaxRowsOrFirstRow() && sqlLimiter != null) { // wrap with a limit offset or ROW_NUMBER() etc - return sqlLimiter.limit(new OrmQueryLimitRequest(sql, orderBy, query, dbPlatform)); + return sqlLimiter.limit(new OrmQueryLimitRequest(sql, orderBy, query, dbPlatform, rsql.isDistinct() || query.isDistinct())); } else { // add back select keyword (it was removed to support sqlQueryLimiter) diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTree.java b/src/main/java/io/ebeaninternal/server/query/SqlTree.java index 91e71fb0d..ad9128f46 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTree.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTree.java @@ -65,6 +65,13 @@ class SqlTree { this.includeJoins = includeJoins; } + /** + * Return true if the query mandates SQL Distinct due to ToMany inclusion. + */ + boolean isSqlDistinct() { + return rootNode.isSqlDistinct(); + } + /** * Return true if the query includes joins (not valid for rawSql). */ diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java index 8fa67bd2c..00ddc14c5 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java @@ -70,6 +70,8 @@ public final class SqlTreeBuilder { private SqlTreeNode rootNode; + private boolean sqlDistinct; + /** * Construct for RawSql query. */ @@ -172,7 +174,7 @@ public final class SqlTreeBuilder { private String buildDistinctOn() { - if (rawSql || !distinctOnPlatform || !query.isSqlDistinct() || Type.COUNT == query.getType()) { + if (rawSql || !distinctOnPlatform || !sqlDistinct || Type.COUNT == query.getType()) { return null; } ctx.startGroupBy(); @@ -273,7 +275,7 @@ public final class SqlTreeBuilder { if (prefix == null && !rawSql) { if (props.requireSqlDistinct(manyWhereJoins)) { - query.setSqlDistinct(true); + sqlDistinct = true; } addManyWhereJoins(myJoinList); } @@ -310,7 +312,7 @@ public final class SqlTreeBuilder { // Optional many property for lazy loading query STreePropertyAssocMany lazyLoadMany = (query == null) ? null : query.getLazyLoadMany(); boolean withId = !rawNoId && !subQuery && (query == null || query.isWithId()); - return new SqlTreeNodeRoot(desc, props, myList, withId, includeJoin, lazyLoadMany, temporalMode, disableLazyLoad); + return new SqlTreeNodeRoot(desc, props, myList, withId, includeJoin, lazyLoadMany, temporalMode, disableLazyLoad, sqlDistinct); } else if (prop instanceof STreePropertyAssocMany) { return new SqlTreeNodeManyRoot(prefix, (STreePropertyAssocMany) prop, props, myList, temporalMode, disableLazyLoad); @@ -361,7 +363,7 @@ public final class SqlTreeBuilder { // as we are now going to join to the many then we need // to add the distinct to the sql query to stop duplicate // rows... - query.setSqlDistinct(true); + sqlDistinct = true; } } } diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java index c78de34a5..70fa0cdd2 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java @@ -20,6 +20,10 @@ interface SqlTreeNode { */ void buildRawSqlSelectChain(List selectChain); + default boolean isSqlDistinct() { + return false; + } + /** * Return true if this node includes an aggregation. */ diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeRoot.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeRoot.java index 2a69a0c16..2f25bf7e2 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeRoot.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeRoot.java @@ -14,14 +14,17 @@ final class SqlTreeNodeRoot extends SqlTreeNodeBean { private final TableJoin includeJoin; + private final boolean sqlDistinct; + /** * Specify for SqlSelect to include an Id property or not. */ SqlTreeNodeRoot(STreeType desc, SqlTreeProperties props, List myList, boolean withId, - TableJoin includeJoin, STreePropertyAssocMany many, SpiQuery.TemporalMode temporalMode, boolean disableLazyLoad) { + TableJoin includeJoin, STreePropertyAssocMany many, SpiQuery.TemporalMode temporalMode, boolean disableLazyLoad, boolean sqlDistinct) { super(desc, props, myList, withId, many, temporalMode, disableLazyLoad); this.includeJoin = includeJoin; + this.sqlDistinct = sqlDistinct; } @Override @@ -29,6 +32,11 @@ final class SqlTreeNodeRoot extends SqlTreeNodeBean { return true; } + @Override + public boolean isSqlDistinct() { + return sqlDistinct; + } + /** * Append the property columns to the buffer. */ diff --git a/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java b/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java index fc43615b3..4b4b906dd 100644 --- a/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java +++ b/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java @@ -146,12 +146,6 @@ public class DefaultOrmQuery implements SpiQuery { */ private boolean distinct; - /** - * Set to true internally by Ebean when it needs the DISTINCT keyword added to the query (id - * property still expected). - */ - private boolean sqlDistinct; - /** * Set to true if this is a future fetch using background threads. */ @@ -772,7 +766,6 @@ public class DefaultOrmQuery implements SpiQuery { copy.rootTableAlias = rootTableAlias; copy.distinct = distinct; - copy.sqlDistinct = sqlDistinct; copy.timeout = timeout; copy.mapKey = mapKey; copy.id = id; @@ -1086,9 +1079,6 @@ public class DefaultOrmQuery implements SpiQuery { if (distinct) { sb.append(",dist:"); } - if (sqlDistinct) { - sb.append(",sqlD:"); - } if (disableLazyLoading) { sb.append(",disLazy:"); } @@ -1635,28 +1625,6 @@ public class DefaultOrmQuery implements SpiQuery { return countDistinctOrder != null; } - /** - * Return true if this query uses SQL DISTINCT either explicitly by the user or internally defined - * by ebean. - */ - @Override - public boolean isDistinctQuery() { - return distinct || sqlDistinct; - } - - @Override - public boolean isSqlDistinct() { - return sqlDistinct; - } - - /** - * Internally set to use SQL DISTINCT on the query but still have id property included. - */ - @Override - public void setSqlDistinct(boolean sqlDistinct) { - this.sqlDistinct = sqlDistinct; - } - @Override public Class getBeanType() { return beanType; diff --git a/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryLimitRequest.java b/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryLimitRequest.java index 085586376..99354bf71 100644 --- a/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryLimitRequest.java +++ b/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryLimitRequest.java @@ -14,11 +14,14 @@ public class OrmQueryLimitRequest implements SqlLimitRequest { private final String sqlOrderBy; - public OrmQueryLimitRequest(String sql, String sqlOrderBy, SpiQuery ormQuery, DatabasePlatform dbPlatform) { + private final boolean distinct; + + public OrmQueryLimitRequest(String sql, String sqlOrderBy, SpiQuery ormQuery, DatabasePlatform dbPlatform, boolean distinct) { this.sql = sql; this.sqlOrderBy = sqlOrderBy; this.ormQuery = ormQuery; this.dbPlatform = dbPlatform; + this.distinct = distinct; } @Override @@ -43,7 +46,7 @@ public class OrmQueryLimitRequest implements SqlLimitRequest { @Override public boolean isDistinct() { - return ormQuery.isDistinctQuery(); + return distinct; } @Override diff --git a/src/test/java/io/ebeaninternal/server/querydefn/OrmQueryPlanKeyTest.java b/src/test/java/io/ebeaninternal/server/querydefn/OrmQueryPlanKeyTest.java index 043ecf6f0..4cffd3e61 100644 --- a/src/test/java/io/ebeaninternal/server/querydefn/OrmQueryPlanKeyTest.java +++ b/src/test/java/io/ebeaninternal/server/querydefn/OrmQueryPlanKeyTest.java @@ -165,28 +165,6 @@ public class OrmQueryPlanKeyTest extends BaseExpressionTest { assertSame(key1, key2); } - @Test - public void equals_when_diffSqlDistinct() { - DefaultOrmQuery q1 = query(); - q1.setSqlDistinct(true); - CQueryPlanKey key1 = q1.createQueryPlanKey(); - CQueryPlanKey key2 = query().createQueryPlanKey(); - - assertDifferent(key1, key2); - } - - @Test - public void equals_when_sameSqlDistinct() { - - DefaultOrmQuery q1 = query(); - q1.setSqlDistinct(true); - - DefaultOrmQuery q2 = query(); - q2.setSqlDistinct(true); - - assertSame(q1, q2); - } - @Test public void equals_when_useDocStore() { CQueryPlanKey key1 = query().setUseDocStore(true).createQueryPlanKey();