From 5544a2cc3e8dc67e4649e1ab13a6249155ad3104 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Fri, 12 Jul 2019 12:59:19 +0200 Subject: [PATCH] Possible fix --- .../io/ebeaninternal/api/ManyWhereJoins.java | 21 ++-- .../server/el/ElPropertyChain.java | 3 +- .../server/query/CQueryBuilder.java | 4 +- .../server/query/SqlTreeBuilder.java | 7 ++ .../query/SqlTreeNodeFormulaWhereJoin.java | 115 ++++++++++++++++++ .../query/joins/TestQueryJoinOnFormula.java | 114 ++++++++--------- 6 files changed, 194 insertions(+), 70 deletions(-) create mode 100644 src/main/java/io/ebeaninternal/server/query/SqlTreeNodeFormulaWhereJoin.java diff --git a/src/main/java/io/ebeaninternal/api/ManyWhereJoins.java b/src/main/java/io/ebeaninternal/api/ManyWhereJoins.java index 0b6499fec..0c6b128a2 100644 --- a/src/main/java/io/ebeaninternal/api/ManyWhereJoins.java +++ b/src/main/java/io/ebeaninternal/api/ManyWhereJoins.java @@ -7,7 +7,9 @@ import io.ebean.util.SplitName; import io.ebeaninternal.server.query.SqlJoinType; import java.io.Serializable; +import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.TreeMap; import java.util.TreeSet; @@ -21,9 +23,7 @@ public class ManyWhereJoins implements Serializable { private final TreeMap joins = new TreeMap<>(); - private StringBuilder formulaProperties = new StringBuilder(); - - private boolean formulaWithJoin; + private List formulaJoinProperties; private boolean aggregation; @@ -125,27 +125,24 @@ public class ManyWhereJoins implements Serializable { * specifically for the findCount query. */ public void addFormulaWithJoin(String propertyName) { - if (formulaWithJoin) { - formulaProperties.append(","); - } else { - formulaProperties = new StringBuilder(); - formulaWithJoin = true; + if (formulaJoinProperties == null) { + formulaJoinProperties = new ArrayList<>(); } - formulaProperties.append(propertyName); + formulaJoinProperties.add(propertyName); } /** * Return true if the query select includes a formula with join. */ public boolean isFormulaWithJoin() { - return formulaWithJoin; + return formulaJoinProperties != null; } /** * Return the formula properties to build the select clause for a findCount query. */ - public String getFormulaProperties() { - return formulaProperties.toString(); + public List getFormulaJoinProperties() { + return formulaJoinProperties; } /** diff --git a/src/main/java/io/ebeaninternal/server/el/ElPropertyChain.java b/src/main/java/io/ebeaninternal/server/el/ElPropertyChain.java index ee2732cbb..a7d0bd779 100644 --- a/src/main/java/io/ebeaninternal/server/el/ElPropertyChain.java +++ b/src/main/java/io/ebeaninternal/server/el/ElPropertyChain.java @@ -136,8 +136,7 @@ public class ElPropertyChain implements ElPropertyValue { @Override public boolean containsFormulaWithJoin() { - // Not cascading the check at this stage - return false; + return lastBeanProperty.containsFormulaWithJoin(); } @Override diff --git a/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java b/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java index 2e1511548..d73458791 100644 --- a/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java +++ b/src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java @@ -265,7 +265,9 @@ class CQueryBuilder { if (!countDistinct) { // minimise select clause for standard count if (manyWhereJoins.isFormulaWithJoin()) { - query.select(manyWhereJoins.getFormulaProperties()); + // FIXME: we join the strings here and on the other side we split them again + // this is not yet optimal + query.select(String.join(",",manyWhereJoins.getFormulaJoinProperties())); } else { query.setSelectId(); } diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java index 7b7530dfd..6983de938 100644 --- a/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java @@ -296,6 +296,13 @@ public final class SqlTreeBuilder { SqlTreeNodeManyWhereJoin nodeJoin = new SqlTreeNodeManyWhereJoin(joinProp.getProperty(), beanProperty, joinProp.getSqlJoinType()); myJoinList.add(nodeJoin); } + if (manyWhereJoins.isFormulaWithJoin()) { + for (String property: manyWhereJoins.getFormulaJoinProperties()) { + STreeProperty beanProperty = desc.findPropertyFromPath(property); + SqlTreeNodeFormulaWhereJoin nodeJoin = new SqlTreeNodeFormulaWhereJoin(beanProperty, SqlJoinType.OUTER); + myJoinList.add(nodeJoin); + } + } } private SqlTreeNode buildNode(String prefix, STreePropertyAssoc prop, STreeType desc, List myList, SqlTreeProperties props) { diff --git a/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeFormulaWhereJoin.java b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeFormulaWhereJoin.java new file mode 100644 index 000000000..4ec6708b9 --- /dev/null +++ b/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeFormulaWhereJoin.java @@ -0,0 +1,115 @@ +package io.ebeaninternal.server.query; + +import io.ebean.Version; +import io.ebean.bean.EntityBean; +import io.ebeaninternal.api.SpiQuery; +import io.ebeaninternal.server.deploy.DbReadContext; +import io.ebeaninternal.server.deploy.DbSqlContext; +import io.ebeaninternal.server.type.ScalarType; + +import java.util.List; +import java.util.Set; + +/** + * Join to Many (or child of a many) to support where clause predicates on many properties. + */ +class SqlTreeNodeFormulaWhereJoin implements SqlTreeNode { + + private final STreeProperty nodeBeanProp; + + /** + * The many where join which is either INNER or OUTER. + */ + private final SqlJoinType manyJoinType; + + SqlTreeNodeFormulaWhereJoin(STreeProperty prop, SqlJoinType manyJoinType) { + this.nodeBeanProp = prop; + this.manyJoinType = manyJoinType; + } + + @Override + public boolean isSingleProperty() { + return true; + } + + @Override + public ScalarType getSingleAttributeReader() { + throw new IllegalStateException("No expected"); + } + + @Override + public void addAsOfTableAlias(SpiQuery query) { + // do nothing here ... + } + + @Override + public void addSoftDeletePredicate(SpiQuery query) { + // do nothing here ... + } + + @Override + public boolean isAggregation() { + return false; + } + + @Override + public void appendDistinctOn(DbSqlContext ctx, boolean subQuery) { + // do nothing here ... + } + + @Override + public void appendGroupBy(DbSqlContext ctx, boolean subQuery) { + // do nothing here + } + + /** + * Append to the FROM clause for this node. + */ + @Override + public void appendFrom(DbSqlContext ctx, SqlJoinType currentJoinType) { + + // always use the join type as per this many where join + // (OUTER for disjunction and otherwise INNER) + nodeBeanProp.appendFrom(ctx, manyJoinType); + } + + + + @Override + public void dependentTables(Set tables) { + //FIXME: we cannot easily determine the dependent tables, this would require an enhancement + //of the @Formula(dependentTables=...) annotation + } + + @Override + public void buildRawSqlSelectChain(List selectChain) { + // nothing to add + } + + @Override + public void appendSelect(DbSqlContext ctx, boolean subQuery) { + // nothing to do here + } + + @Override + public void appendWhere(DbSqlContext ctx) { + // nothing to do here + } + + @Override + public EntityBean load(DbReadContext ctx, EntityBean localBean, EntityBean parentBean) { + // nothing to do here + return null; + } + + @Override + public Version loadVersion(DbReadContext ctx) { + // nothing to do here + return null; + } + + @Override + public boolean hasMany() { + return true; + } +} diff --git a/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java b/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java index e68561c49..3ce0e6051 100644 --- a/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java +++ b/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java @@ -2,13 +2,11 @@ package org.tests.query.joins; import io.ebean.BaseTestCase; import io.ebean.Ebean; -import io.ebean.FetchConfig; -import io.ebean.Query; import org.tests.model.basic.Order; import org.tests.model.basic.ResetBasicData; +import org.tests.model.family.ChildPerson; import org.tests.model.family.ParentPerson; import org.ebeantest.LoggedSqlCollector; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -20,81 +18,55 @@ import static org.junit.Assert.assertEquals; public class TestQueryJoinOnFormula extends BaseTestCase { - + @Before public void init() { ResetBasicData.reset(); } - - /** - * If there is no query.select() or query.fetch() in the query, there should be a meaningful exception. - */ - @Test - public void test_OrderFindIdsNoSelect() { - assertThatThrownBy(() -> { - Ebean.find(Order.class) - .where().eq("totalItems", 3) - .findIds(); - }).hasMessageContaining("property 'totalItems' has to be selected explicitly"); - } - - /** - * If there is no query.select() or query.fetch() in the query, there should be a meaningful exception. - */ - @Test - public void test_OrderFindListNoSelect() { - - assertThatThrownBy(() -> { - Ebean.find(Order.class) - .where().eq("totalItems", 3) - .findList(); - }).hasMessageContaining("property 'totalItems' has to be selected explicitly"); - } - @Test public void test_OrderFindIds() { LoggedSqlCollector.start(); List orderIds = Ebean.find(Order.class) - .select("totalItems") .where().eq("totalItems", 3) .findIds(); assertThat(orderIds).hasSize(2); - + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); + assertThat(loggedSql.get(0)).contains("join (select order_id, count(*) as total_items,"); } - + @Test public void test_OrderFindList() { LoggedSqlCollector.start(); List orders = Ebean.find(Order.class) - .select("totalItems") .where().eq("totalItems", 3) .findList(); assertThat(orders).hasSize(2); - + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); + assertThat(loggedSql.get(0)).contains("join (select order_id, count(*) as total_items,"); } - + @Test public void test_OrderFindCount() { LoggedSqlCollector.start(); int orders = Ebean.find(Order.class) - .select("totalItems") .where().eq("totalItems", 3) .findCount(); assertThat(orders).isEqualTo(2); - + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); + assertThat(loggedSql.get(0)).contains("join (select order_id, count(*) as total_items,"); } @Test @@ -103,15 +75,16 @@ public class TestQueryJoinOnFormula extends BaseTestCase { LoggedSqlCollector.start(); List orderDates = Ebean.find(Order.class) - .select("orderDate,totalItems") + .select("orderDate") .where().eq("totalItems", 3) .findSingleAttributeList(); assertThat(orderDates).hasSize(2); - + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); + assertThat(loggedSql.get(0)).contains("join (select order_id, count(*) as total_items,"); } - + @Test public void test_OrderFindOne() { @@ -123,13 +96,14 @@ public class TestQueryJoinOnFormula extends BaseTestCase { .setMaxRows(1) .orderById(true) .findOne(); - + assertThat(order.getTotalItems()).isEqualTo(3); - + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); + assertThat(loggedSql.get(0)).contains("join (select order_id, count(*) as total_items,"); } - + @Test public void test_ParentPersonFindIds() { @@ -138,26 +112,28 @@ public class TestQueryJoinOnFormula extends BaseTestCase { List orderIds = Ebean.find(ParentPerson.class) .where().eq("totalAge", 3) .findIds(); - assertThat(orderIds).hasSize(2); - + // TODO: There are no beans in database, so for now only the query must run. + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); } - + @Test public void test_ParentPersonFindList() { LoggedSqlCollector.start(); Ebean.find(ParentPerson.class) - .where().eq("totalAge", 3) + .select("identifier") + //.where().eq("totalAge", 3) + .where().eq("familyName", "foo") .findList(); // TODO: There are no beans in database, so for now only the query must run. - + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); } - + @Test public void test_ParentPersonFindCount() { @@ -167,11 +143,11 @@ public class TestQueryJoinOnFormula extends BaseTestCase { .where().eq("totalAge", 3) .findCount(); // TODO: There are no beans in database, so for now only the query must run. - + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); } - + @Test public void test_ParentPersonFindSingleAttributeList() { @@ -182,11 +158,11 @@ public class TestQueryJoinOnFormula extends BaseTestCase { .where().eq("totalAge", 3) .findSingleAttributeList(); // TODO: There are no beans in database, so for now only the query must run. - + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); } - + @Test public void test_ParentPersonFindOne() { @@ -198,7 +174,35 @@ public class TestQueryJoinOnFormula extends BaseTestCase { .orderById(true) .findOne(); // TODO: There are no beans in database, so for now only the query must run. - + + List loggedSql = LoggedSqlCollector.stop(); + assertEquals(1, loggedSql.size()); + } + + @Test + public void test_ChildPersonParentFindIds() { + + LoggedSqlCollector.start(); + + Ebean.find(ChildPerson.class) + .where().eq("parent.totalAge", 3) + .findIds(); + // TODO: There are no beans in database, so for now only the query must run. + + List loggedSql = LoggedSqlCollector.stop(); + assertEquals(1, loggedSql.size()); + } + + @Test + public void test_ChildPersonParentFindCount() { + + LoggedSqlCollector.start(); + + Ebean.find(ChildPerson.class) + .where().eq("parent.totalAge", 3) + .findCount(); + // TODO: There are no beans in database, so for now only the query must run. + List loggedSql = LoggedSqlCollector.stop(); assertEquals(1, loggedSql.size()); }