From cacb14cf946134bb4359ea3e8fa47bbbf62a3af3 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 29 Aug 2024 11:02:13 +0200 Subject: [PATCH] Avoid selection of duplicate columns. Closes #1865 --- .../data/jdbc/core/convert/SqlGenerator.java | 2 +- .../core/convert/SqlGeneratorUnitTests.java | 72 ++++++++++++------- 2 files changed, 46 insertions(+), 28 deletions(-) diff --git a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java index 08996cba7..480189ab5 100644 --- a/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java +++ b/spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java @@ -514,7 +514,7 @@ class SqlGenerator { Table table = getTable(); - List columnExpressions = new ArrayList<>(); + Set columnExpressions = new LinkedHashSet<>(); List joinTables = new ArrayList<>(); for (PersistentPropertyPath path : mappingContext diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java index a8cb037cc..110c6fe61 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java @@ -43,6 +43,7 @@ import org.springframework.data.relational.core.dialect.SqlServerDialect; import org.springframework.data.relational.core.mapping.AggregatePath; import org.springframework.data.relational.core.mapping.Column; import org.springframework.data.relational.core.mapping.DefaultNamingStrategy; +import org.springframework.data.relational.core.mapping.MappedCollection; import org.springframework.data.relational.core.mapping.RelationalMappingContext; import org.springframework.data.relational.core.mapping.RelationalPersistentEntity; import org.springframework.data.relational.core.mapping.RelationalPersistentProperty; @@ -388,7 +389,8 @@ class SqlGeneratorUnitTests { void findAllByPropertyWithKey() { // this would get called when ListParent is th element type of a Map - String sql = sqlGenerator.getFindAllByProperty(BACKREF, new AggregatePath.ColumnInfo(unquoted("key-column"),unquoted("key-column")), false); + String sql = sqlGenerator.getFindAllByProperty(BACKREF, + new AggregatePath.ColumnInfo(unquoted("key-column"), unquoted("key-column")), false); assertThat(sql).isEqualTo("SELECT dummy_entity.id1 AS id1, dummy_entity.x_name AS x_name, " // + "dummy_entity.x_other AS x_other, " // @@ -451,9 +453,9 @@ class SqlGeneratorUnitTests { Identifier emptyIdentifier = Identifier.of(EMPTY, 0, Object.class); assertThatThrownBy(() -> sqlGenerator.getFindAllByProperty(emptyIdentifier, new AggregatePath.ColumnInfo(unquoted("key-column"), unquoted("key-column")), false)) // - .isInstanceOf(IllegalArgumentException.class) // - .hasMessageContaining( - "An empty SqlIdentifier can't be used in condition. Make sure that all composite primary keys are defined in the query"); + .isInstanceOf(IllegalArgumentException.class) // + .hasMessageContaining( + "An empty SqlIdentifier can't be used in condition. Make sure that all composite primary keys are defined in the query"); } @Test // DATAJDBC-219 @@ -625,18 +627,18 @@ class SqlGeneratorUnitTests { assertThat( createSqlGenerator(Chain4.class).createDeleteByPath(getPath("chain3.chain2.chain1.chain0", Chain4.class))) // - .isEqualTo("DELETE FROM chain0 " + // - "WHERE chain0.chain1 IN (" + // - "SELECT chain1.x_one " + // - "FROM chain1 " + // - "WHERE chain1.chain2 IN (" + // - "SELECT chain2.x_two " + // - "FROM chain2 " + // - "WHERE chain2.chain3 IN (" + // - "SELECT chain3.x_three " + // - "FROM chain3 " + // - "WHERE chain3.chain4 = :rootId" + // - ")))"); + .isEqualTo("DELETE FROM chain0 " + // + "WHERE chain0.chain1 IN (" + // + "SELECT chain1.x_one " + // + "FROM chain1 " + // + "WHERE chain1.chain2 IN (" + // + "SELECT chain2.x_two " + // + "FROM chain2 " + // + "WHERE chain2.chain3 IN (" + // + "SELECT chain3.x_three " + // + "FROM chain3 " + // + "WHERE chain3.chain4 = :rootId" + // + ")))"); } @Test // DATAJDBC-359 @@ -644,7 +646,7 @@ class SqlGeneratorUnitTests { assertThat(createSqlGenerator(NoIdChain4.class) .createDeleteByPath(getPath("chain3.chain2.chain1.chain0", NoIdChain4.class))) // - .isEqualTo("DELETE FROM no_id_chain0 WHERE no_id_chain0.no_id_chain4 = :rootId"); + .isEqualTo("DELETE FROM no_id_chain0 WHERE no_id_chain0.no_id_chain4 = :rootId"); } @Test // DATAJDBC-359 @@ -652,16 +654,16 @@ class SqlGeneratorUnitTests { assertThat(createSqlGenerator(IdIdNoIdChain.class) .createDeleteByPath(getPath("idNoIdChain.chain4.chain3.chain2.chain1.chain0", IdIdNoIdChain.class))) // - .isEqualTo( // - "DELETE FROM no_id_chain0 " // - + "WHERE no_id_chain0.no_id_chain4 IN (" // - + "SELECT no_id_chain4.x_four " // - + "FROM no_id_chain4 " // - + "WHERE no_id_chain4.id_no_id_chain IN (" // - + "SELECT id_no_id_chain.x_id " // - + "FROM id_no_id_chain " // - + "WHERE id_no_id_chain.id_id_no_id_chain = :rootId" // - + "))"); + .isEqualTo( // + "DELETE FROM no_id_chain0 " // + + "WHERE no_id_chain0.no_id_chain4 IN (" // + + "SELECT no_id_chain4.x_four " // + + "FROM no_id_chain4 " // + + "WHERE no_id_chain4.id_no_id_chain IN (" // + + "SELECT id_no_id_chain.x_id " // + + "FROM id_no_id_chain " // + + "WHERE id_no_id_chain.id_id_no_id_chain = :rootId" // + + "))"); } @Test // DATAJDBC-340 @@ -926,6 +928,16 @@ class SqlGeneratorUnitTests { "WHERE referenced_entity.parentId"); } + @Test // GH-1865 + void mappingMapKeyToChildShouldNotResultInDuplicateColumn() { + + SqlGenerator sqlGenerator = createSqlGenerator(Child.class); + String sql = sqlGenerator.getFindAllByProperty(Identifier.of(unquoted("parent"), 23, Parent.class), + context.getAggregatePath(getPath("children", Parent.class)).getTableInfo().qualifierColumnInfo(), false); + + assertThat(sql).containsOnlyOnce("child.NICK_NAME AS NICK_NAME"); + } + @Nullable private SqlIdentifier getAlias(Object maybeAliased) { @@ -1117,4 +1129,10 @@ class SqlGeneratorUnitTests { @Id Long id; IdNoIdChain idNoIdChain; } + + record Parent(@Id Long id, String name, @MappedCollection(keyColumn = "NICK_NAME") Map children) { + } + + record Child(@Column("NICK_NAME") String nickName, String name) { + } }