From fd662bf9e62ac2a0504d98d09b7db82436ab16d2 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Thu, 21 Oct 2021 15:39:36 +0200 Subject: [PATCH] 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 Original pull request #1074 --- .../core/convert/SqlGeneratorUnitTests.java | 18 ++++++++ .../data/relational/core/sql/Column.java | 44 ++++++++++++++++++ .../data/relational/core/sql/Table.java | 46 ++++++++++++++++++- 3 files changed, 107 insertions(+), 1 deletion(-) 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 c34eddbe..a83a91f8 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 @@ -423,6 +423,23 @@ class SqlGeneratorUnitTests { + "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 void updateWithVersion() { @@ -862,6 +879,7 @@ class SqlGeneratorUnitTests { Set elements; Map mappedElements; AggregateReference other; + Map mappedReference; } @SuppressWarnings("unused") diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Column.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Column.java index 7d63aeb6..4211fb68 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Column.java +++ b/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; @@ -357,6 +359,27 @@ public class Column extends AbstractSegment implements Expression, Named { 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. */ @@ -396,5 +419,26 @@ public class Column extends AbstractSegment implements Expression, Named { 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); + } } } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java index aa27d73f..5fa1ecb5 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/sql/Table.java @@ -15,16 +15,19 @@ */ 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}. *

* Renders to: {@code } or {@code AS }. *

* * @author Mark Paluch + * @author Jens Schauder * @since 1.1 */ public class Table extends AbstractSegment implements TableLike { @@ -127,6 +130,26 @@ public class Table extends AbstractSegment implements TableLike { 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. */ @@ -164,5 +187,26 @@ public class Table extends AbstractSegment implements TableLike { 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); + } } }