From f031bb72b62453dc7c9b327a1ff3d26d115e1df0 Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Fri, 8 Jun 2018 17:38:15 +1200 Subject: [PATCH] #1408 - Incorrect SQL when @Aggregation formula matches the property - sum(t0.sum(totalKms)) --- .../deploy/meta/DeployBeanProperty.java | 31 +++++ .../org/tests/model/aggregation/DMachine.java | 47 ++++++++ .../model/aggregation/DMachineStats.java | 102 +++++++++++++++++ .../model/aggregation/DMachineStatsAgg.java | 102 +++++++++++++++++ .../aggregation/TestAggregationTopLevel.java | 107 ++++++++++++++++++ 5 files changed, 389 insertions(+) create mode 100644 src/test/java/org/tests/model/aggregation/DMachine.java create mode 100644 src/test/java/org/tests/model/aggregation/DMachineStats.java create mode 100644 src/test/java/org/tests/model/aggregation/DMachineStatsAgg.java create mode 100644 src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java diff --git a/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanProperty.java b/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanProperty.java index f2ae582ee..0bfa5b328 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanProperty.java +++ b/src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanProperty.java @@ -647,11 +647,35 @@ public class DeployBeanProperty { */ public String parseAggregation() { if (aggregation != null) { + int pos = aggregation.indexOf('('); + if (pos > -1) { + // check for recursive property name and formula + String maybePropertyName = aggregation.substring(pos + 1, aggregation.length() - 1); + if (name.equals(maybePropertyName)) { + // e.g. bean property cost mapped to sum(cost) + return aggregationJoin(pos, dbColumn); + } else { + DeployBeanProperty other = desc.getBeanProperty(maybePropertyName); + if (other != null) { + // e.g. bean property maxKms mapped to sum(totalKms) where totalKms is another property + return aggregationJoin(pos, other.getDbColumnRaw()); + } + } + } aggregationParsed = desc.parse(aggregation); } return aggregationParsed; } + /** + * Simple aggregation parsing like sum(someProperty) + */ + private String aggregationJoin(int pos, String dbColumn) { + String p0 = aggregation.substring(0, pos + 1); + aggregationParsed = p0 + "${ta}." + dbColumn + aggregation.substring(aggregation.length() - 1); + return aggregationParsed; + } + public void setAggregation(String aggregation) { this.aggregation = aggregation; this.dbRead = true; @@ -702,6 +726,13 @@ public class DeployBeanProperty { return dbColumn; } + /** + * Return the DB column without any aggregation parsing. + */ + private String getDbColumnRaw() { + return dbColumn; + } + /** * Set the database column name this is mapped to. */ diff --git a/src/test/java/org/tests/model/aggregation/DMachine.java b/src/test/java/org/tests/model/aggregation/DMachine.java new file mode 100644 index 000000000..d45a00a11 --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/DMachine.java @@ -0,0 +1,47 @@ +package org.tests.model.aggregation; + +import io.ebean.Model; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Version; + +@Entity +public class DMachine extends Model { + + @Id + long id; + + String name; + + @Version + long version; + + public DMachine(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/DMachineStats.java b/src/test/java/org/tests/model/aggregation/DMachineStats.java new file mode 100644 index 000000000..642946b30 --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/DMachineStats.java @@ -0,0 +1,102 @@ +package org.tests.model.aggregation; + +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_stats") +public class DMachineStats { + + @Id + long id; + + @ManyToOne + DMachine machine; + + LocalDate date; + + long totalKms; + + long hours; + + BigDecimal rate; + + BigDecimal cost; + + @Version + long version; + + public DMachineStats(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 getTotalKms() { + return totalKms; + } + + public void setTotalKms(long totalKms) { + this.totalKms = totalKms; + } + + public long getHours() { + return hours; + } + + public void setHours(long hours) { + this.hours = hours; + } + + public BigDecimal getRate() { + return rate; + } + + public void setRate(BigDecimal rate) { + this.rate = rate; + } + + public BigDecimal getCost() { + return cost; + } + + public void setCost(BigDecimal cost) { + this.cost = cost; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } +} diff --git a/src/test/java/org/tests/model/aggregation/DMachineStatsAgg.java b/src/test/java/org/tests/model/aggregation/DMachineStatsAgg.java new file mode 100644 index 000000000..94f642d9c --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/DMachineStatsAgg.java @@ -0,0 +1,102 @@ +package org.tests.model.aggregation; + +import io.ebean.annotation.Aggregation; +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_stats", dependentTables = "d_machine_stats") +public class DMachineStatsAgg { + + @ManyToOne + DMachine machine; + + LocalDate date; + + /** + * Matching with column underscore. + */ + @Aggregation("sum(totalKms)") + long totalKms; + + /** + * Matching with no underscore. + */ + @Aggregation("sum(hours)") + long hours; + + @Aggregation("max(rate)") + BigDecimal rate; + + /** + * Not matching. + */ + @Aggregation("sum(cost)") + BigDecimal totalCost; + + /** + * Not matching. + */ + @Aggregation("max(totalKms)") + BigDecimal maxKms; + + 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 getTotalKms() { + return totalKms; + } + + public void setTotalKms(long totalKms) { + this.totalKms = totalKms; + } + + public long getHours() { + return hours; + } + + public void setHours(long hours) { + this.hours = hours; + } + + public BigDecimal getRate() { + return rate; + } + + public void setRate(BigDecimal rate) { + this.rate = rate; + } + + public BigDecimal getTotalCost() { + return totalCost; + } + + public void setTotalCost(BigDecimal totalCost) { + this.totalCost = totalCost; + } + + public BigDecimal getMaxKms() { + return maxKms; + } + + public void setMaxKms(BigDecimal maxKms) { + this.maxKms = maxKms; + } +} diff --git a/src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java b/src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java new file mode 100644 index 000000000..73f693fda --- /dev/null +++ b/src/test/java/org/tests/model/aggregation/TestAggregationTopLevel.java @@ -0,0 +1,107 @@ +package org.tests.model.aggregation; + +import io.ebean.BaseTestCase; +import io.ebean.Ebean; +import io.ebean.Query; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.math.BigDecimal; +import java.time.LocalDate; +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestAggregationTopLevel extends BaseTestCase { + + @BeforeClass + public static void setup() { + loadData(); + } + + @Test + public void query_noSelect() { + + Query query = Ebean.find(DMachineStatsAgg.class) + .where().gt("date", LocalDate.now().minusDays(10)) + .query(); + + List result = query.findList(); + assertThat(sqlOf(query)).contains("select t0.date, t0.machine_id from d_machine_stats t0 where t0.date > ?"); + assertThat(result).isNotEmpty(); + } + + @Test + public void query_machineTotalKms_withHaving() { + + Query query = Ebean.find(DMachineStatsAgg.class) + .select("machine, date, totalKms, totalCost") + .where().gt("date", LocalDate.now().minusDays(10)) + .having().gt("totalCost", 10) + .query(); + + List result = query.findList(); + assertThat(sqlOf(query)).contains("select t0.machine_id, t0.date, sum(t0.total_kms), sum(cost) from d_machine_stats t0 where t0.date > ? group by t0.machine_id, t0.date having sum(cost) > ?"); + assertThat(result).isNotEmpty(); + } + + @Test + public void query_machineTotalKms() { + + Query query = Ebean.find(DMachineStatsAgg.class) + .select("machine, totalKms, totalCost") + .where().gt("date", LocalDate.now().minusDays(10)) + .query(); + + List result = query.findList(); + assertThat(sqlOf(query)).contains("select t0.machine_id, sum(t0.total_kms), sum(cost) from d_machine_stats t0 where t0.date > ? group by t0.machine_id"); + assertThat(result).isNotEmpty(); + } + + @Test + public void query_byDate() { + + Query query = Ebean.find(DMachineStatsAgg.class) + .select("date, totalKms, hours, rate, totalCost, maxKms") + .where().gt("date", LocalDate.now().minusDays(10)) + .having().gt("hours", 2) + .query(); + + List result = query.findList(); + assertThat(sqlOf(query)).contains("select t0.date, sum(t0.total_kms), sum(t0.hours), max(t0.rate), sum(cost), max(t0.total_kms) from d_machine_stats t0 where t0.date > ? group by t0.date having sum(t0.hours) > ?"); + assertThat(result).isNotEmpty(); + } + + private static void loadData() { + + List machines = new ArrayList<>(); + + for (int i = 0; i < 5; i++) { + machines.add(new DMachine("Machine"+i)); + } + + Ebean.saveAll(machines); + + List allStats = new ArrayList<>(); + + LocalDate date = LocalDate.now(); + for (int i = 0; i < 8; i++) { + for (DMachine machine : machines) { + + DMachineStats stats = new DMachineStats(machine, date); + + stats.setHours(i * 4); + stats.setTotalKms(i * 100); + stats.setCost(BigDecimal.valueOf(i * 50)); + stats.setRate(BigDecimal.valueOf(i * 2)); + + allStats.add(stats); + } + + date = date.minusDays(1); + } + + Ebean.saveAll(allStats); + } +}