Avoid duplicate selection of columns.

When the key column of a MappedCollection is also present in the contained entity that column got select twice.
We now check if the column already gets selected before adding it to the selection.

This only works properly if the column names derived for the entity property and the key column match exactly, which might require use of a `@Column` annotation depending on the used NamingStrategy.

Closes #1073

Author: schauder

spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java

@@ -381,11 +381,11 @@ private String createAcquireLockById(LockMode lockMode) {
 		Table table = this.getTable();
 
 		Select select = StatementBuilder //
-			.select(getIdColumn()) //
-			.from(table) //
-			.where(getIdColumn().isEqualTo(getBindMarker(ID_SQL_PARAMETER))) //
-			.lock(lockMode) //
-			.build();
+				.select(getIdColumn()) //
+				.from(table) //
+				.where(getIdColumn().isEqualTo(getBindMarker(ID_SQL_PARAMETER))) //
+				.lock(lockMode) //
+				.build();
 
 		return render(select);
 	}
@@ -395,10 +395,10 @@ private String createAcquireLockAll(LockMode lockMode) {
 		Table table = this.getTable();
 
 		Select select = StatementBuilder //
-			.select(getIdColumn()) //
-			.from(table) //
-			.lock(lockMode) //
-			.build();
+				.select(getIdColumn()) //
+				.from(table) //
+				.lock(lockMode) //
+				.build();
 
 		return render(select);
 	}
@@ -727,9 +727,9 @@ static final class Join {
 
 		Join(Table joinTable, Column joinColumn, Column parentId) {
 
-			Assert.notNull( joinTable,"JoinTable must not be null.");
-			Assert.notNull( joinColumn,"JoinColumn must not be null.");
-			Assert.notNull( parentId,"ParentId must not be null.");
+			Assert.notNull(joinTable, "JoinTable must not be null.");
+			Assert.notNull(joinColumn, "JoinColumn must not be null.");
+			Assert.notNull(parentId, "ParentId must not be null.");
 
 			this.joinTable = joinTable;
 			this.joinColumn = joinColumn;

spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java

@@ -23,7 +23,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.data.annotation.Id;
@@ -374,6 +373,23 @@ public void findAllByPropertyWithKeyOrdered() {
 				+ "WHERE dummy_entity.backref = :backref " + "ORDER BY key-column");
 	}
 
+	@Test // GH-1073
+	public void findAllByPropertyAvoidsDuplicateColumns() {
+
+		final SqlGenerator sqlGenerator = createSqlGenerator(ReferencedEntity.class);
+		final String sql = sqlGenerator.getFindAllByProperty(
+				Identifier.of(quoted("id"), "parent-id-value", DummyEntity.class), //
+				quoted("X_L1ID"), // this key column collides with the name derived by the naming strategy for the id of
+													// ReferencedEntity.
+				false);
+
+		final String id = "referenced_entity.x_l1id AS x_l1id";
+		assertThat(sql.indexOf(id)) //
+				.describedAs(sql) //
+				.isEqualTo(sql.lastIndexOf(id));
+
+	}
+
 	@Test // DATAJDBC-219
 	public void updateWithVersion() {
 
@@ -709,6 +725,7 @@ static class DummyEntity {
 		Set<Element> elements;
 		Map<Integer, Element> mappedElements;
 		AggregateReference<OtherAggregate, Long> other;
+		Map<Long, ReferencedEntity> mappedReference;
 	}
 
 	static class VersionedEntity extends DummyEntity {

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/AbstractSegment.java

@@ -15,12 +15,15 @@
  */
 package org.springframework.data.relational.core.sql;
 
+import java.util.Arrays;
+
 import org.springframework.util.Assert;
 
 /**
  * Abstract implementation to support {@link Segment} implementations.
  *
  * @author Mark Paluch
+ * @author Jens Schauder
  * @since 1.1
  */
 abstract class AbstractSegment implements Segment {
@@ -47,21 +50,21 @@ public void visit(Visitor visitor) {
 		visitor.leave(this);
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * @see java.lang.Object#hashCode()
-	 */
 	@Override
-	public int hashCode() {
-		return toString().hashCode();
+	public boolean equals(Object o) {
+
+		if (this == o) {
+			return true;
+		}
+		if (o == null || getClass() != o.getClass()) {
+			return false;
+		}
+		AbstractSegment that = (AbstractSegment) o;
+		return Arrays.equals(children, that.children);
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * @see java.lang.Object#equals(java.lang.Object)
-	 */
 	@Override
-	public boolean equals(Object obj) {
-		return obj instanceof Segment && toString().equals(obj.toString());
+	public int hashCode() {
+		return Arrays.hashCode(children);
 	}
 }

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Column.java

@@ -15,6 +15,8 @@
  */
 package org.springframework.data.relational.core.sql;
 
+import java.util.Objects;
+
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
@@ -364,6 +366,27 @@ String getPrefix() {
 		return prefix;
 	}
 
+	@Override
+	public boolean equals(Object o) {
+
+		if (this == o) {
+			return true;
+		}
+		if (o == null || getClass() != o.getClass()) {
+			return false;
+		}
+		if (!super.equals(o)) {
+			return false;
+		}
+		Column column = (Column) o;
+		return name.equals(column.name) && table.equals(column.table);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(super.hashCode(), name, table);
+	}
+
 	/**
 	 * {@link Aliased} {@link Column} implementation.
 	 */
@@ -419,5 +442,26 @@ public Column from(Table table) {
 		public String toString() {
 			return getPrefix() + getName() + " AS " + getAlias();
 		}
+
+		@Override
+		public boolean equals(Object o) {
+
+			if (this == o) {
+				return true;
+			}
+			if (o == null || getClass() != o.getClass()) {
+				return false;
+			}
+			if (!super.equals(o)) {
+				return false;
+			}
+			AliasedColumn that = (AliasedColumn) o;
+			return alias.equals(that.alias);
+		}
+
+		@Override
+		public int hashCode() {
+			return Objects.hash(super.hashCode(), alias);
+		}
 	}
 }

spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java

@@ -15,15 +15,18 @@
  */
 package org.springframework.data.relational.core.sql;
 
+import java.util.Objects;
+
 import org.springframework.util.Assert;
 
 /**
- * Represents a table reference within a SQL statement. Typically used to denote {@code FROM} or {@code JOIN} or to
+ * Represents a table reference within a SQL statement. Typically, used to denote {@code FROM} or {@code JOIN} or to
  * prefix a {@link Column}.
  * <p/>
  * Renders to: {@code <name>} or {@code <name> AS <name>}.
  *
  * @author Mark Paluch
+ * @author Jens Schauder
  * @since 1.1
  */
 public class Table extends AbstractSegment implements TableLike {
@@ -107,7 +110,6 @@ public Table as(SqlIdentifier alias) {
 		return new AliasedTable(name, alias);
 	}
 
-
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.relational.core.sql.Named#getName()
@@ -135,6 +137,26 @@ public String toString() {
 		return name.toString();
 	}
 
+	@Override
+	public boolean equals(Object o) {
+		if (this == o) {
+			return true;
+		}
+		if (o == null || getClass() != o.getClass()) {
+			return false;
+		}
+		if (!super.equals(o)) {
+			return false;
+		}
+		Table table = (Table) o;
+		return name.equals(table.name);
+	}
+
+	@Override
+	public int hashCode() {
+		return Objects.hash(super.hashCode(), name);
+	}
+
 	/**
 	 * {@link Aliased} {@link Table} implementation.
 	 */
@@ -184,5 +206,26 @@ public SqlIdentifier getReferenceName() {
 		public String toString() {
 			return getName() + " AS " + getAlias();
 		}
+
+		@Override
+		public boolean equals(Object o) {
+
+			if (this == o) {
+				return true;
+			}
+			if (o == null || getClass() != o.getClass()) {
+				return false;
+			}
+			if (!super.equals(o)) {
+				return false;
+			}
+			AliasedTable that = (AliasedTable) o;
+			return alias.equals(that.alias);
+		}
+
+		@Override
+		public int hashCode() {
+			return Objects.hash(super.hashCode(), alias);
+		}
 	}
 }