From be0a3dd5c9ceee3bbcad4febd142b79c67f15785 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 21 Oct 2025 22:50:33 +1300 Subject: [PATCH] 3683 - Add AggregateFormulaContext to support group_concat etc - Add AggregateFormulaContext with ability to override the default via DatabaseBuilder - Add group_concat, string_agg, listagg to set of known aggregation functions --- .../main/java/io/ebean/DatabaseBuilder.java | 13 ++++ .../ebean/config/AggregateFormulaContext.java | 66 +++++++++++++++++++ .../AggregateFormulaContextBuilder.java | 61 +++++++++++++++++ .../java/io/ebean/config/DatabaseConfig.java | 13 ++++ .../config/AggregateFormulaContextTest.java | 65 ++++++++++++++++++ .../io/ebeaninternal/api/FormulaBuilder.java | 10 +++ .../io/ebeaninternal/api/SpiBeanType.java | 5 ++ .../io/ebeaninternal/api/SpiEbeanServer.java | 3 + .../server/core/DefaultServer.java | 7 ++ .../server/deploy/BeanDescriptor.java | 7 +- .../server/deploy/DFormulaBuilder.java | 19 ++++++ .../server/deploy/FormulaPropertyPath.java | 22 +++---- .../deploy/FormulaPropertyPathTest.java | 8 +-- .../xtest/internal/api/TDSpiEbeanServer.java | 6 ++ .../TestAggregateFormula.java | 22 +++++++ 15 files changed, 309 insertions(+), 18 deletions(-) create mode 100644 ebean-api/src/main/java/io/ebean/config/AggregateFormulaContext.java create mode 100644 ebean-api/src/main/java/io/ebean/config/AggregateFormulaContextBuilder.java create mode 100644 ebean-api/src/test/java/io/ebean/config/AggregateFormulaContextTest.java create mode 100644 ebean-core/src/main/java/io/ebeaninternal/api/FormulaBuilder.java create mode 100644 ebean-core/src/main/java/io/ebeaninternal/server/deploy/DFormulaBuilder.java diff --git a/ebean-api/src/main/java/io/ebean/DatabaseBuilder.java b/ebean-api/src/main/java/io/ebean/DatabaseBuilder.java index e595f61707..5203d1236f 100644 --- a/ebean-api/src/main/java/io/ebean/DatabaseBuilder.java +++ b/ebean-api/src/main/java/io/ebean/DatabaseBuilder.java @@ -989,6 +989,14 @@ default DatabaseBuilder namingConvention(NamingConvention namingConvention) { @Deprecated DatabaseBuilder setNamingConvention(NamingConvention namingConvention); + /** + * Set the AggregateFormulaContext which is used to determine if a database function + * is an aggregate function (like sum, min, max, avg etc). + *

+ * Use this to override the default known aggregation functions. + */ + DatabaseConfig aggregateFormulaContext(AggregateFormulaContext aggregateFormulaContext); + /** * Set to true if all DB column and table names should use quoted identifiers. *

@@ -2610,6 +2618,11 @@ interface Settings extends DatabaseBuilder { */ NamingConvention getNamingConvention(); + /** + * Return the AggregateFormulaContext. + */ + AggregateFormulaContext aggregateFormulaContext(); + /** * Return true if all DB column and table names should use quoted identifiers. */ diff --git a/ebean-api/src/main/java/io/ebean/config/AggregateFormulaContext.java b/ebean-api/src/main/java/io/ebean/config/AggregateFormulaContext.java new file mode 100644 index 0000000000..4973efa2b4 --- /dev/null +++ b/ebean-api/src/main/java/io/ebean/config/AggregateFormulaContext.java @@ -0,0 +1,66 @@ +package io.ebean.config; + +import java.util.Set; + +/** + * Used when parsing formulas to determine if they are aggregation formulas like + * sum, min, max, avg, count etc. + *

+ * Ebean needs to determine if they are aggregation formulas to determine which + * properties should be included in a GROUP BY clause etc. + */ +public interface AggregateFormulaContext { + + /** + * Return true if the outer function is an aggregate function (like sum, count, min, max, avg etc). + */ + boolean isAggregate(String outerFunction); + + /** + * Return true if the aggregate function returns a BIGINT type. + * This is true for functions like count that return a numeric value regardless of the + * type of the property or expression inside the outer function. + */ + boolean isCount(String outerFunction); + + /** + * Return true if the aggregate function returns a VARCHAR type. + * This is true for functions that return a string concatenation like group_concat etc + * regardless of the type of the property used inside the outer function. + */ + boolean isConcat(String outerFunction); + + /** + * Return a builder for the AggregateFormulaContext. + */ + static Builder builder() { + return new AggregateFormulaContextBuilder(); + } + + /** + * A builder for the AggregateFormulaContext. + */ + interface Builder { + + /** + * Override the default set of aggregation functions. + */ + Builder aggregateFunctions(Set count); + + /** + * Override the default set of concat functions. + */ + Builder concatFunctions(Set concat); + + /** + * Override the default set of count functions. + */ + Builder countFunctions(Set count); + + /** + * Build the AggregateFormulaContext. + */ + AggregateFormulaContext build(); + } + +} diff --git a/ebean-api/src/main/java/io/ebean/config/AggregateFormulaContextBuilder.java b/ebean-api/src/main/java/io/ebean/config/AggregateFormulaContextBuilder.java new file mode 100644 index 0000000000..33774c2dbc --- /dev/null +++ b/ebean-api/src/main/java/io/ebean/config/AggregateFormulaContextBuilder.java @@ -0,0 +1,61 @@ +package io.ebean.config; + +import java.util.Set; + +final class AggregateFormulaContextBuilder implements AggregateFormulaContext.Builder { + + private Set aggFunctions = Set.of("count", "max", "min", "avg", "sum", "group_concat", "string_agg", "listagg"); + private Set concat = Set.of("concat", "group_concat", "string_agg", "listagg"); + private Set count = Set.of("count"); + + @Override + public AggregateFormulaContext.Builder aggregateFunctions(Set agg) { + this.aggFunctions = agg; + return this; + } + + @Override + public AggregateFormulaContext.Builder concatFunctions(Set concat) { + this.concat = concat; + return this; + } + + @Override + public AggregateFormulaContext.Builder countFunctions(Set count) { + this.count = count; + return this; + } + + @Override + public AggregateFormulaContext build() { + return new FormulaContext(aggFunctions, concat, count); + } + + private static final class FormulaContext implements AggregateFormulaContext { + + private final Set aggFunctions; + private final Set concat; + private final Set count; + + private FormulaContext(Set aggFunctions, Set concat, Set count) { + this.aggFunctions = aggFunctions; + this.concat = concat; + this.count = count; + } + + @Override + public boolean isAggregate(String outerFunction) { + return aggFunctions.contains(outerFunction); + } + + @Override + public boolean isCount(String outerFunction) { + return count.contains(outerFunction); + } + + @Override + public boolean isConcat(String outerFunction) { + return concat.contains(outerFunction); + } + } +} diff --git a/ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java b/ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java index 2a7b926a85..2f651b1ba0 100644 --- a/ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java +++ b/ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java @@ -356,6 +356,8 @@ public class DatabaseConfig implements DatabaseBuilder.Settings { */ private NamingConvention namingConvention = new UnderscoreNamingConvention(); + private AggregateFormulaContext aggregateFormulaContext = AggregateFormulaContext.builder().build(); + /** * Behaviour of updates in JDBC batch to by default include all properties. */ @@ -1279,6 +1281,17 @@ public DatabaseConfig setNamingConvention(NamingConvention namingConvention) { return this; } + @Override + public AggregateFormulaContext aggregateFormulaContext() { + return aggregateFormulaContext; + } + + @Override + public DatabaseConfig aggregateFormulaContext(AggregateFormulaContext aggregateFormulaContext) { + this.aggregateFormulaContext = aggregateFormulaContext; + return this; + } + @Override public boolean isAllQuotedIdentifiers() { return platformConfig.isAllQuotedIdentifiers(); diff --git a/ebean-api/src/test/java/io/ebean/config/AggregateFormulaContextTest.java b/ebean-api/src/test/java/io/ebean/config/AggregateFormulaContextTest.java new file mode 100644 index 0000000000..487d8e897c --- /dev/null +++ b/ebean-api/src/test/java/io/ebean/config/AggregateFormulaContextTest.java @@ -0,0 +1,65 @@ +package io.ebean.config; + +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; + +class AggregateFormulaContextTest { + + @Test + void defaultContext() { + var defaultContext = AggregateFormulaContext.builder().build(); + for (String aggFunction : List.of("count", "max", "min", "avg", "sum", "group_concat", "string_agg", "listagg")) { + assertThat(defaultContext.isAggregate(aggFunction)).isTrue(); + } + for (String c : List.of("concat", "group_concat", "string_agg", "listagg")) { + assertThat(defaultContext.isConcat(c)).isTrue(); + } + for (String c : List.of("count")) { + assertThat(defaultContext.isCount(c)).isTrue(); + } + + assertThat(defaultContext.isConcat("junk")).isFalse(); + assertThat(defaultContext.isCount("junk")).isFalse(); + assertThat(defaultContext.isAggregate("junk")).isFalse(); + } + + @Test + void overrideAggregateFunctions() { + AggregateFormulaContext mySum = AggregateFormulaContext.builder() + .aggregateFunctions(Set.of("my_sum")) + .build(); + + assertThat(mySum.isAggregate("my_sum")).isTrue(); + assertThat(mySum.isAggregate("avg")).isFalse(); + assertThat(mySum.isCount("count")).isTrue(); + assertThat(mySum.isConcat("group_concat")).isTrue(); + } + + @Test + void overrideConcatFunctions() { + AggregateFormulaContext myConcat = AggregateFormulaContext.builder() + .concatFunctions(Set.of("my_concat")) + .build(); + + assertThat(myConcat.isAggregate("avg")).isTrue(); + assertThat(myConcat.isCount("count")).isTrue(); + assertThat(myConcat.isConcat("group_concat")).isFalse(); + assertThat(myConcat.isConcat("my_concat")).isTrue(); + } + + @Test + void overrideCountFunctions() { + AggregateFormulaContext myCount = AggregateFormulaContext.builder() + .countFunctions(Set.of("my_count")) + .build(); + + assertThat(myCount.isAggregate("avg")).isTrue(); + assertThat(myCount.isCount("count")).isFalse(); + assertThat(myCount.isCount("my_count")).isTrue(); + assertThat(myCount.isConcat("group_concat")).isTrue(); + } +} diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/FormulaBuilder.java b/ebean-core/src/main/java/io/ebeaninternal/api/FormulaBuilder.java new file mode 100644 index 0000000000..e211e2ff26 --- /dev/null +++ b/ebean-core/src/main/java/io/ebeaninternal/api/FormulaBuilder.java @@ -0,0 +1,10 @@ +package io.ebeaninternal.api; + +import io.ebean.config.AggregateFormulaContext; +import io.ebeaninternal.server.query.STreeProperty; + +public interface FormulaBuilder { + + STreeProperty create(AggregateFormulaContext context, String formula, String path); + +} diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/SpiBeanType.java b/ebean-core/src/main/java/io/ebeaninternal/api/SpiBeanType.java index 9923fff3f8..7ecd1cc70f 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/SpiBeanType.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/SpiBeanType.java @@ -14,4 +14,9 @@ public interface SpiBeanType { * or removals from the collection. */ boolean isToManyDirty(EntityBean bean); + + /** + * Return the FormulaBuilder for this type. + */ + FormulaBuilder formulaBuilder(); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java b/ebean-core/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java index 48c22e5deb..2f61af3489 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/SpiEbeanServer.java @@ -1,5 +1,6 @@ package io.ebeaninternal.api; +import io.ebeaninternal.server.query.STreeProperty; import org.jspecify.annotations.Nullable; import io.ebean.*; import io.ebean.bean.BeanCollectionLoader; @@ -377,4 +378,6 @@ public interface SpiEbeanServer extends SpiServer, BeanCollectionLoader { @Nullable SqlRow findOne(SpiSqlQuery query); + + STreeProperty createFormulaProperty(SpiBeanType desc, String formula, String path); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java index d6b28c09d0..fbc14bec3b 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultServer.java @@ -113,6 +113,7 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer { private final long slowQueryMicros; private final SlowQueryListener slowQueryListener; private final boolean disableL2Cache; + private final AggregateFormulaContext formulaContext; private boolean shutdown; /** @@ -128,6 +129,7 @@ public DefaultServer(InternalConfiguration config, ServerCacheManager cache) { this.backgroundExecutor = config.getBackgroundExecutor(); this.extraMetrics = config.getExtraMetrics(); this.serverName = this.config.getName(); + this.formulaContext = config.getConfig().aggregateFormulaContext(); this.lazyLoadBatchSize = this.config.getLazyLoadBatchSize(); this.cqueryEngine = config.getCQueryEngine(); this.expressionFactory = config.getExpressionFactory(); @@ -928,6 +930,11 @@ public T find(Class beanType, Object id, @Nullable Transaction transactio return findId(query); } + @Override + public STreeProperty createFormulaProperty(SpiBeanType desc, String formula, String path) { + return desc.formulaBuilder().create(formulaContext, formula, path); + } + SpiOrmQueryRequest createQueryRequest(Type type, SpiQuery query) { SpiOrmQueryRequest request = buildQueryRequest(type, query); request.prepareQuery(); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java index efa3a320a5..6b580119ff 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java @@ -2410,7 +2410,12 @@ boolean matchBaseTable(String tableName) { */ private STreeProperty findSqlTreeFormula(String formula, String path) { String key = formula + "-" + path; - return dynamicProperty.computeIfAbsent(key, (fullKey) -> FormulaPropertyPath.create(this, formula, path)); + return dynamicProperty.computeIfAbsent(key, (fullKey) -> ebeanServer.createFormulaProperty(this, formula, path)); + } + + @Override + public FormulaBuilder formulaBuilder() { + return new DFormulaBuilder(this); } /** diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DFormulaBuilder.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DFormulaBuilder.java new file mode 100644 index 0000000000..53e4641116 --- /dev/null +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DFormulaBuilder.java @@ -0,0 +1,19 @@ +package io.ebeaninternal.server.deploy; + +import io.ebean.config.AggregateFormulaContext; +import io.ebeaninternal.api.FormulaBuilder; +import io.ebeaninternal.server.query.STreeProperty; + +final class DFormulaBuilder implements FormulaBuilder { + + private final BeanDescriptor descriptor; + + DFormulaBuilder(BeanDescriptor descriptor) { + this.descriptor = descriptor; + } + + @Override + public STreeProperty create(AggregateFormulaContext context, String formula, String path) { + return FormulaPropertyPath.create(descriptor, context, formula, path); + } +} diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java index 1978955578..22bec81ab3 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/FormulaPropertyPath.java @@ -1,6 +1,7 @@ package io.ebeaninternal.server.deploy; import io.ebean.core.type.ScalarType; +import io.ebean.config.AggregateFormulaContext; import io.ebeaninternal.server.el.ElPropertyDeploy; import io.ebeaninternal.server.query.STreeProperty; @@ -9,11 +10,10 @@ final class FormulaPropertyPath { - private static final String[] AGG_FUNCTIONS = {"count", "max", "min", "avg", "sum"}; - private static final String DISTINCT_ = "distinct "; private final BeanDescriptor descriptor; + private final AggregateFormulaContext context; private final String formula; private final String outerFunction; private final String internalExpression; @@ -24,12 +24,13 @@ final class FormulaPropertyPath { private String cast; private String alias; - static STreeProperty create(BeanDescriptor descriptor, String formula, String path) { - return new FormulaPropertyPath(descriptor, formula, path).build(); + static STreeProperty create(BeanDescriptor descriptor, AggregateFormulaContext context, String formula, String path) { + return new FormulaPropertyPath(descriptor, context, formula, path).build(); } - FormulaPropertyPath(BeanDescriptor descriptor, String formula, String path) { + FormulaPropertyPath(BeanDescriptor descriptor, AggregateFormulaContext context, String formula, String path) { this.descriptor = descriptor; + this.context = context; this.formula = formula; int openBracket = formula.indexOf('('); int closeBracket = formula.lastIndexOf(')'); @@ -106,10 +107,10 @@ STreeProperty build() { } return create(scalarType); } - if (isCount()) { + if (context.isCount(outerFunction)) { return create(descriptor.scalarType(Types.BIGINT)); } - if (isConcat()) { + if (context.isConcat(outerFunction)) { return create(descriptor.scalarType(Types.VARCHAR)); } if (firstProp == null) { @@ -144,12 +145,7 @@ private String logicalName() { } private boolean isAggregate() { - for (String aggFunction : AGG_FUNCTIONS) { - if (aggFunction.equals(outerFunction)) { - return true; - } - } - return false; + return context.isAggregate(outerFunction); } private String buildFormula(String parsed) { diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java index 423bc5f311..f3d9807399 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/deploy/FormulaPropertyPathTest.java @@ -1,5 +1,6 @@ package io.ebeaninternal.server.deploy; +import io.ebean.config.AggregateFormulaContext; import org.junit.jupiter.api.Test; import org.tests.model.basic.Customer; @@ -7,11 +8,10 @@ public class FormulaPropertyPathTest extends BaseTest { - private BeanDescriptor customerDesc = getBeanDescriptor(Customer.class); + private final BeanDescriptor customerDesc = getBeanDescriptor(Customer.class); @Test public void isFormula() { - assertFormula("max(version)", "max", "version"); assertFormula("min(name)", "min", "name"); assertFormula("avg(id)", "avg", "id"); @@ -58,8 +58,8 @@ private void assertFormula(String input, String funcName, String expression) { } private void assertFormula(String input, String funcName, String expression, String cast, String alias) { - - FormulaPropertyPath propertyPath = new FormulaPropertyPath(customerDesc, input, null); + var context = AggregateFormulaContext.builder().build(); + FormulaPropertyPath propertyPath = new FormulaPropertyPath(customerDesc, context, input, null); assertThat(propertyPath.internalExpression()).isEqualTo(expression); assertThat(propertyPath.outerFunction()).isEqualTo(funcName); diff --git a/ebean-test/src/test/java/io/ebean/xtest/internal/api/TDSpiEbeanServer.java b/ebean-test/src/test/java/io/ebean/xtest/internal/api/TDSpiEbeanServer.java index b7fc436ee4..beb8e8edc8 100644 --- a/ebean-test/src/test/java/io/ebean/xtest/internal/api/TDSpiEbeanServer.java +++ b/ebean-test/src/test/java/io/ebean/xtest/internal/api/TDSpiEbeanServer.java @@ -1,5 +1,6 @@ package io.ebean.xtest.internal.api; +import io.ebeaninternal.server.query.STreeProperty; import org.jspecify.annotations.NullMarked; import io.ebean.*; import io.ebean.annotation.Platform; @@ -711,6 +712,11 @@ public SqlRow findOne(SpiSqlQuery query) { return null; } + @Override + public STreeProperty createFormulaProperty(SpiBeanType desc, String formula, String path) { + return null; + } + @Override public void save(Object bean) throws OptimisticLockException { } diff --git a/ebean-test/src/test/java/org/tests/aggregateformula/TestAggregateFormula.java b/ebean-test/src/test/java/org/tests/aggregateformula/TestAggregateFormula.java index ca3c8faa46..ee43a7f5ca 100644 --- a/ebean-test/src/test/java/org/tests/aggregateformula/TestAggregateFormula.java +++ b/ebean-test/src/test/java/org/tests/aggregateformula/TestAggregateFormula.java @@ -2,6 +2,7 @@ import io.ebean.xtest.BaseTestCase; import io.ebean.DB; +import io.ebean.xtest.ForPlatform; import io.ebean.xtest.IgnorePlatform; import io.ebean.annotation.Platform; import io.ebean.test.LoggedSql; @@ -126,6 +127,27 @@ public void minOnManyToOne_withFetchQueryWithJoin() { assertThat(contact.getCustomer().getId()).isNotNull(); } + @ForPlatform(Platform.H2) + @Test + public void group_agg() { + ResetBasicData.reset(); + + LoggedSql.start(); + + List orders = DB.find(Order.class) + .select("status, group_concat(id) as customerName") + .findList(); + + List sql = LoggedSql.stop(); + assertSql(sql.get(0)).contains("select t0.status, group_concat(t0.id) customerName from o_order t0 group by t0.status"); + + assertThat(orders).isNotEmpty(); + for (Order order : orders) { + assertThat(order.getStatus()).isNotNull(); + assertThat(order.getCustomerName()).isNotNull(); + } + } + @Test public void sum_withoutAlias() {