Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JdbcAggregateTemplate honors columns specified in query #1967

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

JdbcAggregateTemplate honors columns specified in query #1967

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5aff555
1803-projection - Prepare branch
schauder Dec 23, 2024
cbde1e7
JdbcAggregateTemplate honors columns specified in query.
schauder Dec 23, 2024
9c2a954
Polishing.
mp911de Jan 16, 2025
2326e23
Consider query columns as property names.
schauder Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.5.0-SNAPSHOT</version>
<version>3.5.0-1803-projection-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data Relational Parent</name>
Expand Down Expand Up @@ -46,8 +46,8 @@
<r2dbc-h2.version>1.0.0.RELEASE</r2dbc-h2.version>
<r2dbc-mariadb.version>1.1.4</r2dbc-mariadb.version>
<r2dbc-mssql.version>1.0.2.RELEASE</r2dbc-mssql.version>
<r2dbc-mysql.version>1.3.1</r2dbc-mysql.version>
<oracle-r2dbc.version>1.3.0</oracle-r2dbc.version>
<r2dbc-mysql.version>1.3.0</r2dbc-mysql.version>
<oracle-r2dbc.version>1.2.0</oracle-r2dbc.version>

<!-- test dependencies -->
<awaitility.version>4.2.0</awaitility.version>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-jdbc-distribution/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.5.0-SNAPSHOT</version>
<version>3.5.0-1803-projection-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
4 changes: 2 additions & 2 deletions spring-data-jdbc/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-jdbc</artifactId>
<version>3.5.0-SNAPSHOT</version>
<version>3.5.0-1803-projection-SNAPSHOT</version>

<name>Spring Data JDBC</name>
<description>Spring Data module for JDBC repositories.</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.5.0-SNAPSHOT</version>
<version>3.5.0-1803-projection-SNAPSHOT</version>
</parent>

<properties>
Expand Down
74 changes: 52 additions & 22 deletions spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/SqlGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -507,45 +508,74 @@ private String createFindAllSql() {
}

private SelectBuilder.SelectWhere selectBuilder() {
return selectBuilder(Collections.emptyList());
return selectBuilder(Collections.emptyList(), Query.empty());
}

private SelectBuilder.SelectWhere selectBuilder(Query query) {
return selectBuilder(Collections.emptyList(), query);
}

private SelectBuilder.SelectWhere selectBuilder(Collection<SqlIdentifier> keyColumns) {
return selectBuilder(keyColumns, Query.empty());
}

private SelectBuilder.SelectWhere selectBuilder(Collection<SqlIdentifier> keyColumns, Query query) {

Table table = getTable();

Set<Expression> columnExpressions = new LinkedHashSet<>();
Projection projection = getProjection(keyColumns, query, table);
SelectBuilder.SelectAndFrom selectBuilder = StatementBuilder.select(projection.columns());
SelectBuilder.SelectJoin baseSelect = selectBuilder.from(table);

List<Join> joinTables = new ArrayList<>();
for (PersistentPropertyPath<RelationalPersistentProperty> path : mappingContext
.findPersistentPropertyPaths(entity.getType(), p -> true)) {
for (Join join : projection.joins()) {
baseSelect = baseSelect.leftOuterJoin(join.joinTable).on(join.joinColumn).equals(join.parentId);
}

AggregatePath extPath = mappingContext.getAggregatePath(path);
return (SelectBuilder.SelectWhere) baseSelect;
}

// add a join if necessary
Join join = getJoin(extPath);
if (join != null) {
joinTables.add(join);
private Projection getProjection(Collection<SqlIdentifier> keyColumns, Query query, Table table) {

Set<Expression> columns = new LinkedHashSet<>();
Set<Join> joins = new LinkedHashSet<>();

if (!CollectionUtils.isEmpty(query.getColumns())) {
for (SqlIdentifier columnName : query.getColumns()) {
columns.add(Column.create(columnName, table));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Column selection is missing the mapping from property name to column name. Columns (property paths specifically) should be mapped. Ideally, we can reuse the loop below regarding its functionality with just the input columns to derive property paths from. Also, we do not assume that all columns are mapped onto our domain entities.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}
} else {
for (PersistentPropertyPath<RelationalPersistentProperty> path : mappingContext
.findPersistentPropertyPaths(entity.getType(), p -> true)) {

AggregatePath extPath = 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) {
columnExpressions.add(column);
Column column = getColumn(extPath);
if (column != null) {
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<Expression> columns, Set<Join> joins) {
}

private SelectBuilder.SelectOrdered selectBuilder(Collection<SqlIdentifier> keyColumns, Sort sort,
Expand Down Expand Up @@ -876,7 +906,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();
Expand Down
26 changes: 25 additions & 1 deletion ...ava/org/springframework/data/jdbc/core/AbstractJdbcAggregateTemplateIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -235,7 +236,25 @@ void findAllByQuery() {
Query query = Query.query(criteria);
Iterable<SimpleListParent> 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<SimpleListParent> 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
Expand Down Expand Up @@ -2240,5 +2259,10 @@ static class JdbcAggregateTemplateIntegrationTests extends AbstractJdbcAggregate
static class JdbcAggregateTemplateSingleQueryLoadingIntegrationTests
extends AbstractJdbcAggregateTemplateIntegrationTests {

@Disabled
@Override
void findAllByQueryWithColumns() {
super.findAllByQueryWithColumns();
}
}
}
19 changes: 17 additions & 2 deletions ...-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/SqlGeneratorUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,19 +345,33 @@ 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", //
"WHERE dummy_entity.id1 = :id1" //
);
}

@Test // GH-1803
void selectByQueryWithColumnLimit() {

Query query = Query.empty().columns("id", "alpha", "beta", "gamma");

String sql = sqlGenerator.selectByQuery(query, new MapSqlParameterSource());

assertThat(sql).contains( //
"SELECT dummy_entity.id1, dummy_entity.alpha, dummy_entity.beta, dummy_entity.gamma", //
"FROM dummy_entity" //
);
}

@Test // GH-1919
void selectBySortedQuery() {

Expand All @@ -375,7 +389,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
Expand Down
4 changes: 2 additions & 2 deletions spring-data-r2dbc/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-r2dbc</artifactId>
<version>3.5.0-SNAPSHOT</version>
<version>3.5.0-1803-projection-SNAPSHOT</version>

<name>Spring Data R2DBC</name>
<description>Spring Data module for R2DBC</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.5.0-SNAPSHOT</version>
<version>3.5.0-1803-projection-SNAPSHOT</version>
</parent>

<properties>
Expand Down
4 changes: 2 additions & 2 deletions spring-data-relational/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-relational</artifactId>
<version>3.5.0-SNAPSHOT</version>
<version>3.5.0-1803-projection-SNAPSHOT</version>

<name>Spring Data Relational</name>
<description>Spring Data Relational support</description>

<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>3.5.0-SNAPSHOT</version>
<version>3.5.0-1803-projection-SNAPSHOT</version>
</parent>

<properties>
Expand Down
3 changes: 2 additions & 1 deletion ...g-data-relational/src/main/java/org/springframework/data/relational/core/query/Query.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -84,7 +85,7 @@ private Query(@Nullable CriteriaDefinition criteria, List<SqlIdentifier> columns
* @return
*/
public static Query empty() {
return new Query(null);
return EMPTY;
}

/**
Expand Down