Introduce support for ordering by aliased columns.

If a projection of an HQL query is aliased, be that a function call or a simply alias, apply sorting should NOT result in that order parameter having the primary FROM clause's alias prefixed to.

Resolves #2863.
Related: #2626, #2322.

Author: gregturn

spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java

@@ -19,7 +19,9 @@
 
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.antlr.v4.runtime.ParserRuleContext;
 import org.springframework.data.domain.Sort;
@@ -41,13 +43,15 @@ class HqlQueryTransformer extends HqlQueryRenderer {
 
 	private final @Nullable String countProjection;
 
-	private @Nullable String alias = null;
+	private @Nullable String primaryFromClauseAlias = null;
 
 	private List<JpaQueryParsingToken> projection = Collections.emptyList();
 	private boolean projectionProcessed;
 
 	private boolean hasConstructorExpression = false;
 
+	private Set<String> projectionAliases;
+
 	HqlQueryTransformer() {
 		this(Sort.unsorted(), false, null);
 	}
@@ -67,11 +71,12 @@ private HqlQueryTransformer(Sort sort, boolean countQuery, @Nullable String coun
 		this.sort = sort;
 		this.countQuery = countQuery;
 		this.countProjection = countProjection;
+		this.projectionAliases = new HashSet<>();
 	}
 
 	@Nullable
 	public String getAlias() {
-		return this.alias;
+		return this.primaryFromClauseAlias;
 	}
 
 	public List<JpaQueryParsingToken> getProjection() {
@@ -137,13 +142,14 @@ public List<JpaQueryParsingToken> visitOrderedQuery(HqlParser.OrderedQueryContex
 					if (order.isIgnoreCase()) {
 						tokens.add(TOKEN_LOWER_FUNC);
 					}
+
 					tokens.add(new JpaQueryParsingToken(() -> {
 
-						if (order.getProperty().contains("(")) {
+						if (shouldAlias(order)) {
+							return primaryFromClauseAlias + "." + order.getProperty();
+						} else {
 							return order.getProperty();
 						}
-
-						return this.alias + "." + order.getProperty();
 					}, true));
 					if (order.isIgnoreCase()) {
 						NOSPACE(tokens);
@@ -164,6 +170,33 @@ public List<JpaQueryParsingToken> visitOrderedQuery(HqlParser.OrderedQueryContex
 		return tokens;
 	}
 
+	/**
+	 * Determine when an {@link org.springframework.data.domain.Sort.Order} parameter should alias (or not).
+	 *
+	 * @param order
+	 * @return boolean whether or not to apply the primary FROM clause's alias
+	 */
+	private boolean shouldAlias(Sort.Order order) {
+
+		if (orderParameterIsAFunction(order)) {
+			return false;
+		}
+
+		if (orderParameterReferencesAProjectionAlias(order)) {
+			return false;
+		}
+
+		return true;
+	}
+
+	private boolean orderParameterIsAFunction(Sort.Order order) {
+		return order.getProperty().contains("(");
+	}
+
+	private boolean orderParameterReferencesAProjectionAlias(Sort.Order order) {
+		return projectionAliases.contains(order.getProperty());
+	}
+
 	@Override
 	public List<JpaQueryParsingToken> visitFromQuery(HqlParser.FromQueryContext ctx) {
 
@@ -176,7 +209,7 @@ public List<JpaQueryParsingToken> visitFromQuery(HqlParser.FromQueryContext ctx)
 			if (countProjection != null) {
 				tokens.add(new JpaQueryParsingToken(countProjection));
 			} else {
-				tokens.add(new JpaQueryParsingToken(() -> this.alias, false));
+				tokens.add(new JpaQueryParsingToken(() -> this.primaryFromClauseAlias, false));
 			}
 
 			tokens.add(TOKEN_CLOSE_PAREN);
@@ -240,8 +273,8 @@ public List<JpaQueryParsingToken> visitFromRoot(HqlParser.FromRootContext ctx) {
 			if (ctx.variable() != null) {
 				tokens.addAll(visit(ctx.variable()));
 
-				if (this.alias == null && !isSubquery(ctx)) {
-					this.alias = tokens.get(tokens.size() - 1).getToken();
+				if (primaryFromClauseAlias == null && !isSubquery(ctx)) {
+					primaryFromClauseAlias = tokens.get(tokens.size() - 1).getToken();
 				}
 			}
 		} else if (ctx.subquery() != null) {
@@ -256,8 +289,8 @@ public List<JpaQueryParsingToken> visitFromRoot(HqlParser.FromRootContext ctx) {
 			if (ctx.variable() != null) {
 				tokens.addAll(visit(ctx.variable()));
 
-				if (this.alias == null && !isSubquery(ctx)) {
-					this.alias = tokens.get(tokens.size() - 1).getToken();
+				if (primaryFromClauseAlias == null && !isSubquery(ctx)) {
+					primaryFromClauseAlias = tokens.get(tokens.size() - 1).getToken();
 				}
 			}
 		}
@@ -268,16 +301,22 @@ public List<JpaQueryParsingToken> visitFromRoot(HqlParser.FromRootContext ctx) {
 	@Override
 	public List<JpaQueryParsingToken> visitAlias(HqlParser.AliasContext ctx) {
 
-		List<JpaQueryParsingToken> tokens = newArrayList();
+		List<JpaQueryParsingToken> tokens = super.visitAlias(ctx);
 
-		if (ctx.AS() != null) {
-			tokens.add(new JpaQueryParsingToken(ctx.AS()));
+		if (primaryFromClauseAlias == null && !isSubquery(ctx)) {
+			primaryFromClauseAlias = tokens.get(tokens.size() - 1).getToken();
 		}
 
-		tokens.addAll(visit(ctx.identifier()));
+		return tokens;
+	}
+
+	@Override
+	public List<JpaQueryParsingToken> visitVariable(HqlParser.VariableContext ctx) {
+
+		List<JpaQueryParsingToken> tokens = super.visitVariable(ctx);
 
-		if (this.alias == null && !isSubquery(ctx)) {
-			this.alias = tokens.get(tokens.size() - 1).getToken();
+		if (ctx.identifier() != null) {
+			projectionAliases.add(tokens.get(tokens.size() - 1).getToken());
 		}
 
 		return tokens;
@@ -312,13 +351,13 @@ public List<JpaQueryParsingToken> visitSelectClause(HqlParser.SelectClauseContex
 
 					if (selectionListTokens.stream().anyMatch(hqlToken -> hqlToken.getToken().contains("new"))) {
 						// constructor
-						tokens.add(new JpaQueryParsingToken(() -> this.alias));
+						tokens.add(new JpaQueryParsingToken(() -> this.primaryFromClauseAlias));
 					} else {
 						// keep all the select items to distinct against
 						tokens.addAll(selectionListTokens);
 					}
 				} else {
-					tokens.add(new JpaQueryParsingToken(() -> this.alias));
+					tokens.add(new JpaQueryParsingToken(() -> this.primaryFromClauseAlias));
 				}
 			}
 

spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java

@@ -379,94 +379,85 @@ void doesNotPrefixUnsafeJpaSortFunctionCalls() {
 		assertThat(createQueryFor("select p from Person p", sort)).endsWith("order by sum(foo) asc");
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void doesNotPrefixMultipleAliasedFunctionCalls() {
 
 		String query = "SELECT AVG(m.price) AS avgPrice, SUM(m.stocks) AS sumStocks FROM Magazine m";
 		Sort sort = Sort.by("avgPrice", "sumStocks");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by avgPrice asc, sumStocks asc");
+		assertThat(createQueryFor(query, sort)).endsWith("order by avgPrice asc, sumStocks asc");
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void doesNotPrefixSingleAliasedFunctionCalls() {
 
 		String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m";
 		Sort sort = Sort.by("avgPrice");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by avgPrice asc");
+		assertThat(createQueryFor(query, sort)).endsWith("order by avgPrice asc");
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void prefixesSingleNonAliasedFunctionCallRelatedSortProperty() {
 
 		String query = "SELECT AVG(m.price) AS avgPrice FROM Magazine m";
 		Sort sort = Sort.by("someOtherProperty");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by m.someOtherProperty asc");
+		assertThat(createQueryFor(query, sort)).endsWith("order by m.someOtherProperty asc");
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void prefixesNonAliasedFunctionCallRelatedSortPropertyWhenSelectClauseContainsAliasedFunctionForDifferentProperty() {
 
 		String query = "SELECT m.name, AVG(m.price) AS avgPrice FROM Magazine m";
 		Sort sort = Sort.by("name", "avgPrice");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by m.name asc, avgPrice asc");
+		assertThat(createQueryFor(query, sort)).endsWith("order by m.name asc, avgPrice asc");
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void doesNotPrefixAliasedFunctionCallNameWithMultipleNumericParameters() {
 
 		String query = "SELECT SUBSTRING(m.name, 2, 5) AS trimmedName FROM Magazine m";
 		Sort sort = Sort.by("trimmedName");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by trimmedName asc");
+		assertThat(createQueryFor(query, sort)).endsWith("order by trimmedName asc");
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void doesNotPrefixAliasedFunctionCallNameWithMultipleStringParameters() {
 
 		String query = "SELECT CONCAT(m.name, 'foo') AS extendedName FROM Magazine m";
 		Sort sort = Sort.by("extendedName");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by extendedName asc");
+		assertThat(createQueryFor(query, sort)).endsWith("order by extendedName asc");
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void doesNotPrefixAliasedFunctionCallNameWithUnderscores() {
 
 		String query = "SELECT AVG(m.price) AS avg_price FROM Magazine m";
 		Sort sort = Sort.by("avg_price");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by avg_price asc");
+		assertThat(createQueryFor(query, sort)).endsWith("order by avg_price asc");
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void doesNotPrefixAliasedFunctionCallNameWithDots() {
 
 		String query = "SELECT AVG(m.price) AS m.avg FROM Magazine m";
 		Sort sort = Sort.by("m.avg");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by m.avg asc");
+		assertThatIllegalArgumentException().isThrownBy(() -> createQueryFor(query, sort));
 	}
 
-	@Test // DATAJPA-965, DATAJPA-970
+	@Test // DATAJPA-965, DATAJPA-970, GH-2863
 	void doesNotPrefixAliasedFunctionCallNameWhenQueryStringContainsMultipleWhiteSpaces() {
 
 		String query = "SELECT  AVG(  m.price  )   AS   avgPrice   FROM Magazine   m";
 		Sort sort = Sort.by("avgPrice");
 
-		// TODO: Add support for aliased functions
-		// assertThat(query(query, (Sort) "m")).endsWith("order by avgPrice asc");
+		assertThat(createQueryFor(query, sort)).endsWith("order by avgPrice asc");
 	}
 
 	@Test // DATAJPA-1506
@@ -498,18 +489,18 @@ void createCountQuerySupportsLineBreaksInSelectClause() {
 						" where user.age = 18\n ");
 	}
 
-	@Test // DATAJPA-1061
+	@Test // DATAJPA-1061, GH-2863
 	void appliesSortCorrectlyForFieldAliases() {
 
 		String query = "SELECT  m.price, lower(m.title) AS title, a.name as authorName   FROM Magazine   m INNER JOIN m.author a";
 		Sort sort = Sort.by("authorName");
 
 		String fullQuery = createQueryFor(query, sort);
 
-		assertThat(fullQuery).endsWith("order by m.authorName asc");
+		assertThat(fullQuery).endsWith("order by authorName asc");
 	}
 
-	@Test // GH-2280
+	@Test // GH-2280, GH-2863
 	void appliesOrderingCorrectlyForFieldAliasWithIgnoreCase() {
 
 		String query = "SELECT customer.id as id, customer.name as name FROM CustomerEntity customer";
@@ -518,18 +509,18 @@ void appliesOrderingCorrectlyForFieldAliasWithIgnoreCase() {
 		String fullQuery = createQueryFor(query, sort);
 
 		assertThat(fullQuery).isEqualTo(
-				"SELECT customer.id as id, customer.name as name FROM CustomerEntity customer order by lower(customer.name) asc");
+				"SELECT customer.id as id, customer.name as name FROM CustomerEntity customer order by lower(name) asc");
 	}
 
-	@Test // DATAJPA-1061
+	@Test // DATAJPA-1061, GH-2863
 	void appliesSortCorrectlyForFunctionAliases() {
 
 		String query = "SELECT  m.price, lower(m.title) AS title, a.name as authorName   FROM Magazine   m INNER JOIN m.author a";
 		Sort sort = Sort.by("title");
 
 		String fullQuery = createQueryFor(query, sort);
 
-		assertThat(fullQuery).endsWith("order by m.title asc");
+		assertThat(fullQuery).endsWith("order by title asc");
 	}
 
 	@Test // DATAJPA-1061
@@ -793,6 +784,52 @@ void sortProperlyAppendsToExistingOrderByWithFunction() {
 						"select e from SampleEntity e where function('nativeFunc', ?1) > 'testVal' order by function('nativeFunc', ?1), e.age desc");
 	}
 
+	@Test // GH-2626, GH-2863
+	void orderByAliasedColumn() {
+
+		assertThat(createQueryFor("""
+				select
+					max(resource.name)             as resourceName,
+					max(resource.id) as id,
+					max(resource.description) as description,
+					max(resource.uuid) as uuid,
+					max(resource.type)        as type,
+					max(resource.createdOn)  as createdOn,
+					max(users.firstName)     as authorFirstName,
+					max(users.lastName)      as authorLastName,
+					max(file.version)         as version,
+					max(file.comment)         as comment,
+					file.deployed             as deployed,
+					max(log.date)             as modifiedOn
+				from Resource resource
+				where (
+					cast(:startDate as date) is null
+					or resource.latestLogRecord.date between cast(:startDate as date) and cast(:endDate as date)
+				)
+				group by resource.id, file.deployed, log.author.firstName, file.comment
+				""", Sort.by(Sort.Direction.DESC, "uuid"))).endsWith("order by uuid desc");
+	}
+
+	@Test // GH-2863, GH-2322
+	void sortShouldWorkWhenAliasingFunctions() {
+
+		assertThat(createQueryFor("""
+				SELECT
+					DISTINCT(event.id) as id,
+					event.name as name,
+					MIN(bundle.base_price_amount) as cheapestBundlePrice,
+					MIN(DATE(bundle.start)) as earliestBundleStart
+				FROM event event
+				LEFT JOIN bundle bundle ON event.id = bundle.event_id
+				GROUP BY event.id
+				""", Sort.by(Sort.Direction.ASC, "cheapestBundlePrice") //
+				.and(Sort.by(
+						Sort.Direction.ASC, "earliestBundleStart")) //
+				.and(Sort.by(Sort.Direction.ASC,
+						"name"))))
+				.endsWith(" order by cheapestBundlePrice asc, earliestBundleStart asc, name asc");
+	}
+
 	private void assertCountQuery(String originalQuery, String countQuery) {
 		assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery);
 	}