From 2de57f7b35764bb814436ec00e7ae66678530b16 Mon Sep 17 00:00:00 2001 From: Daryl Stultz Date: Wed, 8 May 2013 14:32:57 -0400 Subject: [PATCH 1/2] Exposed bug 417 (and 408?) --- .../TestInheritQuery.java | 126 +++++++++++++++++- .../model/Warehouse.java | 49 +++++++ .../singleTableInheritance/model/Zone.java | 14 ++ .../model/ZoneExternal.java | 5 + .../model/ZoneInternal.java | 24 ++++ 5 files changed, 213 insertions(+), 5 deletions(-) create mode 100644 src/test/java/com/avaje/tests/singleTableInheritance/model/Warehouse.java create mode 100644 src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneInternal.java diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/TestInheritQuery.java b/src/test/java/com/avaje/tests/singleTableInheritance/TestInheritQuery.java index 048191c10..347b4ecc7 100644 --- a/src/test/java/com/avaje/tests/singleTableInheritance/TestInheritQuery.java +++ b/src/test/java/com/avaje/tests/singleTableInheritance/TestInheritQuery.java @@ -8,16 +8,12 @@ import org.junit.Test; import com.avaje.ebean.BaseTestCase; import com.avaje.ebean.Ebean; -import com.avaje.tests.singleTableInheritance.model.PalletLocation; -import com.avaje.tests.singleTableInheritance.model.PalletLocationExternal; -import com.avaje.tests.singleTableInheritance.model.Zone; -import com.avaje.tests.singleTableInheritance.model.ZoneExternal; +import com.avaje.tests.singleTableInheritance.model.*; public class TestInheritQuery extends BaseTestCase { @Test public void test() { - ZoneExternal zone = new ZoneExternal(); zone.setAttribute("ABC"); Ebean.save(zone); @@ -42,4 +38,124 @@ public class TestInheritQuery extends BaseTestCase { Assert.assertNotNull(rereadZone); Assert.assertTrue(rereadZone instanceof ZoneExternal); } + + @Test + public void testDiscriminator_bug417() { + ZoneInternal zoneInt = new ZoneInternal(); + zoneInt.setAttribute("some zone 1"); + Ebean.save(zoneInt); + + ZoneExternal zoneExt = new ZoneExternal(); + zoneExt.setAttribute("some zone 2"); + Ebean.save(zoneExt); + + // queries of Zone and subclasses as root node of query + + // query abstract class on attribute (root of heirarchy) + List zones = Ebean.find(Zone.class).where().startsWith("attribute", "some zone").findList(); + // select t0.type c0, t0.ID c1, t0.attribute c2, t0.attribute c3 from zones t0 where t0.attribute like ? ; --bind(some zone%) + Assert.assertEquals(2, zones.size()); + Assert.assertTrue(zones.contains(zoneInt)); + Assert.assertTrue(zones.contains(zoneExt)); + + // query internal zones only + // discriminator is in WHERE clause where it belongs + List internalZones = Ebean.find(ZoneInternal.class).where().startsWith("attribute", "some zone").findList(); + // select t0.type c0, t0.ID c1, t0.attribute c2 from zones t0 where t0.type = 'INT' and t0.attribute like ? ; --bind(some zone%) + Assert.assertEquals(1, internalZones.size()); + Assert.assertTrue(internalZones.contains(zoneInt)); + Assert.assertFalse(internalZones.contains(zoneExt)); + Assert.assertTrue(internalZones.get(0) instanceof ZoneInternal); + + // query external zones only + List externalZones = Ebean.find(ZoneExternal.class).where().startsWith("attribute", "some zone").findList(); + // select t0.type c0, t0.ID c1, t0.attribute c2 from zones t0 where t0.type = 'EXT' and t0.attribute like ? ; --bind(some zone%) + Assert.assertEquals(1, externalZones.size()); + Assert.assertTrue(externalZones.contains(zoneExt)); + Assert.assertFalse(externalZones.contains(zoneInt)); + Assert.assertTrue(externalZones.get(0) instanceof ZoneExternal); + + // parents with children of Zones and subclasses + + Warehouse wh = new Warehouse(); + wh.setOfficeZone(zoneInt); // many-to-one + wh.getShippingZones().add(zoneExt); // many-to-many + Ebean.save(wh); + + // JOIN clause, no discriminator + // parent with many-to-one, doesn't put in discriminator, why not, PK sufficient? + // eager join + Warehouse wh2 = Ebean.find(Warehouse.class, wh.getId()); + // select t0.ID c0, t1.type c1, t0.officeZoneId c2 from warehouses t0 left outer join zones t1 on t1.ID = t0.officeZoneId where t0.ID = ? ; --bind(1) + Assert.assertNotNull(wh2); + Assert.assertEquals(wh.getId(), wh2.getId()); + Assert.assertEquals(wh.getOfficeZone(), wh2.getOfficeZone()); + Assert.assertEquals(wh.getOfficeZone().getAttribute(), wh2.getOfficeZone().getAttribute()); + + // before the fix, next assertion runs this lazy query: + + // select t0.ID c0, t1.type c1, t1.ID c2 from warehouses t0 + // left outer join WarehousesShippingZones t1z_ on t1z_.warehouseId = t0.ID + // left outer join zones t1 on t1.ID = t1z_.shippingZoneId + // where t1.type = 'EXT' // this should be in the join clause + // and t0.ID = ? + // order by t0.ID; --bind(1) + + // this works here because we have at least one shipping zone + + Assert.assertEquals(1, wh2.getShippingZones().size()); + Assert.assertTrue(wh2.getShippingZones().contains(zoneExt)); + + // set optional concrete to null to set stage for failure + wh.setOfficeZone(null); + Ebean.save(wh); + + // no discriminator here + wh2 = Ebean.find(Warehouse.class) + .where().eq("id", wh.getId()) + .findUnique(); + + Assert.assertNotNull(wh2); + // discriminator is used here, should be in join + // assuming this "manual" fetch is equivalent to autofetch (i.e., autofetch should work the same way) + // before Daryl's fix + // select t0.ID c0, t1.type c1, t1.ID c2, t1.attribute c3 from warehouses t0 left outer join zones t1 on t1.ID = t0.officeZoneId where t1.type = 'INT' and t0.ID = ? + // todo: after Daryl's fix, not sure if this is proper, no discriminator at all, isn't PK/FK sufficient? + // select t0.ID c0, t1.type c1, t1.ID c2, t1.attribute c3 from warehouses t0 left outer join zones t1 on t1.ID = t0.officeZoneId where t0.ID = ? ; --bind(1) + wh2 = Ebean.find(Warehouse.class) + .fetch("officeZone") + .where().eq("id", wh.getId()) + .findUnique(); + // key assertion #1 - fails due to left join with discriminator in WHERE + Assert.assertNotNull(wh2); + + // clear children to set the stage for left join failure + wh.getShippingZones().clear(); + Ebean.save(wh); + + wh2 = Ebean.find(Warehouse.class, wh.getId()); + Assert.assertNotNull(wh2); + Assert.assertEquals(wh.getId(), wh2.getId()); + Assert.assertEquals(0, wh.getShippingZones().size()); + + // query with lazy load of abstract children + wh = Ebean.find(Warehouse.class) + .where().eq("id", wh.getId()) + .findUnique(); + Assert.assertNotNull(wh); + Assert.assertEquals(wh.getId(), wh2.getId()); + Assert.assertEquals(0, wh.getShippingZones().size()); + + // query with fetch of abstract children + wh = Ebean.find(Warehouse.class) + .fetch("shippingZones") + .where().eq("id", wh.getId()) + .findUnique(); + // key assertion #2 - fails due to left join with discriminator in WHERE + Assert.assertNotNull(wh); + Assert.assertEquals(wh.getId(), wh2.getId()); + Assert.assertEquals(0, wh.getShippingZones().size()); + + + } } \ No newline at end of file diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/model/Warehouse.java b/src/test/java/com/avaje/tests/singleTableInheritance/model/Warehouse.java new file mode 100644 index 000000000..30d66b849 --- /dev/null +++ b/src/test/java/com/avaje/tests/singleTableInheritance/model/Warehouse.java @@ -0,0 +1,49 @@ +package com.avaje.tests.singleTableInheritance.model; + +import java.util.Set; + +import javax.persistence.*; + +@Entity +@Table(name="warehouses") +public class Warehouse { + @Id + @Column(name="ID") + private Integer id; + + @ManyToOne//(optional = false) //todo: should this be nullable with assertions made? + @JoinColumn(name = "officeZoneId") + private ZoneInternal officeZone; + + @ManyToMany(cascade = CascadeType.PERSIST) + @JoinTable(name = "WarehousesShippingZones", + joinColumns = { @JoinColumn(name = "warehouseId", referencedColumnName = "ID") }, + inverseJoinColumns = { @JoinColumn(name = "shippingZoneId", referencedColumnName = "ID") } + ) + private Set shippingZones; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public ZoneInternal getOfficeZone() { + return officeZone; + } + + public void setOfficeZone(ZoneInternal officeZone) { + this.officeZone = officeZone; + } + + public Set getShippingZones() { + return shippingZones; + } + + public void setShippingZones(Set shippingZones) { + this.shippingZones = shippingZones; + } + +} diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/model/Zone.java b/src/test/java/com/avaje/tests/singleTableInheritance/model/Zone.java index cd0b48f9e..9f0d002ce 100644 --- a/src/test/java/com/avaje/tests/singleTableInheritance/model/Zone.java +++ b/src/test/java/com/avaje/tests/singleTableInheritance/model/Zone.java @@ -28,4 +28,18 @@ public class Zone { this.id = id; } + + public int hashCode() { + if (getId() != null) return getId().hashCode(); + else return super.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof Zone) { + return ((Zone) obj).getId().equals(getId()); + } else { + return false; + } + } } diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneExternal.java b/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneExternal.java index 3b1fd1bd8..c3b24734e 100644 --- a/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneExternal.java +++ b/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneExternal.java @@ -18,4 +18,9 @@ public class ZoneExternal extends Zone { this.attribute = attribute; } + + @Override + public String toString() { + return "ZoneExternal " + getId() + " \"" + getAttribute() + "\""; + } } diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneInternal.java b/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneInternal.java new file mode 100644 index 000000000..b7e57bb5e --- /dev/null +++ b/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneInternal.java @@ -0,0 +1,24 @@ +package com.avaje.tests.singleTableInheritance.model; + +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; + +@Entity +@DiscriminatorValue("INT") +public class ZoneInternal extends Zone { + + private String attribute; + + public String getAttribute() { + return attribute; + } + + public void setAttribute(String attribute) { + this.attribute = attribute; + } + + @Override + public String toString() { + return "ZoneInternal " + getId() + " \"" + getAttribute() + "\""; + } +} From 0d3b1c625f1561b9f484b8aa2b3051bc80154543 Mon Sep 17 00:00:00 2001 From: Daryl Stultz Date: Fri, 10 May 2013 13:07:46 -0400 Subject: [PATCH 2/2] Expose order by columns omitted from select. --- .../com/avaje/tests/model/basic/MUser.java | 16 +++- .../avaje/tests/model/basic/MUserType.java | 42 ++++++++++ .../orderby/TestOrderByWithDistinct.java | 82 +++++++++++++++++++ 3 files changed, 136 insertions(+), 4 deletions(-) create mode 100644 src/test/java/com/avaje/tests/model/basic/MUserType.java create mode 100644 src/test/java/com/avaje/tests/query/orderby/TestOrderByWithDistinct.java diff --git a/src/test/java/com/avaje/tests/model/basic/MUser.java b/src/test/java/com/avaje/tests/model/basic/MUser.java index 1725ac692..9fee1f570 100644 --- a/src/test/java/com/avaje/tests/model/basic/MUser.java +++ b/src/test/java/com/avaje/tests/model/basic/MUser.java @@ -3,10 +3,7 @@ package com.avaje.tests.model.basic; import java.util.ArrayList; import java.util.List; -import javax.persistence.CascadeType; -import javax.persistence.Entity; -import javax.persistence.Id; -import javax.persistence.ManyToMany; +import javax.persistence.*; @Entity public class MUser { @@ -21,6 +18,9 @@ public class MUser { @ManyToMany(mappedBy="users",cascade=CascadeType.ALL) List roles; + @ManyToOne + private MUserType userType; + public MUser() { } @@ -59,5 +59,13 @@ public class MUser { } roles.add(role); } + + public MUserType getUserType() { + return userType; + } + + public void setUserType(MUserType userType) { + this.userType = userType; + } } diff --git a/src/test/java/com/avaje/tests/model/basic/MUserType.java b/src/test/java/com/avaje/tests/model/basic/MUserType.java new file mode 100644 index 000000000..6b499631a --- /dev/null +++ b/src/test/java/com/avaje/tests/model/basic/MUserType.java @@ -0,0 +1,42 @@ +package com.avaje.tests.model.basic; + +import javax.persistence.Entity; +import javax.persistence.Id; + +@Entity +public class MUserType { + + @Id + Integer id; + + String name; + + public MUserType() { + super(); + } + + public MUserType(String name) { + this.name = name; + } + + public Integer getId() { + return id; + } + + + public void setId(Integer id) { + this.id = id; + } + + + public String getName() { + return name; + } + + + public void setName(String name) { + this.name = name; + } + + +} diff --git a/src/test/java/com/avaje/tests/query/orderby/TestOrderByWithDistinct.java b/src/test/java/com/avaje/tests/query/orderby/TestOrderByWithDistinct.java new file mode 100644 index 000000000..ec579853e --- /dev/null +++ b/src/test/java/com/avaje/tests/query/orderby/TestOrderByWithDistinct.java @@ -0,0 +1,82 @@ +package com.avaje.tests.query.orderby; + +import java.util.List; + +import junit.framework.Assert; + +import org.junit.Test; + +import com.avaje.ebean.*; +import com.avaje.tests.model.basic.*; + + +public class TestOrderByWithDistinct extends BaseTestCase { + + @Test + public void test() { + /* + * Original conversation: + * https://groups.google.com/forum/?fromgroups=#!topic/ebean/uuvi1btdCDQ%5B1-25-false%5D + * + * This test exposes what may be a general problem with columns required by the order by phrase being omitted from the select. + * I'm not sure this exposes all causes of the problem. + */ + + MUserType ut = new MUserType("md"); + Ebean.save(ut); + MUser user1 = new MUser("one"); + user1.setUserType(ut); + Ebean.save(user1); + MUser user2 = new MUser("two"); + user2.setUserType(ut); + Ebean.save(user2); + + MRole roleA = new MRole("A"); + Ebean.save(roleA); + MRole roleB = new MRole("B"); + Ebean.save(roleB); + + user1.addRole(roleA); + Ebean.save(user1); + user2.addRole(roleB); + Ebean.save(user2); + + Query query = Ebean.find(MUser.class) + .where() + .eq("roles.roleName", "A") + .orderBy("userType.name, userName"); + List list = query.findList(); + + /* + * Produces this query: + select distinct t0.userid c0, t0.user_name c1, t0.user_type_id c2 + from muser t0 + join mrole_muser u1z_ on u1z_.muser_userid = t0.userid + join mrole u1 on u1.roleid = u1z_.mrole_roleid + left outer join muser_type t1 on t1.id = t0.user_type_id + where t1.name = ? and u1.role_name = ? + order by t1.name, t0.user_name + t1.name is not in select + */ + + Assert.assertEquals(1, list.size()); + Assert.assertEquals(user1, list.get(0)); + + // repeat with slight variation, not sure this really produces a different execution path + // this problem also manifests when autofetch eliminates properties from the select that aren't used in the objects + // still need them to be present for purpose of order by + // so here I'm "simulating" a scenario where autofetch has dropped userType.name + query = Ebean.find(MUser.class) + .setAutofetch(false) + .select("userName") + .where() + .eq("roles.roleName", "A") + .orderBy("userType.name"); + list = query.findList(); + + Assert.assertEquals(1, list.size()); + Assert.assertEquals(user1, list.get(0)); + + } + +}