From 73452a5f687ee989a513062ce8d1f514c3fbbd1c Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Thu, 16 Jan 2014 20:36:23 +1300 Subject: [PATCH] Fix for issue 68 - findRowCount sql error when using a @Formula property in the where clause --- .../ebeaninternal/api/ManyWhereJoins.java | 36 +++++++++++++++++++ .../server/deploy/BeanFkeyProperty.java | 5 +++ .../server/deploy/BeanProperty.java | 6 ++++ .../server/el/ElPropertyChain.java | 8 ++++- .../server/el/ElPropertyDeploy.java | 5 +++ .../server/expression/AbstractExpression.java | 13 +++++-- .../server/query/CQueryBuilder.java | 14 ++++++-- .../type/CtCompoundPropertyElAdapter.java | 5 +++ .../query/other/TestFormulaWithFindCount.java | 34 ++++++++++++++++++ 9 files changed, 119 insertions(+), 7 deletions(-) create mode 100644 src/test/java/com/avaje/tests/query/other/TestFormulaWithFindCount.java diff --git a/src/main/java/com/avaje/ebeaninternal/api/ManyWhereJoins.java b/src/main/java/com/avaje/ebeaninternal/api/ManyWhereJoins.java index 4c27f49be..b00cdb123 100644 --- a/src/main/java/com/avaje/ebeaninternal/api/ManyWhereJoins.java +++ b/src/main/java/com/avaje/ebeaninternal/api/ManyWhereJoins.java @@ -19,6 +19,10 @@ public class ManyWhereJoins implements Serializable { private final TreeSet joins = new TreeSet(); + private StringBuilder formulaProperties = new StringBuilder(); + + private boolean formulaWithJoin; + /** * Add a many where join. */ @@ -73,4 +77,36 @@ public class ManyWhereJoins implements Serializable { return joins; } + /** + * In findRowCount query found a formula property with a join clause so building a select clause + * specifically for the findRowCount query. + */ + public void addFormulaWithJoin(String propertyName) { + if (formulaWithJoin) { + formulaProperties.append(","); + } else { + formulaProperties = new StringBuilder(); + formulaWithJoin = true; + } + formulaProperties.append(propertyName); + } + + public boolean isHasMany() { + return formulaWithJoin || !joins.isEmpty(); + } + + /** + * Return true if the findRowCount query just needs the id property in the select clause. + */ + public boolean isSelectId() { + return !formulaWithJoin; + } + + /** + * Return the formula properties to build the select clause for a findRowCount query. + */ + public String getFormulaProperties() { + return formulaProperties.toString(); + } + } diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanFkeyProperty.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanFkeyProperty.java index 1c5742a46..bd85b6d56 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanFkeyProperty.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanFkeyProperty.java @@ -63,6 +63,11 @@ public final class BeanFkeyProperty implements ElPropertyValue { public boolean isDeployOnly() { return true; } + + @Override + public boolean containsFormulaWithJoin() { + return false; + } /** * Returns false. diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanProperty.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanProperty.java index 51c648bd2..53dfd4220 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanProperty.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanProperty.java @@ -837,6 +837,12 @@ public class BeanProperty implements ElPropertyValue { return false; } + + @Override + public boolean containsFormulaWithJoin() { + return formula && sqlFormulaJoin != null; + } + public boolean containsManySince(String sinceProperty) { return containsMany(); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyChain.java b/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyChain.java index e81a296d1..877ddb02d 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyChain.java +++ b/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyChain.java @@ -122,7 +122,13 @@ public class ElPropertyChain implements ElPropertyValue { return false; } - public boolean containsMany() { + @Override + public boolean containsFormulaWithJoin() { + // Not cascading the check at this stage + return false; + } + + public boolean containsMany() { return containsMany; } diff --git a/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyDeploy.java b/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyDeploy.java index a544612bb..128346f45 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyDeploy.java +++ b/src/main/java/com/avaje/ebeaninternal/server/el/ElPropertyDeploy.java @@ -17,6 +17,11 @@ public interface ElPropertyDeploy { */ public static final String ROOT_ELPREFIX = "${}"; + /** + * Return true if the property is a formula with a join clause. + */ + public boolean containsFormulaWithJoin(); + /** * Return true if there is a property on the path that is a many property. */ diff --git a/src/main/java/com/avaje/ebeaninternal/server/expression/AbstractExpression.java b/src/main/java/com/avaje/ebeaninternal/server/expression/AbstractExpression.java index a1c74a641..9059bb8db 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/expression/AbstractExpression.java +++ b/src/main/java/com/avaje/ebeaninternal/server/expression/AbstractExpression.java @@ -40,11 +40,18 @@ public abstract class AbstractExpression implements SpiExpression { public void containsMany(BeanDescriptor desc, ManyWhereJoins manyWhereJoin) { - String propertyName = getPropertyName(); + String propertyName = getPropertyName(); if (propertyName != null){ ElPropertyDeploy elProp = desc.getElPropertyDeploy(propertyName); - if (elProp != null && elProp.containsMany()){ - manyWhereJoin.add(elProp); + if (elProp != null) { + if (elProp.containsFormulaWithJoin()) { + // for findRowCount query select clause + manyWhereJoin.addFormulaWithJoin(propertyName); + } + if (elProp.containsMany()){ + // for findRowCount we join to a many property + manyWhereJoin.add(elProp); + } } } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryBuilder.java b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryBuilder.java index c182cfc56..9f6241cb8 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/CQueryBuilder.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/CQueryBuilder.java @@ -16,6 +16,7 @@ import com.avaje.ebean.config.dbplatform.SqlLimitRequest; import com.avaje.ebean.config.dbplatform.SqlLimitResponse; import com.avaje.ebean.config.dbplatform.SqlLimiter; import com.avaje.ebean.text.PathProperties; +import com.avaje.ebeaninternal.api.ManyWhereJoins; import com.avaje.ebeaninternal.api.SpiQuery; import com.avaje.ebeaninternal.server.core.OrmQueryRequest; import com.avaje.ebeaninternal.server.deploy.BeanDescriptor; @@ -130,9 +131,16 @@ public class CQueryBuilder implements Constants { // always set the order by to null for row count query query.setOrder(null); - boolean hasMany = !query.getManyWhereJoins().isEmpty(); - - query.setSelectId(); + ManyWhereJoins manyWhereJoins = query.getManyWhereJoins(); + + boolean hasMany = manyWhereJoins.isHasMany(); + if (manyWhereJoins.isSelectId()) { + // just select the id property + query.setSelectId(); + } else { + // select the id and the required formula properties + query.select(manyWhereJoins.getFormulaProperties()); + } String sqlSelect = "select count(*)"; if (hasMany) { diff --git a/src/main/java/com/avaje/ebeaninternal/server/type/CtCompoundPropertyElAdapter.java b/src/main/java/com/avaje/ebeaninternal/server/type/CtCompoundPropertyElAdapter.java index 3a07f8e1d..3d5a6f1f9 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/type/CtCompoundPropertyElAdapter.java +++ b/src/main/java/com/avaje/ebeaninternal/server/type/CtCompoundPropertyElAdapter.java @@ -108,6 +108,11 @@ public class CtCompoundPropertyElAdapter implements ElPropertyValue { throw new RuntimeException("Not Supported or Expected"); } + @Override + public boolean containsFormulaWithJoin() { + return false; + } + public boolean containsMany() { return false; } diff --git a/src/test/java/com/avaje/tests/query/other/TestFormulaWithFindCount.java b/src/test/java/com/avaje/tests/query/other/TestFormulaWithFindCount.java new file mode 100644 index 000000000..e70c52d58 --- /dev/null +++ b/src/test/java/com/avaje/tests/query/other/TestFormulaWithFindCount.java @@ -0,0 +1,34 @@ +package com.avaje.tests.query.other; + +import java.util.List; + +import junit.framework.Assert; + +import org.junit.Test; + +import com.avaje.ebean.BaseTestCase; +import com.avaje.ebean.Ebean; +import com.avaje.ebean.EbeanServer; +import com.avaje.ebean.ExpressionList; +import com.avaje.tests.model.basic.Order; +import com.avaje.tests.model.basic.ResetBasicData; + +public class TestFormulaWithFindCount extends BaseTestCase { + + @Test + public void testFindCount() { + + ResetBasicData.reset(); + + EbeanServer server = Ebean.getServer(null); + + + ExpressionList ex = server.find(Order.class).select("id, status ,totalAmount").where().gt("totalAmount", 1d); + List list = ex.findList(); + + ExpressionList expressionList = server.find(Order.class).where().gt("totalAmount", 1d); + int rowCount = expressionList.findRowCount(); + Assert.assertEquals(list.size(), rowCount); + } + +}