Avoid selection of duplicate columns.

Closes #1865
spring-projects · Aug 29, 2024 · 9b0b91b · 9b0b91b
1 parent 07e13aa
commit 9b0b91b
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 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
@@ -514,7 +514,7 @@ private SelectBuilder.SelectWhere selectBuilder(Collection<SqlIdentifier> keyCol
 
 		Table table = getTable();
 
-		List<Expression> columnExpressions = new ArrayList<>();
+		Set<Expression> columnExpressions = new LinkedHashSet<>();
 
 		List<Join> joinTables = new ArrayList<>();
 		for (PersistentPropertyPath<RelationalPersistentProperty> path : mappingContext

diff --git a/...-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java b/...-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java
@@ -43,6 +43,7 @@
 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 @@ void findAllByPropertyWithMultipartIdentifier() {
 	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 @@ void findAllByPropertyWithEmptyBackrefColumn() {
 		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,43 +627,43 @@ void deletingLongChain() {
 
 		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
 	void deletingLongChainNoId() {
 
 		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
 	void deletingLongChainNoIdWithBackreferenceNotReferencingTheRoot() {
 
 		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 @@ void keyColumnShouldIgnoreRenamedParent() {
 				"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 @@ static class IdIdNoIdChain {
 		@Id Long id;
 		IdNoIdChain idNoIdChain;
 	}
+
+	record Parent(@Id Long id, String name, @MappedCollection(keyColumn = "NICK_NAME") Map<String, Child> children) {
+	}
+
+	record Child(@Column("NICK_NAME") String nickName, String name) {
+	}
 }