diff --git a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java index 7df2185b3..490e53762 100644 --- a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java +++ b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java @@ -222,7 +222,7 @@ public final class OrmQueryRequest extends BeanRequest implements SpiOrmQuery if (query.isRawSql()) { return new DeployPropertyParserMap(query.getRawSql().getColumnMapping().getMapping()); } else { - return beanDescriptor.createDeployPropertyParser(); + return beanDescriptor.parser(); } } diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java index 2552d2600..41ba76d32 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java @@ -1582,7 +1582,7 @@ public class BeanDescriptor implements BeanType { } } - public DeployPropertyParser createDeployPropertyParser() { + public DeployPropertyParser parser() { return new DeployPropertyParser(this); } @@ -2453,18 +2453,7 @@ public class BeanDescriptor implements BeanType { */ private SqlTreeProperty findSqlTreeFormula(String formulaExpression) { - return dynamicProperty.computeIfAbsent(formulaExpression, (formula) -> { - FormulaPropertyPath propertyFormula = new FormulaPropertyPath(formula); - if (!propertyFormula.isFormula()) { - throw new IllegalStateException("unable to parse formula [" + formula + "} on bean type " + fullName); - } - String baseName = propertyFormula.basePropertyName(); - BeanProperty base = _findBeanProperty(baseName); - if (base == null) { - throw new IllegalStateException("unable to find property [" + baseName + "] from formula [" + formula + "} on bean type " + fullName); - } - return propertyFormula.formulaProperty(base); - }); + return dynamicProperty.computeIfAbsent(formulaExpression, (formula) -> new FormulaPropertyPath(this, formula).build()); } /** @@ -2497,7 +2486,7 @@ public class BeanDescriptor implements BeanType { return _findBeanProperty(propName); } - private BeanProperty _findBeanProperty(String propName) { + BeanProperty _findBeanProperty(String propName) { BeanProperty prop = propMap.get(propName); if (prop == null && inheritInfo != null) { // search in sub types... diff --git a/src/main/java/io/ebeaninternal/server/deploy/DeployPropertyParser.java b/src/main/java/io/ebeaninternal/server/deploy/DeployPropertyParser.java index 320d76a96..bf98172a2 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/DeployPropertyParser.java +++ b/src/main/java/io/ebeaninternal/server/deploy/DeployPropertyParser.java @@ -22,10 +22,29 @@ public final class DeployPropertyParser extends DeployParser { private final Set includes = new HashSet<>(); + private boolean catchFirst; + + private ElPropertyDeploy firstProp; + DeployPropertyParser(BeanDescriptor beanDescriptor) { this.beanDescriptor = beanDescriptor; } + /** + * Set to true to catch the first property. + */ + public DeployPropertyParser setCatchFirst(boolean catchFirst) { + this.catchFirst = catchFirst; + return this; + } + + /** + * Return the first property found by the parser. + */ + public ElPropertyDeploy getFirstProp() { + return firstProp; + } + @Override public Set getIncludes() { return includes; @@ -45,6 +64,9 @@ public final class DeployPropertyParser extends DeployParser { if (elProp == null) { return null; } else { + if (catchFirst && firstProp == null) { + firstProp = elProp; + } addIncludes(elProp.getElPrefix()); return elProp.getElPlaceholder(encrypted); } diff --git a/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormula.java b/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormula.java index e7f55721e..28f494bd4 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormula.java +++ b/src/main/java/io/ebeaninternal/server/deploy/DynamicPropertyAggregationFormula.java @@ -10,24 +10,27 @@ import javax.persistence.PersistenceException; */ class DynamicPropertyAggregationFormula extends DynamicPropertyBase { - private final String parsedAggregation; + private final String parsedFormula; + + private final boolean aggregate; private final BeanProperty asTarget; - DynamicPropertyAggregationFormula(String name, ScalarType scalarType, String parsedAggregation, BeanProperty asTarget) { + DynamicPropertyAggregationFormula(String name, ScalarType scalarType, String parsedFormula, boolean aggregate, BeanProperty asTarget) { super(name, name, null, scalarType); - this.parsedAggregation = parsedAggregation; + this.parsedFormula = parsedFormula; + this.aggregate = aggregate; this.asTarget = asTarget; } @Override public String toString() { - return "DynamicPropertyFormula[" + parsedAggregation + "]"; + return "DynamicPropertyFormula[" + parsedFormula + "]"; } @Override public boolean isAggregation() { - return true; + return aggregate; } @Override @@ -46,7 +49,7 @@ class DynamicPropertyAggregationFormula extends DynamicPropertyBase { @Override public void appendSelect(DbSqlContext ctx, boolean subQuery) { - ctx.appendParseSelect(parsedAggregation); + ctx.appendParseSelect(parsedFormula); } } diff --git a/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java b/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java index bb350f536..b4c82f790 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java +++ b/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java @@ -1,5 +1,6 @@ package io.ebeaninternal.server.deploy; +import io.ebeaninternal.server.el.ElPropertyDeploy; import io.ebeaninternal.server.query.SqlTreeProperty; import io.ebeaninternal.server.type.ScalarType; @@ -9,27 +10,34 @@ import java.util.regex.Pattern; class FormulaPropertyPath { - private static final Pattern pattern = Pattern.compile("(max|min|avg|count)\\((.*)\\)"); + private static final String[] AGG_FUNCTIONS = {"count","max","min","avg"}; + + private static final Pattern pattern = Pattern.compile("([a-zA-Z]*)\\((.*)\\)"); private static final String DISTINCT_ = "distinct "; - private final String aggType; + private final BeanDescriptor descriptor; - private final String baseName; + private final String formula; + + private final String outerFunction; + + private final String internalExpression; private boolean countDistinct; - FormulaPropertyPath(String propName) { + FormulaPropertyPath(BeanDescriptor descriptor, String formula) { - Matcher matcher = pattern.matcher(propName); - if (matcher.find()) { - aggType = matcher.group(1); - baseName = trimDistinct(matcher.group(2)); + this.descriptor = descriptor; + this.formula = formula; - } else { - aggType = null; - baseName = null; + Matcher matcher = pattern.matcher(formula); + if (!matcher.find()) { + throw new IllegalStateException("Unable to parse formula [" + formula + "]"); } + //int groupCount = matcher.groupCount(); + outerFunction = matcher.group(1); + internalExpression = trimDistinct(matcher.group(2)); } private String trimDistinct(String propertyName) { @@ -41,49 +49,58 @@ class FormulaPropertyPath { } } - boolean isFormula() { - return aggType != null; - } - String aggType() { - return aggType; + return outerFunction; } String basePropertyName() { - return baseName; + return internalExpression; } - /** - * Create a bean property dynamically for the formula in the select clause. - */ - SqlTreeProperty formulaProperty(BeanProperty base) { - String parsedAggregation = buildFormula(base); - String name = logicalName(); + SqlTreeProperty build() { - ScalarType scalarType = base.getScalarType(); + DeployPropertyParser parser = descriptor.parser().setCatchFirst(true); + + String parsed = parser.parse(internalExpression); + ElPropertyDeploy firstProp = parser.getFirstProp(); + + ScalarType scalarType; if (isCount()) { // count maps to Long / BIGINT - scalarType = base.getBeanDescriptor().getScalarType(Types.BIGINT); + scalarType = descriptor.getScalarType(Types.BIGINT); + } else { + // determine scalarType based on first property found by parser + if (firstProp != null) { + scalarType = firstProp.getBeanProperty().getScalarType(); + } else { + throw new IllegalStateException("unable to determine scalarType of formula [" + formula + "] for type " + descriptor+" - maybe use a cast like ::String ?"); + } } - return new DynamicPropertyAggregationFormula(name, scalarType, parsedAggregation, null); + String parsedAggregation = buildFormula(parsed); + return new DynamicPropertyAggregationFormula(formula, scalarType, parsedAggregation, isAggregate(),null); } - private String buildFormula(BeanProperty base) { + private boolean isAggregate() { + for (String aggFunction : AGG_FUNCTIONS) { + if (aggFunction.equals(outerFunction)) { + return true; + } + } + return false; + } + + private String buildFormula(String parsed) { if (countDistinct) { - return "count(distinct ${}"+base.getDbColumn()+")"; + return "count(distinct "+parsed+")"; } else { - return aggType+"(${}"+base.getDbColumn()+")"; + return outerFunction +"("+parsed+")"; } } private boolean isCount() { - return aggType.equals("count"); - } - - private String logicalName() { - return aggType+Character.toUpperCase(baseName.charAt(0))+baseName.substring(1); + return outerFunction.equals("count"); } } diff --git a/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryPropertiesParser.java b/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryPropertiesParser.java index 9b774c9bc..1baa3409a 100644 --- a/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryPropertiesParser.java +++ b/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryPropertiesParser.java @@ -2,8 +2,10 @@ package io.ebeaninternal.server.querydefn; import io.ebean.FetchConfig; import io.ebean.util.StringHelper; +import io.ebeaninternal.server.util.DSelectColumnsParser; import java.util.LinkedHashSet; +import java.util.List; /** * Parses the path properties string. @@ -113,10 +115,10 @@ class OrmQueryPropertiesParser { return null; } - String[] res = inputProperties.split(","); + List res = splitRawSelect(inputProperties); StringBuilder sb = new StringBuilder(70); - LinkedHashSet propertySet = new LinkedHashSet<>(res.length * 2); + LinkedHashSet propertySet = new LinkedHashSet<>(res.size() * 2); int count = 0; String temp; @@ -148,6 +150,13 @@ class OrmQueryPropertiesParser { return propertySet; } + /** + * Split allowing 'dynamic function based properties'. + */ + private List splitRawSelect(String inputProperties) { + return DSelectColumnsParser.parse(inputProperties); + } + private int parseBatchHint(int pos, String option) { int startPos = pos + option.length(); diff --git a/src/main/java/io/ebeaninternal/server/rawsql/DRawSqlColumnsParser.java b/src/main/java/io/ebeaninternal/server/rawsql/DRawSqlColumnsParser.java index 6e17ed5a9..92e3e1078 100644 --- a/src/main/java/io/ebeaninternal/server/rawsql/DRawSqlColumnsParser.java +++ b/src/main/java/io/ebeaninternal/server/rawsql/DRawSqlColumnsParser.java @@ -1,10 +1,12 @@ package io.ebeaninternal.server.rawsql; import io.ebeaninternal.server.rawsql.SpiRawSql.ColumnMapping; +import io.ebeaninternal.server.util.DSelectColumnsParser; -import java.util.regex.Pattern; import javax.persistence.PersistenceException; import java.util.ArrayList; +import java.util.List; +import java.util.regex.Pattern; /** * Parses columnMapping (select clause) mapping columns to bean properties. @@ -13,12 +15,8 @@ final class DRawSqlColumnsParser { private static final Pattern COLINFO_SPLIT = Pattern.compile("\\s(?=[^\\)]*(?:\\(|$))"); - private final int end; - private final String sqlSelect; - private int pos; - private int indexPos; public static ColumnMapping parse(String sqlSelect) { @@ -27,25 +25,21 @@ final class DRawSqlColumnsParser { private DRawSqlColumnsParser(String sqlSelect) { this.sqlSelect = sqlSelect; - this.end = sqlSelect.length(); } private ColumnMapping parse() { - ArrayList columns = new ArrayList<>(); - while (pos <= end) { - ColumnMapping.Column c = nextColumnInfo(); - columns.add(c); - } + List columnList = DSelectColumnsParser.parse(sqlSelect); + List columns = new ArrayList<>(columnList.size()); + + for (String rawColumn : columnList) { + columns.add(parseColumn(rawColumn)); + } return new ColumnMapping(columns); } - private ColumnMapping.Column nextColumnInfo() { - int start = pos; - nextComma(); - String colInfo = sqlSelect.substring(start, pos++); - colInfo = colInfo.trim(); + private ColumnMapping.Column parseColumn(String colInfo) { String[] split = COLINFO_SPLIT.split(colInfo); if (split.length > 1) { @@ -82,18 +76,4 @@ final class DRawSqlColumnsParser { return new ColumnMapping.Column(indexPos++, sb.toString(), split[split.length - 1]); } - private void nextComma() { - boolean inQuote = false; - int inbrackets = 0; - while (pos < end) { - char c = sqlSelect.charAt(pos); - if (c == '\'') inQuote = !inQuote; - else if (c == '(') inbrackets++; - else if (c == ')') inbrackets--; - else if (!inQuote && inbrackets == 0 && c == ',') { - return; - } - pos++; - } - } } diff --git a/src/main/java/io/ebeaninternal/server/util/DSelectColumnsParser.java b/src/main/java/io/ebeaninternal/server/util/DSelectColumnsParser.java new file mode 100644 index 000000000..c172f92b2 --- /dev/null +++ b/src/main/java/io/ebeaninternal/server/util/DSelectColumnsParser.java @@ -0,0 +1,55 @@ +package io.ebeaninternal.server.util; + +import java.util.ArrayList; +import java.util.List; + +/** + * Splits a select clause into 'logical columns' taking into account functions and quotes. + */ +public final class DSelectColumnsParser { + + private final int end; + + private final String selectClause; + + private int pos; + + public static List parse(String sqlSelect) { + return new DSelectColumnsParser(sqlSelect).parse(); + } + + private DSelectColumnsParser(String selectClause) { + this.selectClause = selectClause; + this.end = selectClause.length(); + } + + private List parse() { + + ArrayList columns = new ArrayList<>(); + while (pos <= end) { + columns.add(nextColumnInfo()); + } + return columns; + } + + private String nextColumnInfo() { + int start = pos; + nextComma(); + return selectClause.substring(start, pos++).trim(); + } + + private void nextComma() { + boolean inQuote = false; + int inBrackets = 0; + while (pos < end) { + char c = selectClause.charAt(pos); + if (c == '\'') inQuote = !inQuote; + else if (c == '(') inBrackets++; + else if (c == ')') inBrackets--; + else if (!inQuote && inBrackets == 0 && c == ',') { + return; + } + pos++; + } + } +} diff --git a/src/test/java/io/ebeaninternal/server/deploy/DeployPropertyParserTest.java b/src/test/java/io/ebeaninternal/server/deploy/DeployPropertyParserTest.java index 9c7369a46..10056984e 100644 --- a/src/test/java/io/ebeaninternal/server/deploy/DeployPropertyParserTest.java +++ b/src/test/java/io/ebeaninternal/server/deploy/DeployPropertyParserTest.java @@ -46,7 +46,7 @@ public class DeployPropertyParserTest extends BaseTestCase { @Test public void combined_withAtColumn() { - assertThat(adddressParser().parse("concat(line1, line2, '-EA')")).isEqualTo("concat(${}line_1, ${}line_2, '-EA')"); + assertThat(addressParser().parse("concat(line1, line2, '-EA')")).isEqualTo("concat(${}line_1, ${}line_2, '-EA')"); } @Test @@ -55,11 +55,11 @@ public class DeployPropertyParserTest extends BaseTestCase { } private DeployPropertyParser parser() { - return descriptor.createDeployPropertyParser(); + return descriptor.parser(); } - - private DeployPropertyParser adddressParser() { - return addressBeanDescriptor.createDeployPropertyParser(); + + private DeployPropertyParser addressParser() { + return addressBeanDescriptor.parser(); } } diff --git a/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java b/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java index c3d840603..b990d6077 100644 --- a/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java +++ b/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java @@ -1,35 +1,45 @@ package io.ebeaninternal.server.deploy; import io.ebean.BaseTestCase; +import io.ebeaninternal.server.query.SqlTreeProperty; import org.junit.Test; +import org.tests.model.basic.Customer; import static org.assertj.core.api.Assertions.assertThat; public class FormulaPropertyPathTest extends BaseTestCase { - //private BeanDescriptor customerDesc = getBeanDescriptor(Customer.class); + private BeanDescriptor customerDesc = getBeanDescriptor(Customer.class); @Test public void isFormula() { - assertFormula("max(foo)", "max", "foo"); - assertFormula("min(bar)", "min", "bar"); - assertFormula("avg(baz)", "avg", "baz"); + assertFormula("max(version)", "max", "version"); + assertFormula("min(name)", "min", "name"); + assertFormula("avg(id)", "avg", "id"); } @Test public void isFormula_count() { - assertFormula("count(moo)", "count", "moo"); - assertFormula("count(distinct joo)", "count", "joo"); + assertFormula("count(status)", "count", "status"); + assertFormula("count(distinct name)", "count", "name"); + } + + @Test + public void concat() { + + assertFormula("concat(name,'-end')", "concat", "name,'-end'"); } private void assertFormula(String input, String aggType, String baseProperty) { - FormulaPropertyPath propertyPath = new FormulaPropertyPath(input); + FormulaPropertyPath propertyPath = new FormulaPropertyPath(customerDesc, input); - assertThat(propertyPath.isFormula()).isTrue(); assertThat(propertyPath.basePropertyName()).isEqualTo(baseProperty); assertThat(propertyPath.aggType()).isEqualTo(aggType); + SqlTreeProperty treeProperty = propertyPath.build(); + + assertThat(treeProperty).isNotNull(); } } diff --git a/src/test/java/io/ebeaninternal/server/util/DSelectColumnsParserTest.java b/src/test/java/io/ebeaninternal/server/util/DSelectColumnsParserTest.java new file mode 100644 index 000000000..0b30e225c --- /dev/null +++ b/src/test/java/io/ebeaninternal/server/util/DSelectColumnsParserTest.java @@ -0,0 +1,60 @@ +package io.ebeaninternal.server.util; + +import org.junit.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class DSelectColumnsParserTest { + + @Test + public void parse() { + + List cols = DSelectColumnsParser.parse("a,MD5(id::text) as b,c"); + assertThat(cols).containsExactly("a", "MD5(id::text) as b", "c"); + } + + @Test + public void whitespace_is_trimmed() { + + List cols = DSelectColumnsParser.parse("a , MD5(id::text) as b , c "); + assertThat(cols).containsExactly("a", "MD5(id::text) as b", "c"); + } + + @Test + public void nestedFunctions() { + + List cols = DSelectColumnsParser.parse("a , concat(id,'sd',inner(foo)) as b , c "); + assertThat(cols).containsExactly("a", "concat(id,'sd',inner(foo)) as b", "c"); + } + + @Test + public void basic() { + + List cols = DSelectColumnsParser.parse("name , status , billingAddress "); + assertThat(cols).containsExactly("name", "status", "billingAddress"); + } + + @Test + public void basic_noWhitespace() { + + List cols = DSelectColumnsParser.parse("a,b,c"); + assertThat(cols).containsExactly("a", "b", "c"); + } + + @Test + public void formula_noWhitespace() { + + List cols = DSelectColumnsParser.parse("a,concat(x,y),c"); + assertThat(cols).containsExactly("a", "concat(x,y)", "c"); + } + + @Test + public void with_logicalCast_andAsAlias() { + + List cols = DSelectColumnsParser.parse("name , concat(status,'-end')::String as fullName , billingAddress "); + assertThat(cols).containsExactly("name", "concat(status,'-end')::String as fullName", "billingAddress"); + } + +} diff --git a/src/test/java/org/tests/query/aggregation/TestAggregationCount.java b/src/test/java/org/tests/query/aggregation/TestAggregationCount.java index 34c231b26..6a0c4404a 100644 --- a/src/test/java/org/tests/query/aggregation/TestAggregationCount.java +++ b/src/test/java/org/tests/query/aggregation/TestAggregationCount.java @@ -381,4 +381,25 @@ public class TestAggregationCount extends BaseTestCase { assertThat(sql.get(0)).contains("select count(distinct t0.last_name) from contact t0 where not exists (select 1 from contact_note x where x.contact_id = t0.id)"); } + @Test + public void example_nonAggregateFormula() { + + ResetBasicData.reset(); + + LoggedSqlCollector.start(); + + List names = + + Ebean.find(Contact.class) + .select("concat(lastName,', ',firstName)") + .where().isNull("phone") + .orderBy().asc("lastName") + .findSingleAttributeList(); + + assertThat(names).isNotEmpty(); + + List sql = LoggedSqlCollector.stop(); + assertThat(sql.get(0)).contains("select concat(t0.last_name,', ',t0.first_name) from contact t0 where t0.phone is null order by t0.last_name"); + } + }