mirror of
https://github.com/ebean-orm/ebean.git
synced 2026-09-21 03:18:56 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
99c3d8b07b | ||
|
|
9947a41a9b | ||
|
|
055a3bf638 | ||
|
|
f6ff485041 | ||
|
|
b67e28e60a |
@@ -12,6 +12,7 @@ public final class SpiExpressionValidation {
|
||||
|
||||
private final BeanType<?> desc;
|
||||
private final LinkedHashSet<String> unknown = new LinkedHashSet<>();
|
||||
private final LinkedHashSet<String> all = new LinkedHashSet<>();
|
||||
|
||||
public SpiExpressionValidation(BeanType<?> desc) {
|
||||
this.desc = desc;
|
||||
@@ -21,6 +22,7 @@ public final class SpiExpressionValidation {
|
||||
* Validate that the property expression (path) is valid.
|
||||
*/
|
||||
public void validate(String propertyName) {
|
||||
all.add(propertyName);
|
||||
if (!desc.isValidExpression(propertyName)) {
|
||||
unknown.add(propertyName);
|
||||
}
|
||||
@@ -33,4 +35,14 @@ public final class SpiExpressionValidation {
|
||||
return unknown;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the set of all property names visited during this validation, regardless of
|
||||
* whether they were considered valid against the bean type. Used to inspect the shape of
|
||||
* an expression (for example, to check whether it references any associated/joined path)
|
||||
* without needing a correctly-typed bean descriptor.
|
||||
*/
|
||||
public Set<String> allProperties() {
|
||||
return all;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -181,4 +181,11 @@ public interface DbSqlContext {
|
||||
* Include the filter many predicates if specified into the JOIN clause.
|
||||
*/
|
||||
void includeFilterMany();
|
||||
|
||||
/**
|
||||
* Return true if the given fetch path (relative to the query root) is the exact join clause
|
||||
* that the pending filterMany predicate must be attached to - i.e. the deepest path the
|
||||
* filterMany expression itself references.
|
||||
*/
|
||||
boolean isFilterManyAttachPoint(String prefix);
|
||||
}
|
||||
|
||||
@@ -78,6 +78,11 @@ public final class CQueryPredicates {
|
||||
*/
|
||||
private Set<String> predicateIncludes;
|
||||
private Set<String> orderByIncludes;
|
||||
/**
|
||||
* The fetch path (relative to the query root) of the many-root whose own join clause the
|
||||
* filterMany-in-JOIN predicate is attached to
|
||||
*/
|
||||
private String filterManyAttachPath;
|
||||
|
||||
CQueryPredicates(Binder binder, OrmQueryRequest<?> request) {
|
||||
this.binder = binder;
|
||||
@@ -222,6 +227,8 @@ public final class CQueryPredicates {
|
||||
filterMany = new DefaultExpressionRequest(request, deployParser, binder, filterManyExpr);
|
||||
if (buildSql) {
|
||||
dbFilterMany = filterMany.buildSql();
|
||||
// safe as filterManyJoin only holds when the expression is root-property only -
|
||||
filterManyAttachPath = manyProperty.path();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -399,6 +406,14 @@ public final class CQueryPredicates {
|
||||
return filterManyJoin ? dbFilterMany : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the fetch path of the filterMany-in-JOIN predicate - the path whose own join clause
|
||||
* the predicate must be appended to (or null if there is no filterMany-in-JOIN predicate at all).
|
||||
*/
|
||||
String filterManyAttachPath() {
|
||||
return filterManyJoin ? filterManyAttachPath : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the db column version of the order by clause.
|
||||
*/
|
||||
|
||||
@@ -23,6 +23,7 @@ final class DefaultDbSqlContext implements DbSqlContext {
|
||||
private final ArrayStack<String> prefixStack = new ArrayStack<>();
|
||||
private final String fromForUpdate;
|
||||
private final String dbFilterManyJoin;
|
||||
private final String filterManyAttachPath;
|
||||
private boolean useColumnAlias;
|
||||
private int columnIndex;
|
||||
private int asOfTableCount;
|
||||
@@ -42,7 +43,8 @@ final class DefaultDbSqlContext implements DbSqlContext {
|
||||
private boolean joinSuppressed;
|
||||
|
||||
DefaultDbSqlContext(SqlTreeAlias alias, String columnAliasPrefix, CQueryHistorySupport historySupport,
|
||||
CQueryDraftSupport draftSupport, String fromForUpdate, String dbFilterManyJoin) {
|
||||
CQueryDraftSupport draftSupport, String fromForUpdate, String dbFilterManyJoin,
|
||||
String filterManyAttachPath) {
|
||||
this.alias = alias;
|
||||
this.columnAliasPrefix = columnAliasPrefix;
|
||||
this.useColumnAlias = columnAliasPrefix != null;
|
||||
@@ -51,6 +53,12 @@ final class DefaultDbSqlContext implements DbSqlContext {
|
||||
this.historyQuery = (historySupport != null);
|
||||
this.fromForUpdate = fromForUpdate;
|
||||
this.dbFilterManyJoin = dbFilterManyJoin;
|
||||
this.filterManyAttachPath = filterManyAttachPath;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean isFilterManyAttachPoint(String prefix) {
|
||||
return dbFilterManyJoin != null && filterManyAttachPath != null && filterManyAttachPath.equals(prefix);
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -108,7 +108,7 @@ public final class SqlTreeBuilder {
|
||||
CQueryHistorySupport historySupport = builder.historySupport(query);
|
||||
CQueryDraftSupport draftSupport = builder.draftSupport(query);
|
||||
String colAlias = subQuery || rootNode.isSingleProperty() ? null : columnAliasPrefix;
|
||||
this.ctx = new DefaultDbSqlContext(alias, colAlias, historySupport, draftSupport, fromForUpdate, predicates.dbFilterManyJoin());
|
||||
this.ctx = new DefaultDbSqlContext(alias, colAlias, historySupport, draftSupport, fromForUpdate, predicates.dbFilterManyJoin(), predicates.filterManyAttachPath());
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -342,6 +342,10 @@ class SqlTreeNodeBean implements SqlTreeNode {
|
||||
if (desc.isSoftDelete() && temporalMode != SpiQuery.TemporalMode.SOFT_DELETED) {
|
||||
ctx.append(" and ").append(desc.softDeletePredicate(ctx.tableAlias(prefix)));
|
||||
}
|
||||
if (prefix != null && ctx.isFilterManyAttachPoint(prefix)) {
|
||||
// this node is where we inline the filterMany predicate
|
||||
ctx.includeFilterMany();
|
||||
}
|
||||
return sqlJoinType;
|
||||
}
|
||||
|
||||
|
||||
@@ -45,6 +45,5 @@ final class SqlTreeNodeManyRoot extends SqlTreeNodeBean {
|
||||
@Override
|
||||
public void appendFrom(DbSqlContext ctx, SqlJoinType joinType) {
|
||||
super.appendFrom(ctx, joinType.autoToOuter());
|
||||
ctx.includeFilterMany();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -384,14 +384,18 @@ public final class OrmQueryDetail implements Serializable {
|
||||
OrmQueryProperties chunk = pair.getProperties();
|
||||
if (isQueryJoinCandidate(lazyLoadManyPath, chunk)) {
|
||||
// this is a 'fetch join' (included in main query)
|
||||
if (fetchJoinFirstMany) {
|
||||
BeanDescriptor<?> targetDescriptor = ((BeanPropertyAssoc<?>) elProp.beanProperty()).targetDescriptor();
|
||||
if (fetchJoinFirstMany && !chunk.filterManyHasNestedProperty(targetDescriptor)) {
|
||||
// letting the first one remain a 'fetch join'
|
||||
fetchJoinFirstMany = false;
|
||||
manyFetchProperty = pair.getPath();
|
||||
chunk.filterManyInline();
|
||||
many = elProp;
|
||||
} else {
|
||||
// convert this one over to a 'query join'
|
||||
// convert this one over to a 'query join' - either because another many has already claimed the
|
||||
// 'fetch join' slot, or because its filterMany references a property that requires crossing into
|
||||
// an associated bean and can't safely be included as a JOIN predicate (see
|
||||
// OrmQueryProperties.filterManyHasNestedProperty)
|
||||
chunk.markForQueryJoin();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,7 +9,10 @@ import io.ebean.util.SplitName;
|
||||
import io.ebeaninternal.api.SpiExpression;
|
||||
import io.ebeaninternal.api.SpiExpressionFactory;
|
||||
import io.ebeaninternal.api.SpiExpressionList;
|
||||
import io.ebeaninternal.api.SpiExpressionValidation;
|
||||
import io.ebeaninternal.api.SpiQuery;
|
||||
import io.ebeaninternal.server.deploy.BeanDescriptor;
|
||||
import io.ebeaninternal.server.el.ElPropertyValue;
|
||||
import io.ebeaninternal.server.expression.FilterExprPath;
|
||||
import io.ebeaninternal.server.expression.FilterExpressionList;
|
||||
|
||||
@@ -234,6 +237,26 @@ public final class OrmQueryProperties implements Serializable {
|
||||
return filterMany != null && !markForQueryJoin;
|
||||
}
|
||||
|
||||
/**
|
||||
* Return true if the filterMany expression (if any) references a property that requires
|
||||
* crossing into an associated bean/join - e.g. {@code "group.name"} - rather than only
|
||||
* plain/embedded properties resolving to columns on the many bean's own base table.
|
||||
*/
|
||||
boolean filterManyHasNestedProperty(BeanDescriptor<?> targetDescriptor) {
|
||||
if (filterMany == null) {
|
||||
return false;
|
||||
}
|
||||
SpiExpressionValidation validation = new SpiExpressionValidation(targetDescriptor);
|
||||
filterMany.validate(validation);
|
||||
for (String property : validation.allProperties()) {
|
||||
ElPropertyValue elProp = targetDescriptor.elGetValue(property);
|
||||
if (elProp != null && elProp.isAssocProperty()) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Adjust filterMany expressions for inclusion in main query.
|
||||
*/
|
||||
|
||||
@@ -174,7 +174,7 @@ public class TestQueryFilterMany extends BaseTestCase {
|
||||
assertThat(customers).isNotEmpty();
|
||||
List<String> sqlList = LoggedSql.stop();
|
||||
assertEquals(1, sqlList.size());
|
||||
assertThat(sqlList.get(0)).contains(" left join o_customer t2 on t2.id = t1.kcustomer_id and t1.status = ? where ");
|
||||
assertThat(sqlList.get(0)).contains(" left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null and t1.status = ? left join o_customer t2 on t2.id = t1.kcustomer_id where ");
|
||||
assertThat(sqlList.get(0)).contains(" where lower(t0.name) = ? order by t0.id");
|
||||
}
|
||||
|
||||
@@ -211,7 +211,7 @@ public class TestQueryFilterMany extends BaseTestCase {
|
||||
assertThat(result).isNotEmpty();
|
||||
List<String> sql = LoggedSql.stop();
|
||||
assertThat(sql).hasSize(1);
|
||||
assertThat(sql.get(0)).contains("from o_customer t0 left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null left join o_customer t2 on t2.id = t1.kcustomer_id and t1.order_date is not null order by t0.id");
|
||||
assertThat(sql.get(0)).contains("from o_customer t0 left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null and t1.order_date is not null left join o_customer t2 on t2.id = t1.kcustomer_id order by t0.id");
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -340,9 +340,9 @@ public class TestQueryFilterMany extends BaseTestCase {
|
||||
List<String> sqlList = LoggedSql.stop();
|
||||
assertEquals(1, sqlList.size());
|
||||
if (isPostgresCompatible()) {
|
||||
assertThat(sqlList.get(0)).contains("left join o_customer t2 on t2.id = t1.kcustomer_id and t1.status = any(?) order by t0.id");
|
||||
assertThat(sqlList.get(0)).contains("left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null and t1.status = any(?) left join o_customer t2 on t2.id = t1.kcustomer_id order by t0.id");
|
||||
} else {
|
||||
assertThat(sqlList.get(0)).contains("left join o_customer t2 on t2.id = t1.kcustomer_id and t1.status in (?) order by t0.id");
|
||||
assertThat(sqlList.get(0)).contains("left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null and t1.status in (?) left join o_customer t2 on t2.id = t1.kcustomer_id order by t0.id");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -386,7 +386,7 @@ public class TestQueryFilterMany extends BaseTestCase {
|
||||
|
||||
List<String> sql = LoggedSql.stop();
|
||||
assertEquals(1, sql.size());
|
||||
assertSql(sql.get(0)).contains(" left join o_customer t2 on t2.id = t1.kcustomer_id and (t1.status = ? or t1.order_date = ?) order by t0.id");
|
||||
assertSql(sql.get(0)).contains(" left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null and (t1.status = ? or t1.order_date = ?) left join o_customer t2 on t2.id = t1.kcustomer_id order by t0.id");
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -459,7 +459,10 @@ public class TestQueryFilterMany extends BaseTestCase {
|
||||
|
||||
List<String> sql = LoggedSql.stop();
|
||||
|
||||
assertThat(sql).hasSize(1);
|
||||
assertSql(sql.get(0)).contains(" from o_customer t0 left join contact t1 on t1.customer_id = t0.id left join contact_group t2 on t2.id = t1.group_id and t2.name = ? and t1.cretime is not null order by t0.id");
|
||||
// nested "group.name" reference forces this to a query join so the filter is applied
|
||||
// as a genuine WHERE clause (not misapplied to a LEFT JOIN's ON clause)
|
||||
assertThat(sql).hasSize(2);
|
||||
assertSql(sql.get(1)).contains(" from contact t0 left join contact_group t1 on t1.id = t0.group_id where (t0.customer_id) in (");
|
||||
assertSql(sql.get(1)).contains(" and t1.name = ? and t0.cretime is not null");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,7 +35,7 @@ public class TestQueryFilterManySimple extends BaseTestCase {
|
||||
list.get(0).getOrders().size();
|
||||
List<String> sql = LoggedSql.stop();
|
||||
assertThat(sql).hasSize(2);
|
||||
assertThat(sql.get(0)).contains("left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null left join o_customer t2 on t2.id = t1.kcustomer_id and t1.status = ? and t1.order_date > ?");
|
||||
assertThat(sql.get(0)).contains("left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null and t1.status = ? and t1.order_date > ? left join o_customer t2 on t2.id = t1.kcustomer_id");
|
||||
assertThat(sql.get(0)).contains("order by t0.id");
|
||||
if (isPostgresCompatible()) {
|
||||
assertThat(sql.get(1)).contains("from contact t0 where (t0.customer_id) = any(?) and t0.first_name is not null;");
|
||||
@@ -55,7 +55,10 @@ public class TestQueryFilterManySimple extends BaseTestCase {
|
||||
.findList();
|
||||
|
||||
List<String> sql = LoggedSql.stop();
|
||||
assertThat(sql).hasSize(1);
|
||||
assertThat(sql.get(0)).contains("from o_customer t0 left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null left join o_customer t2 on t2.id = t1.kcustomer_id and t2.status = ? order by t0.id");
|
||||
// nested "customer.status" reference forces this to a query join so the filter is
|
||||
// applied as a genuine WHERE clause (not misapplied to a LEFT JOIN's ON clause)
|
||||
assertThat(sql).hasSize(2);
|
||||
assertThat(sql.get(1)).contains("from o_order t0 join o_customer t1 on t1.id = t0.kcustomer_id where t0.order_date is not null and (t0.kcustomer_id) in (");
|
||||
assertThat(sql.get(1)).contains(" and t1.status = ?");
|
||||
}
|
||||
}
|
||||
|
||||
+73
@@ -0,0 +1,73 @@
|
||||
package org.tests.query;
|
||||
|
||||
import io.ebean.DB;
|
||||
import io.ebean.test.LoggedSql;
|
||||
import io.ebean.xtest.BaseTestCase;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.tests.model.basic.Contact;
|
||||
import org.tests.model.basic.Customer;
|
||||
import org.tests.model.basic.ResetBasicData;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* Reproduces a bug where filterMany() on a property of the many-root itself is misapplied
|
||||
* to the ON clause of a deeper, unrelated nested fetch join instead of the many-root's own join
|
||||
* clause - when an additional fetch path exists beneath the many property that the filterMany
|
||||
* expression itself does not reference.
|
||||
*/
|
||||
public class TestQueryFilterManyWithDeeperNestedFetch extends BaseTestCase {
|
||||
|
||||
@Test
|
||||
void filterMany_onRootProperty_withUnrelatedDeeperNestedFetch_expectFilterOnOwnJoin() {
|
||||
ResetBasicData.reset();
|
||||
|
||||
Customer customer = DB.find(Customer.class).where().ieq("name", "Rob").findOne();
|
||||
assertThat(customer).isNotNull();
|
||||
|
||||
List<Contact> allContacts = DB.find(Contact.class).where().eq("customer", customer).findList();
|
||||
assertThat(allContacts).isNotEmpty();
|
||||
|
||||
// ensure at least one contact isMember=true and one isMember=false
|
||||
Contact memberContact = allContacts.get(0);
|
||||
memberContact.setMember(true);
|
||||
DB.save(memberContact);
|
||||
Contact nonMemberContact;
|
||||
if (allContacts.size() > 1) {
|
||||
nonMemberContact = allContacts.get(1);
|
||||
} else {
|
||||
nonMemberContact = new Contact();
|
||||
nonMemberContact.setFirstName("Extra");
|
||||
nonMemberContact.setLastName("NonMember");
|
||||
nonMemberContact.setCustomer(customer);
|
||||
}
|
||||
nonMemberContact.setMember(false);
|
||||
DB.save(nonMemberContact);
|
||||
|
||||
LoggedSql.start();
|
||||
// filterMany only references "isMember" (a property of Contact - the many-root itself) but
|
||||
// the query ALSO fetches a further nested path beneath "contacts" (contacts.group) that the
|
||||
// filterMany expression does NOT reference at all.
|
||||
List<Customer> found = DB.find(Customer.class)
|
||||
.setBeanCacheMode(io.ebean.CacheMode.OFF)
|
||||
.setPersistenceContextScope(io.ebean.PersistenceContextScope.QUERY)
|
||||
.fetch("contacts", "id,firstName,lastName,isMember")
|
||||
.fetch("contacts.group", "id,name")
|
||||
.filterMany("contacts").eq("isMember", true)
|
||||
.where().idEq(customer.getId())
|
||||
.findList();
|
||||
|
||||
List<String> sql = LoggedSql.stop();
|
||||
assertThat(found).isNotEmpty();
|
||||
assertThat(sql).hasSize(1);
|
||||
// the filterMany predicate must be on contact's own join (t1), not misapplied to the
|
||||
// unrelated deeper contact_group join (t2)
|
||||
assertThat(sql.get(0)).contains("left join contact t1 on t1.customer_id = t0.id and t1.is_member = ? left join contact_group t2 on t2.id = t1.group_id");
|
||||
|
||||
// every contact returned must be a member - the filter must actually exclude non-members
|
||||
assertThat(found.get(0).getContacts()).isNotEmpty();
|
||||
assertThat(found.get(0).getContacts()).allMatch(Contact::isMember);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user