From fd4be0ca59eefdcef1aded3af342f7a847f4d7fc Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Mon, 14 Mar 2022 16:16:14 +1300 Subject: [PATCH] #2593 - Tidy SqlTreeBuilder to not add duplicate BeanProperties --- .../io/ebeaninternal/server/query/SqlTreeBuilder.java | 5 ++--- .../ebeaninternal/server/query/SqlTreeProperties.java | 6 ++++++ .../src/test/java/org/tests/model/nofk/TestNoFk.java | 11 +++++++++-- .../org/tests/query/TestQueryMultiJoinFetchPath.java | 5 ++--- 4 files changed, 19 insertions(+), 8 deletions(-) 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 0e4f11079..c0142ddbc 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 @@ -224,8 +224,7 @@ public final class SqlTreeBuilder { * Recursively build the query tree depending on what leaves in the tree * should be included. */ - private SqlTreeNode buildSelectChain(String prefix, STreePropertyAssoc prop, - STreeType desc, List joinList) { + private SqlTreeNode buildSelectChain(String prefix, STreePropertyAssoc prop, STreeType desc, List joinList) { List myJoinList = new ArrayList<>(); List extraProps = new ArrayList<>(); for (STreePropertyAssocOne one : desc.propsOne()) { @@ -256,7 +255,7 @@ public final class SqlTreeBuilder { } addManyWhereJoins(myJoinList); } - extraProps.forEach(props::add); //addExtra + extraProps.forEach(props::addExtra); if (!rawSql && manyWhereJoins.isFormulaWithJoin(prefix)) { for (String property : manyWhereJoins.getFormulaJoinProperties(prefix)) { diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java index 623b1dd36..806af5ef6 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeProperties.java @@ -33,6 +33,12 @@ public final class SqlTreeProperties { propsList.addAll(Arrays.asList(props)); } + public void addExtra(STreePropertyAssocOne prop) { + if (!propsList.contains(prop)) { + add(prop); + } + } + public void add(STreeProperty prop) { propsList.add(prop); propNames.add(prop.name()); diff --git a/ebean-test/src/test/java/org/tests/model/nofk/TestNoFk.java b/ebean-test/src/test/java/org/tests/model/nofk/TestNoFk.java index e27df52db..ffb853f10 100644 --- a/ebean-test/src/test/java/org/tests/model/nofk/TestNoFk.java +++ b/ebean-test/src/test/java/org/tests/model/nofk/TestNoFk.java @@ -5,6 +5,7 @@ import io.ebean.DB; import io.ebean.SqlRow; import io.ebean.bean.EntityBean; import io.ebean.bean.EntityBeanIntercept; +import io.ebean.test.LoggedSql; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; @@ -142,13 +143,16 @@ public class TestNoFk extends BaseTestCase { assertTrue(ownerEbi.isReference()); assertTrue(ownerEbi.isPartial()); assertTrue(ownerEbi.isLazyLoadFailure()); - } @Test public void testEagerLoadFile() { + LoggedSql.start(); List files = DB.find(EFileNoFk.class).fetch("owner").findList(); assertThat(files).hasSize(2); + List sql = LoggedSql.stop(); + assertThat(sql).hasSize(1); + assertThat(sql.get(0)).contains("select t0.file_name, t0.owner_user_id, t0.owner_soft_del_user_id, t1.user_id, t1.user_name from efile_no_fk t0 left join euser_no_fk t1 on t1.user_id = t0.owner_user_id"); EFileNoFk file1 = files.get(0); EFileNoFk file2 = files.get(1); @@ -175,7 +179,6 @@ public class TestNoFk extends BaseTestCase { assertTrue(ownerEbi.isReference()); assertTrue(ownerEbi.isPartial()); assertTrue(ownerEbi.isLazyLoadFailure()); - } @Test @@ -259,8 +262,12 @@ public class TestNoFk extends BaseTestCase { @Test public void testEagerLoadFileSoftDel() { + LoggedSql.start(); List files = DB.find(EFileNoFk.class).fetch("ownerSoftDel").findList(); assertThat(files).hasSize(2); + List sql = LoggedSql.stop(); + assertThat(sql).hasSize(1); + assertThat(sql.get(0)).contains("select t0.file_name, t0.owner_user_id, t0.owner_soft_del_user_id, t1.user_id, t1.user_name, t1.user_id is null from efile_no_fk t0 left join euser_no_fk_soft_del t1 on t1.user_id = t0.owner_soft_del_user_id"); EFileNoFk file1 = files.get(0); EFileNoFk file2 = files.get(1); diff --git a/ebean-test/src/test/java/org/tests/query/TestQueryMultiJoinFetchPath.java b/ebean-test/src/test/java/org/tests/query/TestQueryMultiJoinFetchPath.java index e8f1a81be..4f439a58e 100644 --- a/ebean-test/src/test/java/org/tests/query/TestQueryMultiJoinFetchPath.java +++ b/ebean-test/src/test/java/org/tests/query/TestQueryMultiJoinFetchPath.java @@ -14,7 +14,6 @@ class TestQueryMultiJoinFetchPath extends BaseTestCase { @Test void test() { - HCustomer c1 = new HCustomer("c1", "c1"); DB.save(c1); @@ -60,9 +59,9 @@ class TestQueryMultiJoinFetchPath extends BaseTestCase { assertThat(accesses).hasSize(2); if (isH2()) { - assertThat(query.getGeneratedSql()).isEqualTo("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t0.accessor_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id in (?,?,?,?,?)"); + assertThat(query.getGeneratedSql()).isEqualTo("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id in (?,?,?,?,?)"); } else { - assertThat(query.getGeneratedSql()).contains("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t0.accessor_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id "); + assertThat(query.getGeneratedSql()).contains("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id "); } }