From 8d88841dc8a275db49bc05b6a9c592a5003c9aad Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Thu, 25 Jun 2026 22:18:10 +1200 Subject: [PATCH] Add @Formula2 support on @ManyToOne association properties (#3798) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends @Formula2 so it can be placed on a @ManyToOne field. Instead of embedding physical SQL aliases (as @Formula(join=...) requires), a @Formula2 expression uses logical bean paths — the required joins are derived automatically. Example `` // Before — physical SQL, brittle alias wiring: @Formula(select = "coalesce(${ta}.some_bean_id, j1.some_bean_id)", join = PARENTS_JOIN) @ManyToOne EBasic effectiveBean; ``` ``` // After — logical paths, joins resolved automatically: @Formula2("coalesce(someBean.id, parent.someBean.id)") @ManyToOne EBasic effectiveBean; `` The generated SQL is identical; the annotation is far more readable and maintainable. What changed Annotation parsing (AnnotationAssocOnes) — @Formula2 on a @ManyToOne is now recognised and the expression parsed into a select fragment and a set of dependency join paths. Query tree building (SqlTreeBuilder) — three scenarios all handled correctly: - Fetched as a tree node — dependency joins are inserted before the formula2 join using addChildFirst - Partial parent fetch — dependency joins are registered even when the parent chunk only selects its ID column - Predicate-only (where clause, no fetch) — addFormula2JoinsFromPredicates runs before buildSelectChain so dependency join paths are populated before buildExtraJoins constructs the extra-join tree; IncludesDistiller uses addChildFirst to preserve ordering within the extra-join tree Init ordering (BeanDescriptorManager) — initFormula2Properties() moved to a dedicated pass 5, after all descriptors are fully initialised, so cross-descriptor path resolution (e.g. parent.parent.someBean.id) is always safe. SqlTreeNodeExtraJoin — gained addChildFirst() to match SqlTreeNodeBean, enabling formula2 dependency joins to be prepended ahead of the formula2 property join in the extra-join tree. Fixes Supersedes and resolves #2773 — the reported bug (wrong join ordering when combining fetch and where on a formula-joined field) is eliminated for ChildPerson.effectiveBean and ParentPerson.effectiveBean, which are now expressed as @Formula2. Co-authored-by: robin.bygrave --- .../server/deploy/BeanDescriptor.java | 5 +- .../server/deploy/BeanDescriptorManager.java | 8 ++ .../server/deploy/BeanProperty.java | 2 +- .../server/deploy/BeanPropertyAssocOne.java | 33 +++++- .../server/deploy/DbSqlContext.java | 16 +++ .../deploy/parse/AnnotationAssocOnes.java | 9 +- .../server/query/DefaultDbSqlContext.java | 22 ++++ .../server/query/SqlTreeBuilder.java | 105 ++++++++++++++++-- .../server/query/SqlTreeNode.java | 8 ++ .../server/query/SqlTreeNodeBean.java | 5 + .../server/query/SqlTreeNodeExtraJoin.java | 11 ++ .../org/tests/model/family/ChildPerson.java | 12 +- .../org/tests/model/family/ParentPerson.java | 8 +- .../query/joins/TestQueryJoinOnFormula.java | 91 ++++++++++++++- 14 files changed, 305 insertions(+), 30 deletions(-) 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 0d2b6779b..8817368a9 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 @@ -472,7 +472,6 @@ public class BeanDescriptor implements BeanType, STreeType, SpiBeanType { if (unidirectional != null) { unidirectional.initialise(initContext); } - initFormula2Properties(); idBinder.initialise(); idBinderInLHSSql = idBinder.bindInSql(baseTableAlias); idBinderIdSql = idBinder.bindEqSql(baseTableAlias); @@ -505,8 +504,10 @@ public class BeanDescriptor implements BeanType, STreeType, SpiBeanType { /** * Parse @Formula2 logical expressions into placeholder-form SQL after all * relationships have been wired up and elPropertyDeploy() is fully functional. + * Called from BeanDescriptorManager in a dedicated pass after all descriptors + * have been fully initialised, so cross-descriptor paths are safe to navigate. */ - private void initFormula2Properties() { + void initFormula2Properties() { for (BeanProperty prop : propertiesAll()) { String rawExpr = prop.formula2RawExpression(); if (rawExpr != null) { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorManager.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorManager.java index d80994f00..bf1f553e0 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorManager.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorManager.java @@ -430,6 +430,14 @@ public final class BeanDescriptorManager implements BeanDescriptorMap, SpiBeanTy d.initialiseDocMapping(); } + // PASS 5: + // parse @Formula2 expressions — runs after all descriptors are fully + // initialised so cross-descriptor property paths (e.g. parent.parent.someBean.id) + // can be resolved safely without hitting null targetDescriptors + for (BeanDescriptor d : descMap.values()) { + d.initFormula2Properties(); + } + // create BeanManager for each non-embedded entity bean for (BeanDescriptor d : descMap.values()) { d.initLast(); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java index d56fa5319..6ce4f352f 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java @@ -119,7 +119,7 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty { final String elPlaceHolderEncrypted; private final String sqlFormulaSelect; final String sqlFormulaJoin; - private String formula2Select; + protected String formula2Select; private Set formula2Includes; private final String formula2RawExpression; private final String aggregation; diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java index be3378eae..8da1d1f47 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssocOne.java @@ -566,10 +566,18 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr return findMatch(embeddedProp, prop, prop.dbColumn(), tableJoin); } + @Override + public boolean isFormula() { + return super.isFormula() || formula2Select != null; + } + @Override public void appendSelect(DbSqlContext ctx, boolean subQuery) { if (!isTransient) { - if (primaryKeyExport) { + if (formula2Select != null) { + // @Formula2 on @ManyToOne: use formula2 expression as FK selector + ctx.appendFormula2Select(formula2Select); + } else if (primaryKeyExport) { descriptor.idProperty().appendSelect(ctx, subQuery); } else { localHelp.appendSelect(ctx, subQuery); @@ -588,9 +596,32 @@ public class BeanPropertyAssocOne extends BeanPropertyAssoc implements STr return super.addJoin(joinType, a1, a2, ctx); } + /** + * Add table join using a prefix to resolve table aliases. + */ + @Override + public SqlJoinType addJoin(SqlJoinType joinType, String prefix, DbSqlContext ctx) { + if (formula2Select != null) { + // @Formula2 on @ManyToOne: join target table with formula2 FK expression as ON clause + String parentPrefix = SplitName.split(prefix)[0]; // null for root-level + String a2 = ctx.tableAlias(prefix); + String resolvedFk = ctx.parseFormula2(formula2Select, parentPrefix); + String joinLiteral = joinType.literal(SqlJoinType.OUTER); + String foreignIdCol = tableJoin.columns()[0].getForeignDbColumn(); + ctx.addFormula2Join(joinLiteral, tableJoin.getTable(), a2, foreignIdCol, resolvedFk); + return SqlJoinType.OUTER; + } + return super.addJoin(joinType, prefix, ctx); + } + @Override public void appendFrom(DbSqlContext ctx, SqlJoinType joinType, String manyWhere) { if (!isTransient && !primaryKeyExport) { + if (formula2Select != null) { + // @Formula2 on @ManyToOne: auto-joins for the formula are handled via formula2JoinIncludes; + // no additional join SQL is emitted here (the FK value comes from the formula2 SELECT expression) + return; + } localHelp.appendFrom(ctx, joinType); if (sqlFormulaJoin != null) { String alias = ctx.tableAliasManyWhere(manyWhere); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbSqlContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbSqlContext.java index 97309c34c..4bd32dda4 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbSqlContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbSqlContext.java @@ -81,6 +81,22 @@ public interface DbSqlContext { */ void appendFormulaJoin(String sqlFormulaJoin, SqlJoinType joinType, String tableAlias); + /** + * Parse a @Formula2 expression, resolving path placeholders (e.g. ${} or ${parent}) + * relative to the given parent prefix. + */ + default String parseFormula2(String formula, String prefix) { + return formula; + } + + /** + * Append a join where the ON clause FK side is a pre-resolved @Formula2 expression. + * Used for @Formula2 on @ManyToOne properties where the FK value is a formula. + */ + default void addFormula2Join(String joinLiteral, String table, String a2, String foreignIdCol, String resolvedFkExpr) { + // default no-op for non-default implementations + } + /** * Return the current content length. */ diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/parse/AnnotationAssocOnes.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/parse/AnnotationAssocOnes.java index ca37032d0..efb7486fb 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/parse/AnnotationAssocOnes.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/parse/AnnotationAssocOnes.java @@ -2,6 +2,7 @@ package io.ebeaninternal.server.deploy.parse; import io.ebean.annotation.DbForeignKey; import io.ebean.annotation.FetchPreference; +import io.ebean.annotation.Formula2; import io.ebean.annotation.TenantId; import io.ebean.annotation.Where; import io.ebean.config.NamingConvention; @@ -88,6 +89,11 @@ final class AnnotationAssocOnes extends AnnotationAssoc { prop.setExtraWhere(processFormula(where.clause())); } + Formula2 formula2 = prop.getMetaAnnotationFormula2(platform); + if (formula2 != null) { + prop.setFormula2Expression(formula2.value()); + } + PrimaryKeyJoinColumn primaryKeyJoin = get(prop, PrimaryKeyJoinColumn.class); if (primaryKeyJoin != null) { readPrimaryKeyJoin(primaryKeyJoin, prop); @@ -141,7 +147,8 @@ final class AnnotationAssocOnes extends AnnotationAssoc { fkeyPrefix = nc.getColumnFromProperty(beanType, prop.name()); } - beanTable.createJoinColumn(fkeyPrefix, prop.getTableJoin(), true, prop.getSqlFormulaSelect()); + String formulaSelect = prop.getSqlFormulaSelect() != null ? prop.getSqlFormulaSelect() : prop.getFormula2Expression(); + beanTable.createJoinColumn(fkeyPrefix, prop.getTableJoin(), true, formulaSelect); } } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultDbSqlContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultDbSqlContext.java index 1e23d0c72..650f2f24d 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultDbSqlContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultDbSqlContext.java @@ -260,6 +260,28 @@ final class DefaultDbSqlContext implements DbSqlContext { sb.append(converted); } + @Override + public String parseFormula2(String formula, String prefix) { + return alias.parseFormula2(formula, prefix); + } + + @Override + public void addFormula2Join(String joinLiteral, String table, String a2, String foreignIdCol, String resolvedFkExpr) { + if (tableJoins == null) { + tableJoins = new HashSet<>(); + } + String joinKey = table + "-formula2-" + a2; + if (tableJoins.contains(joinKey)) { + joinSuppressed = true; + return; + } + joinSuppressed = false; + tableJoins.add(joinKey); + sb.append(' ').append(joinLiteral).append(' ').append(table).append(' ').append(a2); + sb.append(" on ").append(a2).append('.').append(foreignIdCol); + sb.append(" = ").append(resolvedFkExpr); + } + @Override public void appendParseSelect(String parseSelect, String columnAlias) { String converted = this.alias.parse(parseSelect); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java index 995d6dfb9..8482883c3 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeBuilder.java @@ -130,7 +130,6 @@ public final class SqlTreeBuilder { encryptedProps = ctx.encryptedProps(); query.incrementAsOfTableCount(ctx.asOfTableCount()); } - boolean includeJoins = alias != null && alias.isIncludeJoins(); return new SqlTree(rootNode, distinctOn, selectSql, fromSql, groupBy, inheritanceWhereSql, encryptedProps, manyProperty, includeJoins); } @@ -215,6 +214,12 @@ public final class SqlTreeBuilder { } private SqlTreeNode buildRootNode(STreeType desc) { + if (!rawSql) { + // Populate formula2JoinIncludes from predicate paths BEFORE buildSelectChain runs, + // because buildSelectChain → buildNode → buildExtraJoins needs formula2JoinIncludes + // to already have predicate-based formula2 dependency joins (the where-only case). + addFormula2JoinsFromPredicates(desc, predicates.predicateIncludes()); + } SqlTreeNode root = buildSelectChain(null, null, desc, null); if (!rawSql) { alias.addJoin(queryDetail.getFetchPaths(), desc); @@ -228,6 +233,28 @@ public final class SqlTreeBuilder { return root; } + /** + * Scan predicate include paths for @Formula2 properties and add their dependency joins to + * formula2JoinIncludes. This covers the case where a @Formula2 @ManyToOne is referenced in + * a where/order-by clause but not fetched (so addFormula2Joins was never called for it during + * tree building). + */ + private void addFormula2JoinsFromPredicates(STreeType desc, Set predicateIncludes) { + if (predicateIncludes == null) return; + for (String path : predicateIncludes) { + ExtraJoin extra = desc.extraJoin(path); + if (extra != null) { + Set f2Joins = extra.property().formula2Joins(); + if (f2Joins != null) { + String prefix = SplitName.split(path)[0]; // parent of this path + for (String join : f2Joins) { + formula2JoinIncludes.add(SplitName.add(prefix, join)); + } + } + } + } + } + /** * Recursively build the query tree depending on what leaves in the tree * should be included. @@ -375,13 +402,19 @@ public final class SqlTreeBuilder { // look for predicateIncludes that are not in selectIncludes and add // them as extra joins to the query - IncludesDistiller extraJoinDistill = new IncludesDistiller(desc, selectIncludes, predicateIncludes, manyWhereJoins, temporalMode); + IncludesDistiller extraJoinDistill = new IncludesDistiller(desc, selectIncludes, predicateIncludes, manyWhereJoins, temporalMode, formula2JoinIncludes); Collection extraJoins = extraJoinDistill.getExtraJoinRootNodes(); if (!extraJoins.isEmpty()) { // add extra joins required to support predicates and/or order by clause for (SqlTreeNodeExtraJoin extraJoin : extraJoins) { if (!addToParent(extraJoin, myList)) { - myList.add(extraJoin); + if (isFormula2Dependency(extraJoin.prefix())) { + // formula2 dependency joins at root level must come before tree nodes + // that reference their table aliases in ON clauses + myList.add(0, extraJoin); + } else { + myList.add(extraJoin); + } } if (extraJoin.isManyJoin()) { // as we are now going to join to the many then we need @@ -395,18 +428,40 @@ public final class SqlTreeBuilder { /** * Return true if the extra join was added as a child to one of the nodes. + * Formula2 dependency joins are prepended (addChildFirst) so they appear in SQL + * before the tree nodes that reference their table aliases. */ private boolean addToParent(SqlTreeNodeExtraJoin extraJoin, List myList) { String parentPath = SplitName.split(extraJoin.prefix())[0]; for (SqlTreeNode maybeParent : myList) { if (maybeParent.prefix().equals(parentPath)) { - maybeParent.addChild(extraJoin); + if (isFormula2Dependency(extraJoin.prefix())) { + // formula2 dependency joins must precede child tree nodes that reference their aliases + maybeParent.addChildFirst(extraJoin); + } else { + maybeParent.addChild(extraJoin); + } return true; } } return false; } + /** + * Return true if the path is a formula2 dependency — either directly in formula2JoinIncludes, + * or an ancestor of a path in formula2JoinIncludes (e.g. "parent.parent" is an ancestor of + * "parent.parent.someBean" and must also be ordered before any formula2 tree node that + * references its descendants' table aliases). + */ + private boolean isFormula2Dependency(String path) { + if (formula2JoinIncludes.contains(path)) return true; + String prefix = path + "."; + for (String dep : formula2JoinIncludes) { + if (dep.startsWith(prefix)) return true; + } + return false; + } + /** * A subQuery has slightly different rules in that it just generates SQL (into * the where clause) and its properties are not required to read the resultSet @@ -543,7 +598,17 @@ public final class SqlTreeBuilder { private SqlTreeProperties getBaseSelect(STreeType desc, OrmQueryProperties queryProps, String prefix) { boolean partial = queryProps != null && !queryProps.allProperties(); if (partial) { - return getBaseSelectPartial(desc, queryProps); + SqlTreeProperties result = getBaseSelectPartial(desc, queryProps); + // Even in partial select mode, we must register formula2 dependency joins for any + // @Formula2 @ManyToOne properties that are included as bean joins (tree nodes). + // These properties are not in the partial property list, but their formula2 dependency + // joins still need to appear in the FROM clause before the tree node's formula2 join. + for (STreePropertyAssocOne p : desc.propsOne()) { + if (queryProps.isIncludedBeanJoin(p.name())) { + addFormula2Joins(p, prefix); + } + } + return result; } SqlTreeProperties selectProps = new SqlTreeProperties(); @@ -559,7 +624,6 @@ public final class SqlTreeBuilder { selectProps.add(desc.propsEmbedded()); for (STreePropertyAssocOne propertyAssocOne : desc.propsOne()) { - //noinspection StatementWithEmptyBody if (queryProps != null && queryProps.isIncludedBeanJoin(propertyAssocOne.name()) && propertyAssocOne.hasForeignKey() @@ -568,6 +632,8 @@ public final class SqlTreeBuilder { // as it will have its own entire Node in the SqlTree } else { selectProps.add(propertyAssocOne); + // @Formula2 on @ManyToOne: register auto-join paths so the required joins are added + addFormula2Joins(propertyAssocOne, prefix); } } return selectProps; @@ -622,6 +688,7 @@ public final class SqlTreeBuilder { private final STreeType desc; private final Set selectIncludes; private final Set predicateIncludes; + private final Set formula2Deps; private final SpiQuery.TemporalMode temporalMode; private final ManyWhereJoins manyWhereJoins; @@ -629,12 +696,28 @@ public final class SqlTreeBuilder { private final Map rootRegister = new HashMap<>(); private IncludesDistiller(STreeType desc, Set selectIncludes, - Set predicateIncludes, ManyWhereJoins manyWhereJoins, SpiQuery.TemporalMode temporalMode) { + Set predicateIncludes, ManyWhereJoins manyWhereJoins, SpiQuery.TemporalMode temporalMode, + Set formula2Deps) { this.desc = desc; this.selectIncludes = selectIncludes; this.predicateIncludes = predicateIncludes; this.manyWhereJoins = manyWhereJoins; this.temporalMode = temporalMode; + this.formula2Deps = formula2Deps; + } + + /** + * Return true if this path is a formula2 dependency — either directly in formula2Deps or + * an ancestor of a path in formula2Deps. Such joins must appear before formula2 joins that + * reference their table aliases. + */ + private boolean isFormula2Dependency(String path) { + if (formula2Deps.contains(path)) return true; + String prefix = path + "."; + for (String dep : formula2Deps) { + if (dep.startsWith(prefix)) return true; + } + return false; } /** @@ -725,7 +808,13 @@ public final class SqlTreeBuilder { parentJoin = createJoinLeaf(parentPropertyName); } - parentJoin.addChild(childJoin); + // formula2 dependency joins must appear before formula2 property joins that reference + // their table aliases — use addChildFirst to push dependencies ahead + if (isFormula2Dependency(childJoin.prefix())) { + parentJoin.addChildFirst(childJoin); + } else { + parentJoin.addChild(childJoin); + } childJoin = parentJoin; includeProp = parentPropertyName; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java index 225d93081..c77a9ab58 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNode.java @@ -92,4 +92,12 @@ interface SqlTreeNode { default void addChild(SqlTreeNode extraJoin) { throw new UnsupportedOperationException(); } + + /** + * Add a child node at the beginning of the children list. + * Used to ensure formula2 dependency joins appear before the tree nodes that reference them. + */ + default void addChildFirst(SqlTreeNode extraJoin) { + addChild(extraJoin); + } } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java index f5f132877..0036108df 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeBean.java @@ -106,6 +106,11 @@ class SqlTreeNodeBean implements SqlTreeNode { children.add(extraJoin); } + @Override + public void addChildFirst(SqlTreeNode extraJoin) { + children.add(0, extraJoin); + } + @Override public SqlTreeLoad createLoad() { return new SqlTreeLoadBean(this); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java index 6634e7947..43feefdc7 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeNodeExtraJoin.java @@ -102,6 +102,17 @@ final class SqlTreeNodeExtraJoin implements SqlTreeNode { children.add(child); } + @Override + public void addChildFirst(SqlTreeNode child) { + if (children == null) { + children = new ArrayList<>(); + } + if (child.hasMany()) { + manyJoin = true; + } + children.add(0, child); + } + @Override public void dependentTables(Set tables) { tables.add(assocBeanProperty.target().baseTable(SpiQuery.TemporalMode.CURRENT)); diff --git a/ebean-test/src/test/java/org/tests/model/family/ChildPerson.java b/ebean-test/src/test/java/org/tests/model/family/ChildPerson.java index 866f49590..d2f6d5910 100644 --- a/ebean-test/src/test/java/org/tests/model/family/ChildPerson.java +++ b/ebean-test/src/test/java/org/tests/model/family/ChildPerson.java @@ -1,6 +1,5 @@ package org.tests.model.family; -import io.ebean.annotation.Formula; import io.ebean.annotation.Formula2; import org.tests.model.basic.EBasic; @@ -12,9 +11,6 @@ import jakarta.persistence.ManyToOne; public class ChildPerson extends InheritablePerson { private static final long serialVersionUID = 1L; - private static final String PARENTS_JOIN = "join parent_person j1 on j1.identifier = ${ta}.parent_identifier " - + "join grand_parent_person j2 on j2.identifier = j1.parent_identifier"; - @ManyToOne(cascade = CascadeType.ALL) private ParentPerson parent; @@ -22,18 +18,16 @@ public class ChildPerson extends InheritablePerson { private String address; - //@Formula2("coalesce(familyName, parent.familyName, parent.parent.familyName)") - @Formula(select = "coalesce(${ta}.family_name, j1.family_name, j2.family_name)", join = PARENTS_JOIN) + @Formula2("coalesce(familyName, parent.familyName, parent.parent.familyName)") private String effectiveFamilyName; @Formula2("coalesce(familyName, parent.familyName, parent.parent.familyName)") private String derivedFamilyName; - //@Coalesce({ "address", "parent.address", "parent.parent.address" }) - @Formula(select = "coalesce(${ta}.address, j1.address, j2.address)", join = PARENTS_JOIN) + @Formula2("coalesce(address, parent.address, parent.parent.address)") private String effectiveAddress; - @Formula(select = "coalesce(${ta}.some_bean_id, j1.some_bean_id, j2.some_bean_id)") + @Formula2("coalesce(someBean.id, parent.someBean.id, parent.parent.someBean.id)") @ManyToOne private EBasic effectiveBean; diff --git a/ebean-test/src/test/java/org/tests/model/family/ParentPerson.java b/ebean-test/src/test/java/org/tests/model/family/ParentPerson.java index 4b8e95a7e..c30ab41ee 100644 --- a/ebean-test/src/test/java/org/tests/model/family/ParentPerson.java +++ b/ebean-test/src/test/java/org/tests/model/family/ParentPerson.java @@ -18,8 +18,6 @@ public class ParentPerson extends InheritablePerson { + "(select i2.parent_identifier, count(*) as child_count, sum(i2.age) as child_age from child_person i2 group by i2.parent_identifier) " + "f2 on f2.parent_identifier = ${ta}.identifier"; - private static final String GRAND_PARENT_PERSON_JOIN = "join grand_parent_person j1 on j1.identifier = ${ta}.parent_identifier"; - @ManyToOne(cascade = CascadeType.ALL) private GrandParentPerson parent; @@ -40,7 +38,7 @@ public class ParentPerson extends InheritablePerson { private String address; //@Coalesce({ "familyName", "parent.familyName" }) - @Formula(select = "coalesce(${ta}.family_name, j1.family_name)", join = GRAND_PARENT_PERSON_JOIN) + @Formula2("coalesce(familyName, parent.familyName)") private String effectiveFamilyName; //@Formula2 equivalent - logical path-based, joins are auto-detected @@ -53,10 +51,10 @@ public class ParentPerson extends InheritablePerson { private String lazyDerivedFamilyName; //@Coalesce({ "address", "parent.address" }) - @Formula(select = "coalesce(${ta}.address, j1.address)", join = GRAND_PARENT_PERSON_JOIN) + @Formula2("coalesce(address, parent.address)") private String effectiveAddress; - @Formula(select = "coalesce(${ta}.some_bean_id, j1.some_bean_id)", join = GRAND_PARENT_PERSON_JOIN) + @Formula2("coalesce(someBean.id, parent.someBean.id)") @ManyToOne private EBasic effectiveBean; diff --git a/ebean-test/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java b/ebean-test/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java index a2b0803c8..f790cda0a 100644 --- a/ebean-test/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java +++ b/ebean-test/src/test/java/org/tests/query/joins/TestQueryJoinOnFormula.java @@ -383,12 +383,12 @@ public class TestQueryJoinOnFormula extends BaseTestCase { assertEquals(1, sql.size()); String querySql = sql.get(0); - // only one coalesce(family_name,...) -> the non-transient derivedFamilyName. - // If the transient property leaked into the default select there would be two. + // two non-transient @Formula2 properties with same expression (effectiveFamilyName + derivedFamilyName). + // If the transient lazyDerivedFamilyName also leaked in there would be three. int occurrences = querySql.split("coalesce\\(t0.family_name, t1.family_name\\)", -1).length - 1; assertThat(occurrences) .as("transient @Formula2 must not be selected by default") - .isEqualTo(1); + .isEqualTo(2); } @Test @@ -409,6 +409,91 @@ public class TestQueryJoinOnFormula extends BaseTestCase { assertThat(querySql).contains("left join grand_parent_person t1 on t1.identifier = t0.parent_identifier"); } + /** + * Tests covering the scenario from PR #2773: + * fetch and/or where on a @Formula2 @ManyToOne via path should produce correct SQL joins. + * Previously (with old @Formula + physical join SQL), the join was incorrectly added to the + * root bean instead of the already-existing child node when fetch and where were combined. + * With @Formula2, the join ordering is always correct regardless of fetch/where combination. + */ + @Test + void formula2_manyToOne_fetchOnly() { + LoggedSql.start(); + + DB.find(ChildPerson.class) + .select("name") + .fetch("parent.effectiveBean") + .findList(); + + List sql = LoggedSql.stop(); + assertEquals(1, sql.size()); + String q = sql.get(0); + // parent join must appear, grand_parent join for formula2 dependency, then effectiveBean join + assertThat(q).contains("left join parent_person t1 on t1.identifier = t0.parent_identifier"); + assertThat(q).containsPattern("left join grand_parent_person t\\d+ on t\\d+\\.identifier = t1\\.parent_identifier"); + // effectiveBean join uses formula2: resolves through join aliases (someBean.id, parent.someBean.id) + assertThat(q).containsPattern("left join e_basic t\\d+ on t\\d+\\.id = coalesce\\(t\\d+\\.id, t\\d+\\.id\\)"); + } + + @Test + void formula2_manyToOne_whereOnly() { + LoggedSql.start(); + + DB.find(ChildPerson.class) + .select("name") + .where().eq("parent.effectiveBean.name", "foo") + .findList(); + + List sql = LoggedSql.stop(); + assertEquals(1, sql.size()); + String q = sql.get(0); + assertThat(q).contains("left join parent_person t1 on t1.identifier = t0.parent_identifier"); + assertThat(q).containsPattern("left join grand_parent_person t\\d+ on t\\d+\\.identifier = t1\\.parent_identifier"); + assertThat(q).containsPattern("left join e_basic t\\d+ on t\\d+\\.id = coalesce\\(t\\d+\\.id, t\\d+\\.id\\)"); + assertThat(q).contains("where t"); + assertThat(q).contains(".name = ?"); + } + + @Test + void formula2_manyToOne_fetchPartialWithWhere() { + LoggedSql.start(); + + DB.find(ChildPerson.class) + .select("name") + .fetch("parent", "name") + .where().eq("parent.effectiveBean.name", "foo") + .findList(); + + List sql = LoggedSql.stop(); + assertEquals(1, sql.size()); + String q = sql.get(0); + assertThat(q).contains("left join parent_person t1 on t1.identifier = t0.parent_identifier"); + assertThat(q).containsPattern("left join grand_parent_person t\\d+ on t\\d+\\.identifier = t1\\.parent_identifier"); + assertThat(q).containsPattern("left join e_basic t\\d+ on t\\d+\\.id = coalesce\\(t\\d+\\.id, t\\d+\\.id\\)"); + assertThat(q).contains("where t"); + assertThat(q).contains(".name = ?"); + } + + @Test + void formula2_manyToOne_fetchCompleteWithWhere() { + LoggedSql.start(); + + DB.find(ChildPerson.class) + .select("name") + .fetch("parent") + .where().eq("parent.effectiveBean.name", "foo") + .findList(); + + List sql = LoggedSql.stop(); + assertEquals(1, sql.size()); + String q = sql.get(0); + assertThat(q).contains("left join parent_person t1 on t1.identifier = t0.parent_identifier"); + assertThat(q).containsPattern("left join grand_parent_person t\\d+ on t\\d+\\.identifier = t1\\.parent_identifier"); + assertThat(q).containsPattern("left join e_basic t\\d+ on t\\d+\\.id = coalesce\\(t\\d+\\.id, t\\d+\\.id\\)"); + assertThat(q).contains("where t"); + assertThat(q).contains(".name = ?"); + } + @Test public void test_OrderFindOne() {