Polishing.

Extract method. Extend tests. Introduce empty Query object to avoid nullability.

Author: mp911de

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<SqlIdentifier> keyColumns) {
-		return selectBuilder(keyColumns, null);
+		return selectBuilder(keyColumns, Query.empty());
 	}
 
-	private SelectBuilder.SelectWhere selectBuilder(Collection<SqlIdentifier> keyColumns, @Nullable Query query) {
+	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);
+
+		for (Join join : projection.joins()) {
+			baseSelect = baseSelect.leftOuterJoin(join.joinTable).on(join.joinColumn).equals(join.parentId);
+		}
+
+		return (SelectBuilder.SelectWhere) baseSelect;
+	}
 
-		List<Join> joinTables = new ArrayList<>();
+	private Projection getProjection(Collection<SqlIdentifier> keyColumns, Query query, Table table) {
 
-		if (query != null && !query.getColumns().isEmpty()) {
+		Set<Expression> columns = new LinkedHashSet<>();
+		Set<Join> 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<RelationalPersistentProperty> path : mappingContext
@@ -540,29 +552,30 @@ private SelectBuilder.SelectWhere selectBuilder(Collection<SqlIdentifier> 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<Expression> columns, Set<Join> joins) {
 	}
 
 	private SelectBuilder.SelectOrdered selectBuilder(Collection<SqlIdentifier> 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 <code>parameterSource</code>

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" //
 		);
 	}

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<SqlIdentifier> columns
 	 * @return
 	 */
 	public static Query empty() {
-		return new Query(null);
+		return EMPTY;
 	}
 
 	/**