From 5aff55545c2ad4c0fd1b6900a7c30d1fc5ad8574 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 23 Dec 2024 12:47:07 +0100 Subject: [PATCH 1/4] 1803-projection - Prepare branch --- pom.xml | 6 +++--- spring-data-jdbc-distribution/pom.xml | 2 +- spring-data-jdbc/pom.xml | 4 ++-- spring-data-r2dbc/pom.xml | 4 ++-- spring-data-relational/pom.xml | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pom.xml b/pom.xml index 028dfdbd68..9e0b3d54e9 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-relational-parent - 3.5.0-SNAPSHOT + 3.5.0-1803-projection-SNAPSHOT pom Spring Data Relational Parent @@ -46,8 +46,8 @@ 1.0.0.RELEASE 1.1.4 1.0.2.RELEASE - 1.3.1 - 1.3.0 + 1.3.0 + 1.2.0 4.2.0 diff --git a/spring-data-jdbc-distribution/pom.xml b/spring-data-jdbc-distribution/pom.xml index 9c02f50608..91fe3bb4cc 100644 --- a/spring-data-jdbc-distribution/pom.xml +++ b/spring-data-jdbc-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.5.0-SNAPSHOT + 3.5.0-1803-projection-SNAPSHOT ../pom.xml diff --git a/spring-data-jdbc/pom.xml b/spring-data-jdbc/pom.xml index aba16d9e30..919d71f93f 100644 --- a/spring-data-jdbc/pom.xml +++ b/spring-data-jdbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-jdbc - 3.5.0-SNAPSHOT + 3.5.0-1803-projection-SNAPSHOT Spring Data JDBC Spring Data module for JDBC repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.5.0-SNAPSHOT + 3.5.0-1803-projection-SNAPSHOT diff --git a/spring-data-r2dbc/pom.xml b/spring-data-r2dbc/pom.xml index fd450fd21b..2324c0c820 100644 --- a/spring-data-r2dbc/pom.xml +++ b/spring-data-r2dbc/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-r2dbc - 3.5.0-SNAPSHOT + 3.5.0-1803-projection-SNAPSHOT Spring Data R2DBC Spring Data module for R2DBC @@ -15,7 +15,7 @@ org.springframework.data spring-data-relational-parent - 3.5.0-SNAPSHOT + 3.5.0-1803-projection-SNAPSHOT diff --git a/spring-data-relational/pom.xml b/spring-data-relational/pom.xml index 9760a0f1e7..b2547c2659 100644 --- a/spring-data-relational/pom.xml +++ b/spring-data-relational/pom.xml @@ -6,7 +6,7 @@ 4.0.0 spring-data-relational - 3.5.0-SNAPSHOT + 3.5.0-1803-projection-SNAPSHOT Spring Data Relational Spring Data Relational support @@ -14,7 +14,7 @@ org.springframework.data spring-data-relational-parent - 3.5.0-SNAPSHOT + 3.5.0-1803-projection-SNAPSHOT From cbde1e71101416311d9515bf014f6e258bff9277 Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 23 Dec 2024 14:09:53 +0100 Subject: [PATCH 2/4] JdbcAggregateTemplate honors columns specified in query. If no columns are given, all columns are selected by default. If columns are specified, only these are selected. Joins normally triggered by columns from 1:1 relationships are not implemented, and the corresponding columns don't get loaded and can't be specified in a query. Limiting columns is not supported for single query loading. Closes #1803 --- .../data/jdbc/core/convert/SqlGenerator.java | 44 +++++++++++++------ ...JdbcAggregateTemplateIntegrationTests.java | 26 ++++++++++- .../core/convert/SqlGeneratorUnitTests.java | 16 ++++++- 3 files changed, 71 insertions(+), 15 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 cf173ff570..a181b9c05c 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 @@ -507,33 +507,50 @@ private String createFindAllSql() { } private SelectBuilder.SelectWhere selectBuilder() { - return selectBuilder(Collections.emptyList()); + return selectBuilder(Collections.emptyList(), null); + } + + + private SelectBuilder.SelectWhere selectBuilder(Query query) { + return selectBuilder(Collections.emptyList(), query); } private SelectBuilder.SelectWhere selectBuilder(Collection keyColumns) { + return selectBuilder(keyColumns, null); + } + + private SelectBuilder.SelectWhere selectBuilder(Collection keyColumns, @Nullable Query query) { Table table = getTable(); Set columnExpressions = new LinkedHashSet<>(); List joinTables = new ArrayList<>(); - for (PersistentPropertyPath path : mappingContext - .findPersistentPropertyPaths(entity.getType(), p -> true)) { - AggregatePath extPath = mappingContext.getAggregatePath(path); - - // add a join if necessary - Join join = getJoin(extPath); - if (join != null) { - joinTables.add(join); + if (query != null && !query.getColumns().isEmpty()) { + for (SqlIdentifier columnName : query.getColumns()) { + columnExpressions.add(Column.create(columnName, table)); } + } else { + for (PersistentPropertyPath path : mappingContext + .findPersistentPropertyPaths(entity.getType(), p -> true)) { + + AggregatePath extPath = mappingContext.getAggregatePath(path); + + // add a join if necessary + Join join = getJoin(extPath); + if (join != null) { + joinTables.add(join); + } - Column column = getColumn(extPath); - if (column != null) { - columnExpressions.add(column); + Column column = getColumn(extPath); + if (column != null) { + columnExpressions.add(column); + } } } + for (SqlIdentifier keyColumn : keyColumns) { columnExpressions.add(table.column(keyColumn).as(keyColumn)); } @@ -876,7 +893,7 @@ public String selectByQuery(Query query, MapSqlParameterSource parameterSource) Assert.notNull(parameterSource, "parameterSource must not be null"); - SelectBuilder.SelectWhere selectBuilder = selectBuilder(); + SelectBuilder.SelectWhere selectBuilder = selectBuilder(query); Select select = applyQueryOnSelect(query, parameterSource, selectBuilder) // .build(); @@ -884,6 +901,7 @@ public String selectByQuery(Query query, MapSqlParameterSource parameterSource) return render(select); } + /** * Constructs a single sql query that performs select based on the provided query and pagination information. * Additional the bindings for the where clause are stored after execution into the parameterSource diff --git a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java index c08e58de66..6762b5f3e4 100644 --- a/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java +++ b/spring-data-jdbc/src/test/java/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java @@ -29,6 +29,7 @@ import java.util.stream.IntStream; import org.assertj.core.api.SoftAssertions; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationEventPublisher; @@ -235,7 +236,25 @@ void findAllByQuery() { Query query = Query.query(criteria); Iterable reloadedById = template.findAll(query, SimpleListParent.class); - assertThat(reloadedById).extracting(e -> e.id, e -> e.content.size()).containsExactly(tuple(two.id, 2)); + assertThat(reloadedById) // + .extracting(e -> e.id, e-> e.name, e -> e.content.size()) // + .containsExactly(tuple(two.id, two.name, 2)); + } + + @Test // GH-1803 + void findAllByQueryWithColumns() { + + template.save(SimpleListParent.of("one", "one_1")); + SimpleListParent two = template.save(SimpleListParent.of("two", "two_1", "two_2")); + template.save(SimpleListParent.of("three", "three_1", "three_2", "three_3")); + + CriteriaDefinition criteria = CriteriaDefinition.from(Criteria.where("id").is(two.id)); + Query query = Query.query(criteria).columns("id"); + Iterable reloadedById = template.findAll(query, SimpleListParent.class); + + assertThat(reloadedById) // + .extracting(e -> e.id, e-> e.name, e -> e.content.size()) // + .containsExactly(tuple(two.id, null, 2)); } @Test // GH-1601 @@ -2240,5 +2259,10 @@ static class JdbcAggregateTemplateIntegrationTests extends AbstractJdbcAggregate static class JdbcAggregateTemplateSingleQueryLoadingIntegrationTests extends AbstractJdbcAggregateTemplateIntegrationTests { + @Disabled + @Override + void findAllByQueryWithColumns() { + super.findAllByQueryWithColumns(); + } } } 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 d095b27ccb..764549f820 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 @@ -358,6 +358,19 @@ void selectByQuery() { ); } + @Test // GH-1803 + void selectByQueryWithColumnLimit() { + + Query query = Query.empty().columns("alpha", "beta", "gamma"); + + String sql = sqlGenerator.selectByQuery(query, new MapSqlParameterSource()); + + assertThat(sql).contains( // + "SELECT dummy_entity.alpha, dummy_entity.beta, dummy_entity.gamma", // + "FROM dummy_entity" // + ); + } + @Test // GH-1919 void selectBySortedQuery() { @@ -375,7 +388,8 @@ void selectBySortedQuery() { "ORDER BY dummy_entity.id1 ASC" // ); assertThat(sql).containsOnlyOnce("LEFT OUTER JOIN referenced_entity ref ON ref.dummy_entity = dummy_entity.id1"); - assertThat(sql).containsOnlyOnce("LEFT OUTER JOIN second_level_referenced_entity ref_further ON ref_further.referenced_entity = ref.x_l1id"); + assertThat(sql).containsOnlyOnce( + "LEFT OUTER JOIN second_level_referenced_entity ref_further ON ref_further.referenced_entity = ref.x_l1id"); } @Test // DATAJDBC-131, DATAJDBC-111 From 9c2a954842c77fd46380d48c1df27b16edaf5cd6 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 16 Jan 2025 14:15:24 +0100 Subject: [PATCH 3/4] Polishing. Extract method. Extend tests. Introduce empty Query object to avoid nullability. --- .../data/jdbc/core/convert/SqlGenerator.java | 52 ++++++++++++------- .../core/convert/SqlGeneratorUnitTests.java | 7 +-- .../data/relational/core/query/Query.java | 3 +- 3 files changed, 38 insertions(+), 24 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 a181b9c05c..b834b927e6 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 @@ -39,6 +39,7 @@ import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; /** * Generates SQL statements to be used by {@link SimpleJdbcRepository} @@ -507,29 +508,40 @@ private String createFindAllSql() { } private SelectBuilder.SelectWhere selectBuilder() { - return selectBuilder(Collections.emptyList(), null); + return selectBuilder(Collections.emptyList(), Query.empty()); } - private SelectBuilder.SelectWhere selectBuilder(Query query) { return selectBuilder(Collections.emptyList(), query); } private SelectBuilder.SelectWhere selectBuilder(Collection keyColumns) { - return selectBuilder(keyColumns, null); + return selectBuilder(keyColumns, Query.empty()); } - private SelectBuilder.SelectWhere selectBuilder(Collection keyColumns, @Nullable Query query) { + private SelectBuilder.SelectWhere selectBuilder(Collection keyColumns, Query query) { Table table = getTable(); - Set columnExpressions = new LinkedHashSet<>(); + Projection projection = getProjection(keyColumns, query, table); + SelectBuilder.SelectAndFrom selectBuilder = StatementBuilder.select(projection.columns()); + SelectBuilder.SelectJoin baseSelect = selectBuilder.from(table); + + for (Join join : projection.joins()) { + baseSelect = baseSelect.leftOuterJoin(join.joinTable).on(join.joinColumn).equals(join.parentId); + } + + return (SelectBuilder.SelectWhere) baseSelect; + } - List joinTables = new ArrayList<>(); + private Projection getProjection(Collection keyColumns, Query query, Table table) { - if (query != null && !query.getColumns().isEmpty()) { + Set columns = new LinkedHashSet<>(); + Set joins = new LinkedHashSet<>(); + + if (!CollectionUtils.isEmpty(query.getColumns())) { for (SqlIdentifier columnName : query.getColumns()) { - columnExpressions.add(Column.create(columnName, table)); + columns.add(Column.create(columnName, table)); } } else { for (PersistentPropertyPath path : mappingContext @@ -540,29 +552,30 @@ private SelectBuilder.SelectWhere selectBuilder(Collection keyCol // add a join if necessary Join join = getJoin(extPath); if (join != null) { - joinTables.add(join); + joins.add(join); } Column column = getColumn(extPath); if (column != null) { - columnExpressions.add(column); + columns.add(column); } } } - for (SqlIdentifier keyColumn : keyColumns) { - columnExpressions.add(table.column(keyColumn).as(keyColumn)); + columns.add(table.column(keyColumn).as(keyColumn)); } - SelectBuilder.SelectAndFrom selectBuilder = StatementBuilder.select(columnExpressions); - SelectBuilder.SelectJoin baseSelect = selectBuilder.from(table); - - for (Join join : joinTables) { - baseSelect = baseSelect.leftOuterJoin(join.joinTable).on(join.joinColumn).equals(join.parentId); - } + return new Projection(columns, joins); + } - return (SelectBuilder.SelectWhere) baseSelect; + /** + * Projection including its source joins. + * + * @param columns + * @param joins + */ + record Projection(Set columns, Set joins) { } private SelectBuilder.SelectOrdered selectBuilder(Collection keyColumns, Sort sort, @@ -901,7 +914,6 @@ public String selectByQuery(Query query, MapSqlParameterSource parameterSource) return render(select); } - /** * Constructs a single sql query that performs select based on the provided query and pagination information. * Additional the bindings for the where clause are stored after execution into the parameterSource 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 764549f820..199add84c8 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 @@ -345,12 +345,13 @@ void findAllPagedAndSorted() { @Test // GH-1919 void selectByQuery() { - Query query = Query.query(Criteria.where("id").is(23L)); + Query query = Query.query(Criteria.where("id").is(23L)).columns(new String[0]); String sql = sqlGenerator.selectByQuery(query, new MapSqlParameterSource()); assertThat(sql).contains( // "SELECT", // + "dummy_entity.id1 AS id1, dummy_entity.x_name AS x_name", // "FROM dummy_entity", // "LEFT OUTER JOIN referenced_entity ref ON ref.dummy_entity = dummy_entity.id1", // "LEFT OUTER JOIN second_level_referenced_entity ref_further ON ref_further.referenced_entity = ref.x_l1id", // @@ -361,12 +362,12 @@ void selectByQuery() { @Test // GH-1803 void selectByQueryWithColumnLimit() { - Query query = Query.empty().columns("alpha", "beta", "gamma"); + Query query = Query.empty().columns("id", "alpha", "beta", "gamma"); String sql = sqlGenerator.selectByQuery(query, new MapSqlParameterSource()); assertThat(sql).contains( // - "SELECT dummy_entity.alpha, dummy_entity.beta, dummy_entity.gamma", // + "SELECT dummy_entity.id1, dummy_entity.alpha, dummy_entity.beta, dummy_entity.gamma", // "FROM dummy_entity" // ); } diff --git a/spring-data-relational/src/main/java/org/springframework/data/relational/core/query/Query.java b/spring-data-relational/src/main/java/org/springframework/data/relational/core/query/Query.java index 3a8e9d72c6..6d1ed69d4f 100644 --- a/spring-data-relational/src/main/java/org/springframework/data/relational/core/query/Query.java +++ b/spring-data-relational/src/main/java/org/springframework/data/relational/core/query/Query.java @@ -41,6 +41,7 @@ */ public class Query { + private static final Query EMPTY = new Query(null); private static final int NO_LIMIT = -1; private final @Nullable CriteriaDefinition criteria; @@ -84,7 +85,7 @@ private Query(@Nullable CriteriaDefinition criteria, List columns * @return */ public static Query empty() { - return new Query(null); + return EMPTY; } /** From 2326e234cdec718cf5c9d5e6e0a1d6d67b868c8c Mon Sep 17 00:00:00 2001 From: Jens Schauder Date: Mon, 20 Jan 2025 09:55:57 +0100 Subject: [PATCH 4/4] Consider query columns as property names. Query columns now get checked against property names. If such a property name is found, the property is used. Otherwise the column is considered a literal column and used as is in the SQL generation. Original pull request #1967 See #1803 --- .../data/jdbc/core/convert/SqlGenerator.java | 39 +++++++++++++------ .../core/convert/SqlGeneratorUnitTests.java | 2 +- 2 files changed, 28 insertions(+), 13 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 b834b927e6..357d921131 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 @@ -541,24 +541,25 @@ private Projection getProjection(Collection keyColumns, Query que if (!CollectionUtils.isEmpty(query.getColumns())) { for (SqlIdentifier columnName : query.getColumns()) { - columns.add(Column.create(columnName, table)); + + String columnNameString = columnName.getReference(); + RelationalPersistentProperty property = entity.getPersistentProperty(columnNameString); + if (property != null) { + + AggregatePath aggregatePath = mappingContext.getAggregatePath( + mappingContext.getPersistentPropertyPath(columnNameString, entity.getTypeInformation())); + gatherColumn(aggregatePath, joins, columns); + } else { + columns.add(Column.create(columnName, table)); + } } } else { for (PersistentPropertyPath path : mappingContext .findPersistentPropertyPaths(entity.getType(), p -> true)) { - AggregatePath extPath = mappingContext.getAggregatePath(path); + AggregatePath aggregatePath = mappingContext.getAggregatePath(path); - // add a join if necessary - Join join = getJoin(extPath); - if (join != null) { - joins.add(join); - } - - Column column = getColumn(extPath); - if (column != null) { - columns.add(column); - } + gatherColumn(aggregatePath, joins, columns); } } @@ -569,6 +570,20 @@ private Projection getProjection(Collection keyColumns, Query que return new Projection(columns, joins); } + private void gatherColumn(AggregatePath aggregatePath, Set joins, Set columns) { + + // add a join if necessary + Join join = getJoin(aggregatePath); + if (join != null) { + joins.add(join); + } + + Column column = getColumn(aggregatePath); + if (column != null) { + columns.add(column); + } + } + /** * Projection including its source joins. * 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 199add84c8..69286031cb 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 @@ -367,7 +367,7 @@ void selectByQueryWithColumnLimit() { String sql = sqlGenerator.selectByQuery(query, new MapSqlParameterSource()); assertThat(sql).contains( // - "SELECT dummy_entity.id1, dummy_entity.alpha, dummy_entity.beta, dummy_entity.gamma", // + "SELECT dummy_entity.id1 AS id1, dummy_entity.alpha, dummy_entity.beta, dummy_entity.gamma", // "FROM dummy_entity" // ); }