From b44091ff3a4ce86b399434bd1d7595189e174dd2 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Thu, 21 Jan 2021 16:56:42 +1300 Subject: [PATCH] =?UTF-8?q?#2147=20Fix=20for=20ADD:=20multiple=20to=20many?= =?UTF-8?q?=20outer=20joins=20cause=20wrong=20count=20in=20distinct=20coun?= =?UTF-8?q?t=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copy of Rolands fix in FOCONIS branch. This also brings over some of the extra tests found there. Note that the SQL is slightly different from the FOCONIS branch in that there is additional foreign key columns included in the sub-query select clause. --- .../server/query/CQueryBuilder.java | 16 +- .../query/other/TestQuerySingleAttribute.java | 148 +++++++++++++++++- 2 files changed, 151 insertions(+), 13 deletions(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java index 765251134..75aa7e3cc 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java @@ -571,8 +571,8 @@ class CQueryBuilder { private final CQueryPredicates predicates; private final SqlTree select; private final boolean updateStatement; - private final boolean distinct; + private final boolean countSingleAttribute; private final String dbOrderBy; private boolean useSqlLimiter; private boolean hasWhere; @@ -590,6 +590,7 @@ class CQueryBuilder { this.updateStatement = updateStatement; this.distinct = query.isDistinct() || select.isSqlDistinct(); this.dbOrderBy = predicates.getDbOrderBy(); + this.countSingleAttribute = query.isCountDistinct() && query.isSingleAttribute(); } private void appendSelect() { @@ -601,8 +602,13 @@ class CQueryBuilder { if (!useSqlLimiter) { appendSelectDistinct(); } - if (query.isCountDistinct() && query.isSingleAttribute()) { - sb.append("r1.attribute_, count(*) from (select ").append(select.getSelectSql()).append(" as attribute_"); + if (countSingleAttribute) { + sb.append("r1.attribute_, count(*) from (select "); + if (distinct) { + sb.append("distinct t0."); + sb.append(request.getBeanDescriptor().getIdProperty().getDbColumn()).append(", "); + } + sb.append(select.getSelectSql()).append(" as attribute_"); } else { sb.append(select.getSelectSql()); } @@ -621,7 +627,7 @@ class CQueryBuilder { private void appendSelectDistinct() { sb.append("select "); - if (distinct) { + if (distinct && !countSingleAttribute) { if (request.isInlineCountDistinct()) { sb.append("count("); } @@ -730,7 +736,7 @@ class CQueryBuilder { sb.append(" order by ").append(dbOrderBy); } - if (query.isCountDistinct() && query.isSingleAttribute()) { + if (countSingleAttribute) { sb.append(") r1 group by r1.attribute_"); sb.append(toSql(query.getCountDistinctOrder())); } diff --git a/ebean-core/src/test/java/org/tests/query/other/TestQuerySingleAttribute.java b/ebean-core/src/test/java/org/tests/query/other/TestQuerySingleAttribute.java index a5cd5d481..f476ed084 100644 --- a/ebean-core/src/test/java/org/tests/query/other/TestQuerySingleAttribute.java +++ b/ebean-core/src/test/java/org/tests/query/other/TestQuerySingleAttribute.java @@ -6,16 +6,21 @@ import io.ebean.CountedValue; import io.ebean.DB; import io.ebean.Ebean; import io.ebean.Query; +import org.junit.After; +import org.junit.Before; import org.junit.Ignore; import org.junit.Test; import org.tests.inherit.ChildA; import org.tests.inherit.Data; import org.tests.inherit.EUncle; +import org.tests.lazyforeignkeys.MainEntity; +import org.tests.lazyforeignkeys.MainEntityRelation; 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 org.tests.model.basic.VwCustomer; +import org.tests.o2m.OmBasicParent; import java.sql.Date; import java.time.LocalDate; @@ -24,7 +29,7 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; public class TestQuerySingleAttribute extends BaseTestCase { - + @Test public void findSingleAttributesTwoToMany() { ResetBasicData.reset(); @@ -39,7 +44,7 @@ public class TestQuerySingleAttribute extends BaseTestCase { CountedValue robs0 = (CountedValue) query0.findSingleAttributeList().get(0); assertThat(robs0.getValue()).isEqualTo("Rob"); assertThat(robs0.getCount()).isEqualTo(1); - + // Query with or with equals causing joins Query query = DB.find(Customer.class) .select("name") @@ -56,12 +61,13 @@ public class TestQuerySingleAttribute extends BaseTestCase { assertThat(robs.getValue()).isEqualTo("Rob"); // only one Customer named rob exists, but 7 is returned for the amount of Customers named Rob assertThat(robs.getCount()).isEqualTo(1); - - // TODO check correct future query - assertThat(sqlOf(query)).contains("select r1.attribute_1, count(*) cnt" - + " from (select t1.id attribute_1 from main_entity_relation t0 left join main_entity t1 on t1.id = t0.id1 ) r1" - + " group by r1.attribute_1" - + " order by count(*) desc, r1.attribute_1"); + + assertThat(sqlOf(query)).contains("select r1.attribute_, count(*) " + + "from (select distinct t0.id, t0.name as attribute_ " + + "from o_customer t0 left join contact u1 on u1.customer_id = t0.id left join o_order u2 on u2.kcustomer_id = t0.id " + + "where t0.name = ? and (u2.status = ? or u1.first_name = ?)) r1 " + + "group by r1.attribute_ " + + "order by count(*) desc, r1.attribute_"); } @Test @@ -142,6 +148,93 @@ public class TestQuerySingleAttribute extends BaseTestCase { assertThat(name).isNotNull(); } + @Test + public void findSingleAttributeList_with_join_column() { + ResetBasicData.reset(); + Query query = Ebean.find(MainEntityRelation.class) + .fetch("entity1", "attr1") + .setDistinct(true) + .setCountDistinct(CountDistinctOrder.COUNT_DESC_ATTR_ASC) + .where().query(); + + List> attr1list = query.findSingleAttributeList(); + + assertThat(sqlOf(query)).contains("select r1.attribute_, count(*)" + + " from (select distinct t0.id, t0.id1, t1.attr1 as attribute_ from main_entity_relation t0 left join main_entity t1 on t1.id = t0.id1) r1" + + " group by r1.attribute_" + + " order by count(*) desc, r1.attribute_"); // sub-query select clause includes t0.id1 + assertThat(attr1list).isNotNull(); + assertThat(attr1list).hasSize(2); + assertThat(attr1list.get(0).getValue()).isEqualTo("a1"); + assertThat(attr1list.get(0).getCount()).isEqualTo(2l); + assertThat(attr1list.get(1).getValue()).isEqualTo("a2"); + assertThat(attr1list.get(1).getCount()).isEqualTo(1l); + } + + @Test + public void findSingleAttributesVariousSelection1() { + Query query = Ebean.find(MainEntityRelation.class) + .fetch("entity1", "attr1") + .setCountDistinct(CountDistinctOrder.COUNT_DESC_ATTR_ASC) + .where().query(); + query.findSingleAttributeList(); + assertThat(sqlOf(query)).contains("select r1.attribute_, count(*)" + + " from (select t0.id1, t1.attr1 as attribute_ from main_entity_relation t0 left join main_entity t1 on t1.id = t0.id1) r1" + + " group by r1.attribute_" + + " order by count(*) desc, r1.attribute_"); // sub-query select clause includes t0.id1 + } + + @Test + public void findSingleAttributesVariousSelection2() { + Query query = Ebean.find(MainEntityRelation.class) + .select("attr1") + .setCountDistinct(CountDistinctOrder.COUNT_DESC_ATTR_ASC) + .where().query(); + query.findSingleAttributeList(); + assertThat(sqlOf(query)).contains("select r1.attribute_, count(*)" + + " from (select t0.attr1 as attribute_ from main_entity_relation t0) r1" + + " group by r1.attribute_" + + " order by count(*) desc, r1.attribute_"); + } + + @Test + public void findSingleAttributesVariousSelection3() { + Query query = Ebean.find(MainEntityRelation.class) + .select("id") + .setCountDistinct(CountDistinctOrder.COUNT_DESC_ATTR_ASC) + .where().query(); + query.findSingleAttributeList(); + assertThat(sqlOf(query)).contains("select r1.attribute_, count(*)" + + " from (select t0.id as attribute_ from main_entity_relation t0) r1" + + " group by r1.attribute_" + + " order by count(*) desc, r1.attribute_"); + } + + @Test + public void findSingleAttributesVariousSelection4() { + Query query = Ebean.find(MainEntityRelation.class) + .fetch("entity1", "id") + .setCountDistinct(CountDistinctOrder.COUNT_DESC_ATTR_ASC) + .where().query(); + query.findSingleAttributeList(); + assertThat(sqlOf(query)).contains("select r1.attribute_, count(*)" + + " from (select t0.id1, t1.id as attribute_ from main_entity_relation t0 left join main_entity t1 on t1.id = t0.id1) r1" + + " group by r1.attribute_" + + " order by count(*) desc, r1.attribute_"); // sub-query select clause includes t0.id1, + } + + @Test + public void findSingleAttributesVariousSelection5() { + Query query = Ebean.find(OmBasicParent.class) + .fetch("children", "name") + .setCountDistinct(CountDistinctOrder.COUNT_DESC_ATTR_ASC) + .where().query(); + query.findSingleAttributeList(); + assertThat(sqlOf(query)).contains("select r1.attribute_, count(*)" + + " from (select t1.id, t1.name as attribute_ from om_basic_parent t0 left join om_basic_child t1 on t1.parent_id = t0.id) r1 " + + "group by r1.attribute_ order by count(*) desc, r1.attribute_"); // sub-query select clause includes t1.id, + } + @Test public void findSingleAttribute_with_aggregate() { @@ -664,4 +757,43 @@ public class TestQuerySingleAttribute extends BaseTestCase { System.out.println(" count:" + entry.getCount()+" orderStatus:" + entry.getValue() ); } } + + @Before + public void setup() { + MainEntity e1 = new MainEntity(); + e1.setId("1"); + e1.setAttr1("a1"); + DB.save(e1); + + MainEntity e2 = new MainEntity(); + e2.setId("2"); + e2.setAttr1("a2"); + DB.save(e2); + + MainEntity e3 = new MainEntity(); + e3.setId("3"); + e3.setAttr1("a1"); + DB.save(e3); + + MainEntityRelation rel = new MainEntityRelation(); + rel.setEntity1(e1); + rel.setEntity2(e1); + DB.save(rel); + + rel = new MainEntityRelation(); + rel.setEntity1(e2); + rel.setEntity2(e2); + DB.save(rel); + + rel = new MainEntityRelation(); + rel.setEntity1(e3); + rel.setEntity2(e3); + DB.save(rel); + } + + @After + public void cleanup() { + Ebean.find(MainEntityRelation.class).delete(); + Ebean.find(MainEntity.class).delete(); + } }