diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java index b94b16bc9..8add0f49a 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java @@ -2586,9 +2586,9 @@ public class BeanDescriptor implements BeanType, STreeType { /** * Return a 'dynamic property' used to read a formula. */ - private STreeProperty findSqlTreeFormula(String formulaExpression) { - - return dynamicProperty.computeIfAbsent(formulaExpression, (formula) -> new FormulaPropertyPath(this, formula).build()); + private STreeProperty findSqlTreeFormula(String formula, String path) { + String key = formula + "-" + path; + return dynamicProperty.computeIfAbsent(key, (fullKey) -> new FormulaPropertyPath(this, formula, path).build()); } /** @@ -2597,9 +2597,9 @@ public class BeanDescriptor implements BeanType, STreeType { * The property can be a dynamic formula or a well known bean property. */ @Override - public STreeProperty findPropertyWithDynamic(String propName) { + public STreeProperty findPropertyWithDynamic(String propName, String path) { if (propName.indexOf('(') > -1) { - return findSqlTreeFormula(propName); + return findSqlTreeFormula(propName, path); } return _findBeanProperty(propName); } diff --git a/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java b/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java index df7333c01..20926ceb0 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java +++ b/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java @@ -20,15 +20,17 @@ class FormulaPropertyPath { private final String internalExpression; + private final String path; + private boolean countDistinct; private String cast; private String alias; - FormulaPropertyPath(BeanDescriptor descriptor, String formula) { - + FormulaPropertyPath(BeanDescriptor descriptor, String formula, String path) { this.descriptor = descriptor; this.formula = formula; + this.path = path; int openBracket = formula.indexOf('('); int closeBracket = formula.lastIndexOf(')'); @@ -94,6 +96,11 @@ class FormulaPropertyPath { DeployPropertyParser parser = descriptor.parser().setCatchFirst(true); String parsed = parser.parse(internalExpression); + if (path != null) { + // fetch("machineStats", "sum(hours), sum(totalKms)") + parsed = parsed.replace("${}", "${" + path + "}"); + } + ElPropertyDeploy firstProp = parser.getFirstProp(); ScalarType scalarType; diff --git a/src/main/java/io/ebeaninternal/server/query/STreeType.java b/src/main/java/io/ebeaninternal/server/query/STreeType.java index 1e4d40e9c..ee47e45f8 100644 --- a/src/main/java/io/ebeaninternal/server/query/STreeType.java +++ b/src/main/java/io/ebeaninternal/server/query/STreeType.java @@ -120,7 +120,7 @@ public interface STreeType { /** * Find and return property allowing for dynamic formula properties. */ - STreeProperty findPropertyWithDynamic(String baseName); + STreeProperty findPropertyWithDynamic(String baseName, String path); /** * Return an extra join if the property path requires it. diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java index 1c21b2e6f..4d881ab2a 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java @@ -273,7 +273,10 @@ public final class SqlTreeBuilder { OrmQueryProperties queryProps = queryDetail.getChunk(prefix, false); SqlTreeProperties props = getBaseSelect(desc, queryProps); - if (prefix == null && !rawSql) { + if (prefix != null) { + // check for aggregation on a fetch + props.checkAggregation(); + } else if (!rawSql) { if (props.requireSqlDistinct(manyWhereJoins)) { sqlDistinct = true; } @@ -417,10 +420,10 @@ public final class SqlTreeBuilder { // make sure we only included the base/embedded bean once if (!selectProps.containsProperty(baseName)) { - STreeProperty p = desc.findPropertyWithDynamic(baseName); + STreeProperty p = desc.findPropertyWithDynamic(baseName, null); if (p == null) { // maybe dynamic formula with schema prefix - p = desc.findPropertyWithDynamic(propName); + p = desc.findPropertyWithDynamic(propName, null); if (p != null) { selectProps.add(p); } else { @@ -437,7 +440,7 @@ public final class SqlTreeBuilder { } else { // find the property including searching the // sub class hierarchy if required - STreeProperty p = desc.findPropertyWithDynamic(propName); + STreeProperty p = desc.findPropertyWithDynamic(propName, queryProps.getPath()); if (p == null) { logger.error("property [" + propName + "] not found on " + desc + " for query - excluding it."); p = desc.findProperty("id"); diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java index a62106a52..10fcca6c0 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java @@ -378,7 +378,7 @@ class SqlTreeNodeBean implements SqlTreeNode { if (!readId || temporalVersions) { // a bean with no Id (never found in context) - if (lazyLoadParentId != null && desc.isElementType()) { + if (lazyLoadParentId != null) { ctx.setLazyLoadedChildBean(localBean, lazyLoadParentId); } return localBean; @@ -422,6 +422,9 @@ class SqlTreeNodeBean implements SqlTreeNode { ctx.pushJoin(prefix); ctx.pushTableAlias(prefix); + if (lazyLoadParent != null) { + lazyLoadParent.addSelectExported(ctx, prefix); + } if (readId) { appendSelectId(ctx, idBinder.getBeanProperty()); } @@ -486,7 +489,15 @@ class SqlTreeNodeBean implements SqlTreeNode { @Override public boolean isAggregation() { - return aggregation; + if (aggregation) { + return true; + } + for (SqlTreeNode child : children) { + if (child.isAggregation()) { + return true; + } + } + return false; } /** diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java index 5ec9b286d..f51240a12 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java @@ -92,6 +92,13 @@ public class SqlTreeProperties { return aggregation; } + /** + * Check for aggregation (need for groug by clause). + */ + public void checkAggregation() { + aggregationJoin(); + } + /** * Return the property to join for aggregation. */ diff --git a/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java b/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java index d20581c0b..92b839fbb 100644 --- a/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java +++ b/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java @@ -61,7 +61,7 @@ public class FormulaPropertyPathTest extends BaseTestCase { private void assertFormula(String input, String funcName, String expression, String cast, String alias) { - FormulaPropertyPath propertyPath = new FormulaPropertyPath(customerDesc, input); + FormulaPropertyPath propertyPath = new FormulaPropertyPath(customerDesc, input, null); assertThat(propertyPath.internalExpression()).isEqualTo(expression); assertThat(propertyPath.outerFunction()).isEqualTo(funcName); diff --git a/src/test/java/org/tests/model/aggregation/DMachine.java b/src/test/java/org/tests/model/aggregation/DMachine.java index 8276591bc..afc915bb9 100644 --- a/src/test/java/org/tests/model/aggregation/DMachine.java +++ b/src/test/java/org/tests/model/aggregation/DMachine.java @@ -4,6 +4,7 @@ import io.ebean.Model; import javax.persistence.Entity; import javax.persistence.Id; +import javax.persistence.ManyToOne; import javax.persistence.OneToMany; import javax.persistence.Version; import java.util.List; @@ -12,20 +13,32 @@ import java.util.List; public class DMachine extends Model { @Id - long id; + private long id; - String name; + private String name; + + @ManyToOne + private final DOrg organisation; @Version - long version; + private long version; @OneToMany(mappedBy = "machine") - List machineStats; + private List machineStats; - public DMachine(String name) { + @OneToMany(mappedBy = "machine") + private List auxUseAggs; + + public DMachine(DOrg organisation, String name) { + this.organisation = organisation; this.name = name; } + @Override + public String toString() { + return name; + } + public long getId() { return id; } @@ -42,6 +55,10 @@ public class DMachine extends Model { this.name = name; } + public DOrg getOrganisation() { + return organisation; + } + public long getVersion() { return version; } @@ -57,4 +74,12 @@ public class DMachine extends Model { public void setMachineStats(List machineStats) { this.machineStats = machineStats; } + + public List getAuxUseAggs() { + return auxUseAggs; + } + + public void setAuxUseAggs(List auxUseAggs) { + this.auxUseAggs = auxUseAggs; + } } diff --git a/src/test/java/org/tests/model/aggregation/DMachineAuxUse.java b/src/test/java/org/tests/model/aggregation/DMachineAuxUse.java new file mode 100644 index 000000000..5835913e5 --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/DMachineAuxUse.java @@ -0,0 +1,87 @@ +package org.tests.model.aggregation; + +import io.ebean.Model; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Table; +import javax.persistence.Version; +import java.math.BigDecimal; +import java.time.LocalDate; + +@Entity +@Table(name = "d_machine_aux_use") +public class DMachineAuxUse extends Model { + + @Id + private long id; + + @ManyToOne(optional = false) + private DMachine machine; + + private String name; + + private LocalDate date; + + private long useSecs; + + private BigDecimal fuel; + + @Version + private long version; + + public DMachineAuxUse(DMachine machine, LocalDate date, String name) { + this.machine = machine; + this.date = date; + this.name = name; + } + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public DMachine getMachine() { + return machine; + } + + public void setMachine(DMachine machine) { + this.machine = machine; + } + + public LocalDate getDate() { + return date; + } + + public void setDate(LocalDate date) { + this.date = date; + } + + public long getUseSecs() { + return useSecs; + } + + public void setUseSecs(long useSecs) { + this.useSecs = useSecs; + } + + public BigDecimal getFuel() { + return fuel; + } + + public void setFuel(BigDecimal fuel) { + this.fuel = fuel; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } +} diff --git a/src/test/java/org/tests/model/aggregation/DMachineAuxUseAgg.java b/src/test/java/org/tests/model/aggregation/DMachineAuxUseAgg.java new file mode 100644 index 000000000..9b3798baf --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/DMachineAuxUseAgg.java @@ -0,0 +1,60 @@ +package org.tests.model.aggregation; + +import io.ebean.annotation.Sum; +import io.ebean.annotation.View; + +import javax.persistence.Entity; +import javax.persistence.ManyToOne; +import java.math.BigDecimal; +import java.time.LocalDate; + +@Entity +@View(name = "d_machine_aux_use") +public class DMachineAuxUseAgg { + + @ManyToOne + private DMachine machine; + + private String name; + + private LocalDate date; + + @Sum + private long useSecs; + + @Sum + private BigDecimal fuel; + + public DMachine getMachine() { + return machine; + } + + public void setMachine(DMachine machine) { + this.machine = machine; + } + + public LocalDate getDate() { + return date; + } + + public void setDate(LocalDate date) { + this.date = date; + } + + public long getUseSecs() { + return useSecs; + } + + public void setUseSecs(long useSecs) { + this.useSecs = useSecs; + } + + public BigDecimal getFuel() { + return fuel; + } + + public void setFuel(BigDecimal fuel) { + this.fuel = fuel; + } + +} diff --git a/src/test/java/org/tests/model/aggregation/DMachineUse.java b/src/test/java/org/tests/model/aggregation/DMachineUse.java new file mode 100644 index 000000000..d98e49bd0 --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/DMachineUse.java @@ -0,0 +1,94 @@ +package org.tests.model.aggregation; + +import io.ebean.Model; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Table; +import javax.persistence.Version; +import java.math.BigDecimal; +import java.time.LocalDate; + +@Entity +@Table(name = "d_machine_use") +public class DMachineUse extends Model { + + @Id + private long id; + + @ManyToOne(optional = false) + private DMachine machine; + + private LocalDate date; + + private long distanceKms; + + private long timeSecs; + + private BigDecimal fuel; + + @Version + private long version; + + public DMachineUse(DMachine machine, LocalDate date) { + this.machine = machine; + this.date = date; + } + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public DMachine getMachine() { + return machine; + } + + public void setMachine(DMachine machine) { + this.machine = machine; + } + + public LocalDate getDate() { + return date; + } + + public void setDate(LocalDate date) { + this.date = date; + } + + public long getDistanceKms() { + return distanceKms; + } + + public void setDistanceKms(long distanceKms) { + this.distanceKms = distanceKms; + } + + public long getTimeSecs() { + return timeSecs; + } + + public void setTimeSecs(long timeSecs) { + this.timeSecs = timeSecs; + } + + public BigDecimal getFuel() { + return fuel; + } + + public void setFuel(BigDecimal fuel) { + this.fuel = fuel; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } +} diff --git a/src/test/java/org/tests/model/aggregation/DOrg.java b/src/test/java/org/tests/model/aggregation/DOrg.java new file mode 100644 index 000000000..e0cbd9352 --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/DOrg.java @@ -0,0 +1,48 @@ +package org.tests.model.aggregation; + +import io.ebean.Model; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Version; + +@Entity +public class DOrg extends Model { + + @Id + long id; + + String name; + + @Version + long version; + + public DOrg(String name) { + this.name = name; + } + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } + +} diff --git a/src/test/java/org/tests/model/aggregation/MachineUseData.java b/src/test/java/org/tests/model/aggregation/MachineUseData.java new file mode 100644 index 000000000..06452fdbd --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/MachineUseData.java @@ -0,0 +1,66 @@ +package org.tests.model.aggregation; + +import io.ebean.Ebean; +import io.ebean.annotation.Transactional; + +import java.math.BigDecimal; +import java.math.MathContext; +import java.time.LocalDate; + +public class MachineUseData { + + public static DOrg load() { + + int count = Ebean.find(DMachineUse.class).findCount(); + if (count == 0) { + return new MachineUseData().loadData(); + } else { + return Ebean.find(DOrg.class).where().eq("name", "org0").findOne(); + } + } + + @Transactional(batchSize = 50) + private DOrg loadData() { + + DOrg org = new DOrg("org0"); + org.save(); + + for (int i = 0; i < 5; i++) { + DMachine machine = new DMachine(org, "org0-m" + i); + machine.save(); + + LocalDate startDate = LocalDate.of(2018, 1, 1); + + for (int j = 0; j < 4; j++) { + startDate = startDate.plusDays(5); + createUseFor(machine, startDate, i, j); + } + } + + return org; + } + + private void createUseFor(DMachine machine, LocalDate date, int machineIndex, int dayIndex) { + + DMachineUse use = new DMachineUse(machine, date); + int kms = 100 * machineIndex + dayIndex; + use.setDistanceKms(kms); + use.setFuel(BigDecimal.valueOf(kms).multiply(new BigDecimal(0.1), new MathContext(4))); + use.setTimeSecs(10000 * machineIndex); + use.save(); + + String[] aux = {"Aux1", "Aux2"}; + if (machineIndex % 2 != 0) { + aux = new String[]{"Aux3"}; + } + + int count = 1; + for (String auxName : aux) { + DMachineAuxUse auxUse = new DMachineAuxUse(machine, date, auxName); + auxUse.setUseSecs(10 * machineIndex * 10 + (dayIndex + count++)); + auxUse.setFuel(BigDecimal.valueOf((dayIndex + machineIndex * 7) + count)); + auxUse.save(); + } + + } +} diff --git a/src/test/java/org/tests/model/aggregation/TestAggregationMany.java b/src/test/java/org/tests/model/aggregation/TestAggregationMany.java new file mode 100644 index 000000000..bc0aa7ac4 --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/TestAggregationMany.java @@ -0,0 +1,94 @@ +package org.tests.model.aggregation; + +import io.ebean.BaseTestCase; +import io.ebean.Ebean; +import org.ebeantest.LoggedSqlCollector; +import org.junit.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestAggregationMany extends BaseTestCase { + + @Test + public void fetchQuery_toAggregate() { + + DOrg org = MachineUseData.load(); + + LoggedSqlCollector.start(); + + List machines = Ebean.find(DMachine.class) + .setDisableLazyLoading(true) + .fetchQuery("auxUseAggs", "name, useSecs, fuel") + .where().eq("organisation", org) + .findList(); + + for (DMachine machine : machines) { + assertThat(machine.getAuxUseAggs()).isNotEmpty(); + assertThat(machine.getMachineStats()).isEmpty(); + } + + List sql = LoggedSqlCollector.stop(); + + assertThat(sql).hasSize(2); + assertThat(sql.get(0)).contains("select t0.id, t0.name, t0.version, t0.organisation_id from dmachine t0 where t0.organisation_id = ?"); + + assertThat(machines).hasSize(5); + if (isH2()) { + assertThat(sql.get(1)).contains("select t0.machine_id, t0.name, sum(t0.use_secs), sum(t0.fuel) from d_machine_aux_use t0 where (t0.machine_id) in (?, ?, ?, ?, ? ) group by t0.machine_id, t0.name;"); + } + } + + + @Test + public void fetch_toAggregate() { + + DOrg org = MachineUseData.load(); + + LoggedSqlCollector.start(); + + List machines = Ebean.find(DMachine.class) + .setDisableLazyLoading(true) + .select("name") + .fetch("auxUseAggs", "name, useSecs, fuel") + .where().eq("organisation", org) + .findList(); + + for (DMachine machine : machines) { + assertThat(machine.getAuxUseAggs()).isNotEmpty(); + assertThat(machine.getMachineStats()).isEmpty(); + } + + List sql = LoggedSqlCollector.stop(); + + assertThat(sql).hasSize(1); + assertThat(sql.get(0)).contains("select t0.id, t0.name, t1.name, sum(t1.use_secs), sum(t1.fuel) from dmachine t0 left join d_machine_aux_use t1 on t1.machine_id = t0.id where t0.organisation_id = ? group by t0.id, t0.name, t1.name order by t0.id"); + } + + @Test + public void fetch_toAggregate_onlyAggColumnsInFetchProperties() { + + DOrg org = MachineUseData.load(); + + LoggedSqlCollector.start(); + + List machines = Ebean.find(DMachine.class) + .setDisableLazyLoading(true) + .select("name") + .fetch("auxUseAggs", "useSecs, fuel") + .where().eq("organisation", org) + .findList(); + + for (DMachine machine : machines) { + assertThat(machine.getAuxUseAggs()).isNotEmpty(); + System.out.println(machine); + assertThat(machine.getMachineStats()).isEmpty(); + } + + List sql = LoggedSqlCollector.stop(); + + assertThat(sql).hasSize(1); + assertThat(sql.get(0)).contains("select t0.id, t0.name, sum(t1.use_secs), sum(t1.fuel) from dmachine t0 left join d_machine_aux_use t1 on t1.machine_id = t0.id where t0.organisation_id = ? group by t0.id, t0.name order by t0.id"); + } +} diff --git a/src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java b/src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java index f3bcfda2e..7eb72309c 100644 --- a/src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java +++ b/src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java @@ -200,27 +200,62 @@ public class TestAggregationTopLevel extends BaseTestCase { assertThat(result).isNotEmpty(); } -// @Test -// public void groupBy_in_fetchClause() { -// -// Query query = Ebean.find(DMachine.class) -// .select("name") -// .fetch("machineStats", "date, max(rate), sum(totalKms)") -// .where().gt("machineStats.date", LocalDate.now().minusDays(10)) -// .having().gt("sum(machineStats.totalKms)", 1) -// .query(); -// -// List result = query.findList(); -// assertThat(sqlOf(query)).contains("select distinct t0.id, t0.name, t1.date, max(t1.rate), sum(t1.total_kms) from dmachine t0 left join d_machine_stats t1 on t1.machine_id = t0.id where date > ? group by t0.id, t0.name, t1.date having sum(t1.total_kms) > ? order by t0.id"); -// assertThat(result).isNotEmpty(); -// } + @Test + public void groupBy_in_fetchClause_singleRowInMany() { + + LoggedSqlCollector.start(); + + Query query = Ebean.find(DMachine.class) + .select("name") + .fetch("machineStats", "sum(totalKms)") + .where().eq("name", "Machine0") + .query(); + + List result = query.findList(); + assertThat(result).isNotEmpty(); + assertThat(result.get(0).getMachineStats().get(0).getTotalKms()).isNotNull(); + + List sql = LoggedSqlCollector.stop(); + assertThat(sql).hasSize(1); + + assertThat(sqlOf(query)).contains("select t0.id, t0.name, sum(t1.total_kms) from dmachine t0 left join d_machine_stats t1 on t1.machine_id = t0.id where t0.name = ? group by t0.id, t0.name order by t0.id"); + } + + @Test + public void groupBy_in_fetchClause_multipleRowsInMany() { + + LoggedSqlCollector.start(); + + Query query = Ebean.find(DMachine.class) + .select("name") + .fetch("machineStats", "date, max(rate), sum(totalKms)") + .where().eq("name", "Machine0") + .query(); + + List result = query.findList(); + assertThat(result).isNotEmpty(); + assertThat(result.get(0).getMachineStats().size()).isGreaterThan(1); // expect 8 as grouped by date + + DMachineStats firstStat = result.get(0).getMachineStats().get(0); + assertThat(firstStat.getTotalKms()).isNotNull(); + assertThat(firstStat.getRate()).isNotNull(); + assertThat(firstStat.getDate()).isNotNull(); + + List sql = LoggedSqlCollector.stop(); + assertThat(sql).hasSize(1); + + assertThat(sqlOf(query)).contains("select t0.id, t0.name, t1.date, max(t1.rate), sum(t1.total_kms) from dmachine t0 left join d_machine_stats t1 on t1.machine_id = t0.id where t0.name = ? group by t0.id, t0.name, t1.date order by t0.id"); + } + public static void loadData() { List machines = new ArrayList<>(); + DOrg org = new DOrg("other"); + org.save(); for (int i = 0; i < 5; i++) { - machines.add(new DMachine("Machine"+i)); + machines.add(new DMachine(org, "Machine" + i)); } Ebean.saveAll(machines);