From f7ece0f13cb4679bbe940ed0f5673d4963d89d80 Mon Sep 17 00:00:00 2001 From: rbygrave Date: Mon, 21 Jul 2014 21:49:31 +1200 Subject: [PATCH] Fix for #174 - Incorrect SQL generated with syntax error at the "order by" clause --- .../deploy/DeployPropertyParserMap.java | 47 ++++++------- .../server/query/CQueryPredicates.java | 27 ++++++-- .../server/query/SqlTreeBuilder.java | 1 + .../com/avaje/tests/model/pview/Paggview.java | 34 ++++++++++ .../com/avaje/tests/model/pview/Pview.java | 66 +++++++++++++++++++ .../avaje/tests/model/pview/TestPview.java | 33 ++++++++++ .../com/avaje/tests/model/pview/Wview.java | 39 +++++++++++ 7 files changed, 218 insertions(+), 29 deletions(-) create mode 100644 src/test/java/com/avaje/tests/model/pview/Paggview.java create mode 100644 src/test/java/com/avaje/tests/model/pview/Pview.java create mode 100644 src/test/java/com/avaje/tests/model/pview/TestPview.java create mode 100644 src/test/java/com/avaje/tests/model/pview/Wview.java diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/DeployPropertyParserMap.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/DeployPropertyParserMap.java index 13f41684c..19b1078fa 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/DeployPropertyParserMap.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/DeployPropertyParserMap.java @@ -1,5 +1,6 @@ package com.avaje.ebeaninternal.server.deploy; +import java.util.Collections; import java.util.Map; import java.util.Set; @@ -8,33 +9,33 @@ import java.util.Set; */ public final class DeployPropertyParserMap extends DeployParser { - private final Map map; + private final Map map; - public DeployPropertyParserMap(Map map) { - this.map = map; - } + public DeployPropertyParserMap(Map map) { + this.map = map; + } - /** - * Returns null for raw sql queries. - */ - public Set getIncludes() { - return null; - } + /** + * Returns null for raw sql queries. + */ + public Set getIncludes() { + return Collections.emptySet(); + } - public String convertWord() { - String r = getDeployWord(word); - return r == null ? word : r; - } + public String convertWord() { + String r = getDeployWord(word); + return r == null ? word : r; + } - @Override - public String getDeployWord(String expression) { - - String deployExpr = map.get(expression); - if (deployExpr == null) { - return null; - } else { - return deployExpr; - } + @Override + public String getDeployWord(String expression) { + + String deployExpr = map.get(expression); + if (deployExpr == null) { + return null; + } else { + return deployExpr; } + } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryPredicates.java b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryPredicates.java index 1f11e6f68..c1951fb8b 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryPredicates.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryPredicates.java @@ -2,8 +2,12 @@ package com.avaje.ebeaninternal.server.query; import java.sql.SQLException; import java.util.ArrayList; +import java.util.HashSet; import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import com.avaje.ebeaninternal.api.BindParams; import com.avaje.ebeaninternal.api.BindParams.OrderedList; import com.avaje.ebeaninternal.api.SpiExpressionList; @@ -17,8 +21,6 @@ import com.avaje.ebeaninternal.server.querydefn.OrmQueryProperties; import com.avaje.ebeaninternal.server.type.DataBind; import com.avaje.ebeaninternal.server.util.BindParamsParser; import com.avaje.ebeaninternal.util.DefaultExpressionRequest; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Compile Query Predicates. @@ -121,6 +123,8 @@ public class CQueryPredicates { * Includes from where and order by clauses. */ private Set predicateIncludes; + + private Set orderByIncludes; public CQueryPredicates(Binder binder, OrmQueryRequest request) { this.binder = binder; @@ -316,16 +320,20 @@ public class CQueryPredicates { */ private void parsePropertiesToDbColumns(DeployParser deployParser) { - dbWhere = deriveWhere(deployParser); - dbFilterMany = deriveFilterMany(deployParser); - dbHaving = deriveHaving(deployParser); - // order by is dependent on the manyProperty (if there is one) logicalOrderBy = deriveOrderByWithMany(request.getManyProperty()); if (logicalOrderBy != null) { dbOrderBy = deployParser.parse(logicalOrderBy); } + + // create a copy of the includes required to support the orderBy + orderByIncludes = new HashSet(deployParser.getIncludes()); + dbWhere = deriveWhere(deployParser); + dbFilterMany = deriveFilterMany(deployParser); + dbHaving = deriveHaving(deployParser); + + // all includes including ones for manyWhere clause predicateIncludes = deployParser.getIncludes(); } @@ -504,6 +512,13 @@ public class CQueryPredicates { return predicateIncludes; } + /** + * Return the orderBy includes. + */ + public Set getOrderByIncludes() { + return orderByIncludes; + } + /** * The where sql with named bind parameters converted to ?. */ diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java b/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java index ebb2e4a26..92415e2a2 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeBuilder.java @@ -291,6 +291,7 @@ public class SqlTreeBuilder { // remove ManyWhereJoins from the predicateIncludes predicateIncludes.removeAll(manyWhereJoins.getPropertyNames()); + predicateIncludes.addAll(predicates.getOrderByIncludes()); // look for predicateIncludes that are not in selectIncludes and add // them as extra joins to the query diff --git a/src/test/java/com/avaje/tests/model/pview/Paggview.java b/src/test/java/com/avaje/tests/model/pview/Paggview.java new file mode 100644 index 000000000..4ea8bf0ce --- /dev/null +++ b/src/test/java/com/avaje/tests/model/pview/Paggview.java @@ -0,0 +1,34 @@ +package com.avaje.tests.model.pview; + +import javax.persistence.Basic; +import javax.persistence.Entity; +import javax.persistence.OneToOne; +import javax.persistence.Table; + +@Entity +@Table(name = "paggview") +public class Paggview { + + @OneToOne + private Pview pview; + + @Basic(optional = false) + private Integer amount; + + public Pview getPview() { + return pview; + } + + public void setPview(Pview pview) { + this.pview = pview; + } + + public Integer getAmount() { + return amount; + } + + public void setAmount(Integer amount) { + this.amount = amount; + } + +} diff --git a/src/test/java/com/avaje/tests/model/pview/Pview.java b/src/test/java/com/avaje/tests/model/pview/Pview.java new file mode 100644 index 000000000..126bdaaab --- /dev/null +++ b/src/test/java/com/avaje/tests/model/pview/Pview.java @@ -0,0 +1,66 @@ +package com.avaje.tests.model.pview; + +import java.util.List; +import java.util.UUID; + +import javax.persistence.Basic; +import javax.persistence.CascadeType; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.JoinTable; +import javax.persistence.ManyToMany; +import javax.persistence.Table; + +@Entity +@Table(name = "pp") +public class Pview { + + @Id + private UUID id; + + private String name; + + @Basic(optional = false) + @Column(length = 100, nullable = false) + private String value; + + @JoinTable(name = "pp_to_ww", joinColumns = { @JoinColumn(name = "pp_id", referencedColumnName = "id") }, inverseJoinColumns = { @JoinColumn(name = "ww_id", referencedColumnName = "id") }) + @ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.LAZY) + private List wviews; + + public UUID getId() { + return id; + } + + public void setId(UUID id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getValue() { + return value; + } + + public void setValue(String value) { + this.value = value; + } + + public List getWviews() { + return wviews; + } + + public void setWviews(List wviews) { + this.wviews = wviews; + } + +} diff --git a/src/test/java/com/avaje/tests/model/pview/TestPview.java b/src/test/java/com/avaje/tests/model/pview/TestPview.java new file mode 100644 index 000000000..3519ef6e6 --- /dev/null +++ b/src/test/java/com/avaje/tests/model/pview/TestPview.java @@ -0,0 +1,33 @@ +package com.avaje.tests.model.pview; + +import java.util.UUID; + +import org.junit.Assert; +import org.junit.Test; + +import com.avaje.ebean.BaseTestCase; +import com.avaje.ebean.Ebean; +import com.avaje.ebean.Query; +import com.avaje.ebean.config.GlobalProperties; + +public class TestPview extends BaseTestCase { + + @Test + public void test() { + + GlobalProperties.put("ebean.search.packages", "com.avaje.tests.model.odd"); + + Wview wview = Ebean.getReference(Wview.class, UUID.randomUUID()); + + Query query = Ebean.find(Paggview.class); + query.select("amount"); + query.where().eq("pview.wviews", wview); + query.orderBy("pview.value"); + query.findList(); + String generatedSql = query.getGeneratedSql(); + + Assert.assertTrue(generatedSql.contains("select distinct t0.amount c0, t1.value from paggview t0 join pp u1 on u1.id = t0.pview_id join pp_to_ww u2z_ on u2z_.pp_id = u1.id join wview u2 on u2.id = u2z_.ww_id left outer join pp t1 on t1.id = t0.pview_id where u2.id = ? order by t1.value")); + + } + +} diff --git a/src/test/java/com/avaje/tests/model/pview/Wview.java b/src/test/java/com/avaje/tests/model/pview/Wview.java new file mode 100644 index 000000000..a33cac601 --- /dev/null +++ b/src/test/java/com/avaje/tests/model/pview/Wview.java @@ -0,0 +1,39 @@ +package com.avaje.tests.model.pview; + +import java.util.UUID; + +import javax.persistence.Basic; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Table; + +@Entity +@Table(name = "wview") +public class Wview { + + @Id + @Column(name = "id") + private UUID id; + + @Basic(optional = false) + @Column(unique = true) + private String name; + + public UUID getId() { + return id; + } + + public void setId(UUID id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + +} \ No newline at end of file