Skip to content

Commit

Permalink
Merge pull request #3474 from ebean-orm/feature/3453
Browse files Browse the repository at this point in the history
#3453 - Different filterMany behavior when no matches found
  • Loading branch information
rbygrave authored Sep 11, 2024
2 parents cbac00a + dc7d349 commit 0964980
Show file tree
Hide file tree
Showing 14 changed files with 70 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,9 @@ public interface SpiQueryManyJoin {
* Order by clause defined via mapping on the ToMany property.
*/
String fetchOrderBy();

/**
* Wrap the filter many expression with a condition allowing lEFT JOIN null matching row.
*/
String idNullOr(String filterManyExpression);
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ public String toString() {
return "prefix:" + prefix + " name:" + name + " dbColumn:" + dbColumn + " ph:" + placeHolder;
}

@Override
public String idNullOr(String filterManyExpression) {
throw new UnsupportedOperationException();
}

@Override
public boolean isAggregation() {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,11 @@ public boolean isAssignableFrom(Class<?> type) {
return owningType.isAssignableFrom(type);
}

@Override
public String idNullOr(String filterManyExpression) {
throw new UnsupportedOperationException();
}

@Override
public void loadIgnore(DbReadContext ctx) {
ctx.dataReader().incrementPos(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,11 @@ public String fetchOrderBy() {
return fetchOrderBy;
}

@Override
public String idNullOr(String filterManyExpression) {
return targetIdBinder.idNullOr(name, filterManyExpression);
}

/**
* Return the order by for use when lazy loading the associated collection.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ public interface IdBinder {
*/
void initialise();

/**
* Wrap the filter many expression with a condition allowing lEFT JOIN null matching row.
*/
String idNullOr(String name, String filterManyExpression);

String idSelect();

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,20 @@ public void initialise() {
this.idInValueSql = idInExpandedForm ? idInExpanded() : idInCompressed();
}

@Override
public String idNullOr(String prefix, String filterManyExpression) {
StringBuilder sb = new StringBuilder(100);
sb.append("((");
for (int i = 0; i < props.length; i++) {
if (i > 0) {
sb.append(" and ");
}
sb.append("${").append(prefix).append('.').append(embIdProperty.name()).append('}').append(props[i].name()).append(" is null");
}
sb.append(") or (").append(filterManyExpression).append("))");
return sb.toString();
}

@Override
public String idSelect() {
return embIdProperty.name();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ public IdBinderEmpty() {
public void initialise() {
}

@Override
public String idNullOr(String name, String filterManyExpression) {
throw new UnsupportedOperationException();
}

@Override
public String idSelect() {
return "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,4 +260,9 @@ public String cacheKeyFromBean(EntityBean bean) {
final Object value = idProperty.getValue(bean);
return scalarType.format(value);
}

@Override
public String idNullOr(String prefix, String filterManyExpression) {
return "(${" + prefix + "}" + idProperty.name() + " is null or (" + filterManyExpression + "))";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -286,4 +286,8 @@ public void pathSet(Object bean, Object value) {
}
}

@Override
public String idNullOr(String filterManyExpression) {
throw new UnsupportedOperationException();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public void prepare(boolean buildSql) {
if (filterManyExpr != null) {
this.filterMany = new DefaultExpressionRequest(request, deployParser, binder, filterManyExpr);
if (buildSql) {
dbFilterMany = filterMany.buildSql();
dbFilterMany = manyProperty.idNullOr(filterMany.buildSql());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ void filterMany() {
.query();

q.findList();
assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterMany */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where t1.first_name like ? escape'|' and t1.email is not null order by t0.id");
assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterMany */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where (t1.id is null or (t1.first_name like ? escape'|' and t1.email is not null)) order by t0.id");
}

@Test
Expand All @@ -313,7 +313,7 @@ void filterManySingle() {
.query();

q.findList();
assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterManySingle */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where t1.first_name like ? escape'|' order by t0.id");
assertThat(q.getGeneratedSql()).isEqualTo("select /* QCustomerTest.filterManySingle */ t0.id, t0.name, t1.id, t1.first_name, t1.last_name from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where (t1.id is null or (t1.first_name like ? escape'|')) order by t0.id");
}

@Test
Expand Down Expand Up @@ -348,7 +348,7 @@ void filterManySingleQuery() {
.query();

q.findList();
assertThat(q.getGeneratedSql()).contains(" from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where t1.first_name like ? and t1.first_name like ");
assertThat(q.getGeneratedSql()).contains(" from be_customer t0 left join be_contact t1 on t1.customer_id = t0.id where (t1.id is null or (t1.first_name like ? and t1.first_name like ? escape'|')) order by t0.id");
}

@Test
Expand Down
21 changes: 10 additions & 11 deletions ebean-test/src/test/java/org/tests/query/TestQueryFilterMany.java
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ public void filterManyRaw_singleQuery() {
assertThat(customers).isNotEmpty();
List<String> sqlList = LoggedSql.stop();
assertEquals(1, sqlList.size());
assertThat(sqlList.get(0)).contains(" where lower(t0.name) = ? and t1.status = ? order by t0.id");
assertThat(sqlList.get(0)).contains(" where lower(t0.name) = ? and (t1.id is null or (t1.status = ?)) order by t0.id");
}

@Test
Expand Down Expand Up @@ -212,8 +212,7 @@ public void test_with_findOne_rawSameQuery() {
List<String> sql = LoggedSql.stop();
assertThat(sql).hasSize(1);
assertThat(sql.get(0)).contains("from o_customer t0 left join o_order t1");
assertThat(sql.get(0)).contains("where t1.order_date is not null");
assertThat(sql.get(0)).contains("order by t0.id");
assertThat(sql.get(0)).contains("where (t1.id is null or (t1.order_date is not null)) order by t0.id");
}

@Test
Expand Down Expand Up @@ -250,7 +249,7 @@ public void test_filterMany_with_isNotEmpty() {

List<String> sqlList = LoggedSql.stop();
assertEquals(1, sqlList.size());
assertThat(sqlList.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 where exists (select 1 from o_order x where x.kcustomer_id = t0.id and x.order_date is not null) and 1=0 order by t0.id");
assertThat(sqlList.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 where exists (select 1 from o_order x where x.kcustomer_id = t0.id and x.order_date is not null) and (t1.id is null or (1=0)) order by t0.id");
}

@Test
Expand Down Expand Up @@ -288,11 +287,11 @@ public void test_filterMany_copy_findList() {

List<String> sqlList = LoggedSql.stop();
assertEquals(1, sqlList.size());
assertThat(sqlList.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 where t1.status ");
assertThat(sqlList.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 where (t1.id is null or (t1.status ");
if (isPostgresCompatible()) {
assertThat(sqlList.get(0)).contains("where t1.status = any(?) order by t0.id");
assertThat(sqlList.get(0)).contains("where (t1.id is null or (t1.status = any(?) order by t0.id");
} else {
assertThat(sqlList.get(0)).contains("where t1.status in (?) order by t0.id");
assertThat(sqlList.get(0)).contains("where (t1.id is null or (t1.status in (?))) order by t0.id");
}
}

Expand Down Expand Up @@ -336,7 +335,7 @@ public void testDisjunction() {

List<String> sql = LoggedSql.stop();
assertEquals(1, sql.size());
assertSql(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 where (t1.status = ? or t1.order_date = ?) order by t0.id");
assertSql(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 where (t1.id is null or ((t1.status = ? or t1.order_date = ?))) order by t0.id");
}

@Test
Expand All @@ -352,7 +351,7 @@ public void testNestedFilterMany() {
List<String> sql = LoggedSql.stop();

assertThat(sql.size()).isGreaterThan(1);
assertSql(sql.get(0)).contains(" from o_customer t0 left join contact t1 on t1.customer_id = t0.id where t1.first_name is not null order by t0.id; --bind()");
assertSql(sql.get(0)).contains(" from o_customer t0 left join contact t1 on t1.customer_id = t0.id where (t1.id is null or (t1.first_name is not null)) order by t0.id; --bind()");
platformAssertIn(sql.get(1), " from contact_note t0 where (t0.contact_id)");
assertSql(sql.get(1)).contains(" and lower(t0.title) like");
}
Expand Down Expand Up @@ -390,9 +389,9 @@ public void testFilterManyUsingExpression() {

assertThat(sql).hasSize(1);
if (isSqlServer()) {
assertSql(sql.get(0)).contains(" from o_customer t0 left join contact t1 on t1.customer_id = t0.id where (t1.first_name is not null and lower(t1.email) like ?) order by t0.id; --bind(rob%)");
assertSql(sql.get(0)).contains(" from o_customer t0 left join contact t1 on t1.customer_id = t0.id where (t1.id is null or ((t1.first_name is not null and lower(t1.email) like ?))) order by t0.id; --bind(rob%)");
} else {
assertSql(sql.get(0)).contains(" from o_customer t0 left join contact t1 on t1.customer_id = t0.id where (t1.first_name is not null and lower(t1.email) like ? escape'|') order by t0.id; --bind(rob%)");
assertSql(sql.get(0)).contains(" from o_customer t0 left join contact t1 on t1.customer_id = t0.id where (t1.id is null or ((t1.first_name is not null and lower(t1.email) like ? escape'|'))) order by t0.id; --bind(rob%)");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public void test() {

List<String> sql = LoggedSql.stop();
assertThat(sql).hasSize(1);
assertThat(sql.get(0)).contains("select t0.userid, t0.user_name, t0.user_type_id, t1.roleid, t1.role_name from muser t0 left join mrole_muser t1z_ on t1z_.muser_userid = t0.userid left join mrole t1 on t1.roleid = t1z_.mrole_roleid where lower(t1.role_name) like");
assertThat(sql.get(0)).contains("select t0.userid, t0.user_name, t0.user_type_id, t1.roleid, t1.role_name from muser t0 left join mrole_muser t1z_ on t1z_.muser_userid = t0.userid left join mrole t1 on t1.roleid = t1z_.mrole_roleid where (t1.roleid is null or (lower(t1.role_name) like ");
assertThat(sql.get(0)).contains("order by t0.userid;");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -787,10 +787,10 @@ void oneToMany_notNull() {
List<Order.Status> statusList = query.findSingleAttributeList();
assertSql(query)
.contains("select distinct t1.status from o_customer t0 "
+ "left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null where t1.status is not null")
+ "left join o_order t1 on t1.kcustomer_id = t0.id and t1.order_date is not null where (t1.id is null or (t1.status is not null))")
.doesNotContain("order by");

assertThat(statusList).hasSize(3);
assertThat(statusList).hasSize(4);
}

@Test
Expand Down

0 comments on commit 0964980

Please sign in to comment.