diff --git a/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormulaMTO.java b/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormulaMTO.java index d539dfef5..eb8ba9997 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormulaMTO.java +++ b/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormulaMTO.java @@ -11,6 +11,11 @@ public class DynamicPropertyAggregationFormulaMTO extends DynamicPropertyAggrega this.prop = prop; } + @Override + public boolean isAggregationManyToOne() { + return true; + } + @Override public void load(SqlBeanLoad sqlBeanLoad) { Object value; diff --git a/src/main/java/io/ebeaninternal/server/query/STreeProperty.java b/src/main/java/io/ebeaninternal/server/query/STreeProperty.java index b0c2ee916..95911b026 100644 --- a/src/main/java/io/ebeaninternal/server/query/STreeProperty.java +++ b/src/main/java/io/ebeaninternal/server/query/STreeProperty.java @@ -39,6 +39,13 @@ public interface STreeProperty extends ScalarDataReader { */ boolean isAggregation(); + /** + * Return true if the property is an aggregation on a ManyToOne. + */ + default boolean isAggregationManyToOne() { + return false; + } + /** * Return true if the property is a formula. */ diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java index 4d881ab2a..8d15d0085 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java @@ -273,10 +273,7 @@ public final class SqlTreeBuilder { OrmQueryProperties queryProps = queryDetail.getChunk(prefix, false); SqlTreeProperties props = getBaseSelect(desc, queryProps); - if (prefix != null) { - // check for aggregation on a fetch - props.checkAggregation(); - } else if (!rawSql) { + if (prefix == null && !rawSql) { if (props.requireSqlDistinct(manyWhereJoins)) { sqlDistinct = true; } @@ -479,12 +476,24 @@ public final class SqlTreeBuilder { // Also note that this can include transient properties. // This makes sense for transient properties used to // hold sum() count() type values (with SqlSelect) - for (String propName : queryProps.getSelectProperties()) { + final Set selectInclude = queryProps.getSelectInclude(); + for (String propName : selectInclude) { if (!propName.isEmpty()) { addProperty(selectProps, desc, queryProps, propName); } } + if (!selectProps.isAggregationManyToOne()) { + final Set selectQueryJoin = queryProps.getSelectQueryJoin(); + if (selectQueryJoin != null) { + for (String joinProperty : selectQueryJoin) { + if (!selectInclude.contains(joinProperty)) { + addProperty(selectProps, desc, queryProps, joinProperty); + } + } + } + } + return selectProps; } diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java index f51240a12..d36a0f2cd 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java @@ -29,6 +29,8 @@ public class SqlTreeProperties { private boolean allProperties; + private boolean aggregationManyToOne; + private boolean aggregation; private String aggregationPath; @@ -47,6 +49,15 @@ public class SqlTreeProperties { public void add(STreeProperty prop) { propsList.add(prop); propNames.add(prop.getName()); + if (prop.isAggregation()) { + if (!aggregation) { + aggregation = true; + aggregationPath = prop.getElPrefix(); + } + if (prop.isAggregationManyToOne()) { + aggregationManyToOne = true; + } + } } public STreeProperty[] getProps() { @@ -85,6 +96,13 @@ public class SqlTreeProperties { } } + /** + * Return true if this is an aggregation formula on a ManyToOne. + */ + boolean isAggregationManyToOne() { + return aggregationManyToOne; + } + /** * Return true if this contains an aggregation property. */ @@ -92,33 +110,17 @@ public class SqlTreeProperties { return aggregation; } - /** - * Check for aggregation (need for groug by clause). - */ - public void checkAggregation() { - aggregationJoin(); - } - /** * Return the property to join for aggregation. */ private String aggregationJoin() { - if (!allProperties) { - for (STreeProperty beanProperty : propsList) { - if (beanProperty.isAggregation()) { - aggregation = true; - aggregationPath = beanProperty.getElPrefix(); - return aggregationPath; - } - } - } - return null; + return aggregationPath; } /** * Return true if a top level aggregation which means the Id property must be excluded. */ - public boolean isAggregationRoot() { + boolean isAggregationRoot() { return aggregation && (aggregationPath == null); } } diff --git a/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java b/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java index 964250c4a..ca7a4828e 100644 --- a/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java +++ b/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java @@ -348,22 +348,12 @@ public class OrmQueryProperties implements Serializable { includedBeanJoin.add(propertyName); } - /** - * This excludes the bean joined properties. - *

- * This is because bean joins will have there own node in the SqlTree. - *

- */ - public Set getSelectProperties() { + public Set getSelectInclude() { + return included; + } - if (secondaryQueryJoins == null) { - return included; - } - - LinkedHashSet temp = new LinkedHashSet<>(2 * (secondaryQueryJoins.size() + included.size())); - temp.addAll(included); - temp.addAll(secondaryQueryJoins); - return temp; + public Set getSelectQueryJoin() { + return secondaryQueryJoins; } void addSecondaryQueryJoin(String property) { diff --git a/src/test/java/io/ebean/plugin/BeanTypeTest.java b/src/test/java/io/ebean/plugin/BeanTypeTest.java index 42cfb3aa2..2b880805a 100644 --- a/src/test/java/io/ebean/plugin/BeanTypeTest.java +++ b/src/test/java/io/ebean/plugin/BeanTypeTest.java @@ -25,9 +25,9 @@ import static org.junit.Assert.assertTrue; public class BeanTypeTest { - static Database db = DB.getDefault(); + private static Database db = DB.getDefault(); - BeanType beanType(Class cls) { + private BeanType beanType(Class cls) { return db.getPluginApi().getBeanType(cls); } @@ -177,11 +177,9 @@ public class BeanTypeTest { beanType(Order.class).docStore().applyPath(orderQuery); OrmQueryDetail detail = orderQuery.getDetail(); - assertThat(detail.getChunk("customer", false).getSelectProperties()) - .containsExactly("id", "name"); + assertThat(detail.getChunk("customer", false).getSelectInclude()).containsExactly("id", "name"); } - @Test(expected = IllegalStateException.class) public void docStoreIndex() throws Exception { beanType(Order.class).docStore().index(1, new Order(), null); diff --git a/src/test/java/org/tests/aggregateformula/TestAggregateFormula.java b/src/test/java/org/tests/aggregateformula/TestAggregateFormula.java index 471171e79..16be122b1 100644 --- a/src/test/java/org/tests/aggregateformula/TestAggregateFormula.java +++ b/src/test/java/org/tests/aggregateformula/TestAggregateFormula.java @@ -4,13 +4,16 @@ import io.ebean.BaseTestCase; import io.ebean.Ebean; import org.ebeantest.LoggedSqlCollector; import org.junit.Test; +import org.tests.model.basic.Address; import org.tests.model.basic.Contact; +import org.tests.model.basic.Customer; import org.tests.model.basic.Order; import org.tests.model.basic.ResetBasicData; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertNotNull; public class TestAggregateFormula extends BaseTestCase { @@ -36,35 +39,71 @@ public class TestAggregateFormula extends BaseTestCase { assertThat(contact.getCustomer().getId()).isNotNull(); } -// @Test -// public void minOnManyToOne_withFetchQuery() { -// -// ResetBasicData.reset(); -// -// LoggedSqlCollector.start(); -// -// List contacts = Ebean.find(Contact.class) -// .select("lastName, min(customer)") -// .fetchQuery("customer", "name, status") -// //.fetch("customer.billingAddress", "city, country") -// .findList(); -// -// for (Contact contact : contacts) { -// contact.getCustomer().getName(); -// } -// -// List sql = LoggedSqlCollector.stop(); -// assertThat(sql).hasSize(2); -// assertThat(sql.get(0)).contains("select t0.last_name, min(t0.customer_id) from contact t0 group by t0.last_name"); -// assertThat(sql.get(1)).contains("select t0.id, t0.name, t0.status from o_customer t0 where"); -// -// assertThat(contacts).isNotEmpty(); -// -// Contact contact = contacts.get(0); -// assertThat(contact.getLastName()).isNotNull(); -// assertThat(contact.getCustomer()).isNotNull(); -// assertThat(contact.getCustomer().getId()).isNotNull(); -// } + @Test + public void minOnManyToOne_withFetchQuery() { + + ResetBasicData.reset(); + + LoggedSqlCollector.start(); + + List contacts = Ebean.find(Contact.class) + .select("lastName, min(customer)") + .fetchQuery("customer", "name, status") + .findList(); + + for (Contact contact : contacts) { + Customer customer = contact.getCustomer(); + assertNotNull(customer.getName()); + assertNotNull(customer.getStatus()); + } + + List sql = LoggedSqlCollector.stop(); + assertThat(sql).hasSize(2); + assertThat(sql.get(0)).contains("select t0.last_name, min(t0.customer_id) from contact t0 group by t0.last_name"); + assertThat(sql.get(1)).contains("select t0.id, t0.name, t0.status from o_customer t0 where"); + + assertThat(contacts).isNotEmpty(); + + Contact contact = contacts.get(0); + assertThat(contact.getLastName()).isNotNull(); + assertThat(contact.getCustomer()).isNotNull(); + assertThat(contact.getCustomer().getId()).isNotNull(); + } + + @Test + public void minOnManyToOne_withFetchQueryWithJoin() { + + ResetBasicData.reset(); + LoggedSqlCollector.start(); + + List contacts = Ebean.find(Contact.class) + .select("lastName, min(customer)") + .fetchQuery("customer", "name, status") + .fetch("customer.billingAddress", "city, country") + .findList(); + + for (Contact contact : contacts) { + Customer customer = contact.getCustomer(); + assertNotNull(customer.getName()); + assertNotNull(customer.getStatus()); + final Address billingAddress = customer.getBillingAddress(); + if (billingAddress != null) { + assertNotNull(billingAddress.getCountry()); + } + } + + List sql = LoggedSqlCollector.stop(); + assertThat(sql).hasSize(2); + assertThat(sql.get(0)).contains("select t0.last_name, min(t0.customer_id) from contact t0 group by t0.last_name"); + assertThat(sql.get(1)).contains("select t0.id, t0.name, t0.status, t1.id, t1.city, t1.country_code from o_customer t0 left join o_address t1 on t1.id = t0.billing_address_id where"); + + assertThat(contacts).isNotEmpty(); + + Contact contact = contacts.get(0); + assertThat(contact.getLastName()).isNotNull(); + assertThat(contact.getCustomer()).isNotNull(); + assertThat(contact.getCustomer().getId()).isNotNull(); + } @Test public void sum_withoutAlias() {