Skip to content

Suppress selection item aliasing for the actual count query #3553

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.

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-GH-3536-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data JPA Parent</name>
Expand Down
4 changes: 2 additions & 2 deletions spring-data-envers/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-envers</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-GH-3536-SNAPSHOT</version>

<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-GH-3536-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-jpa-distribution/pom.xml
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-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-GH-3536-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
2 changes: 1 addition & 1 deletion spring-data-jpa-performance/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-GH-3536-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
4 changes: 2 additions & 2 deletions spring-data-jpa/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-GH-3536-SNAPSHOT</version>

<name>Spring Data JPA</name>
<description>Spring Data module for JPA repositories.</description>
Expand All @@ -15,7 +15,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-jpa-parent</artifactId>
<version>3.4.0-SNAPSHOT</version>
<version>3.4.0-GH-3536-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@
*/
package org.springframework.data.jpa.repository.query;

import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_CLOSE_PAREN;
import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_COMMA;
import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_COUNT_FUNC;
import static org.springframework.data.jpa.repository.query.QueryTokens.*;

import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder;
import org.springframework.data.jpa.repository.query.QueryTransformers.CountSelectionTokenStream;
Expand Down Expand Up @@ -98,7 +96,7 @@ public QueryTokenStream visitSelect_clause(EqlParser.Select_clauseContext ctx) {
private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectionListbuilder) {

QueryRendererBuilder nested = new QueryRendererBuilder();
CountSelectionTokenStream countSelection = QueryTransformers.filterCountSelection(selectionListbuilder);
CountSelectionTokenStream countSelection = CountSelectionTokenStream.create(selectionListbuilder);

if (countSelection.requiresPrimaryAlias()) {
// constructor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,7 @@
*/
package org.springframework.data.jpa.repository.query;

import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_AS;
import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_CLOSE_PAREN;
import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_COUNT_FUNC;
import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_DOUBLE_UNDERSCORE;
import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_OPEN_PAREN;
import static org.springframework.data.jpa.repository.query.QueryTokens.TOKEN_SELECT_COUNT;
import static org.springframework.data.jpa.repository.query.QueryTokens.*;

import org.springframework.data.jpa.repository.query.HqlParser.SelectClauseContext;
import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder;
Expand Down Expand Up @@ -208,6 +203,22 @@ public QueryTokenStream visitSelectClause(HqlParser.SelectClauseContext ctx) {
return builder;
}

@Override
public QueryTokenStream visitSelection(HqlParser.SelectionContext ctx) {

if (isSubquery(ctx)) {
return super.visitSelection(ctx);
}

QueryRendererBuilder builder = QueryRenderer.builder();

builder.append(visit(ctx.selectExpression()));

// do not append variables to skip AS field aliasing

return builder;
}

@Override
public QueryRendererBuilder visitQueryOrder(HqlParser.QueryOrderContext ctx) {

Expand Down Expand Up @@ -240,14 +251,14 @@ private QueryRendererBuilder visitSubQuerySelectClause(SelectClauseContext ctx,
private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectionListbuilder) {

QueryRendererBuilder nested = new QueryRendererBuilder();
CountSelectionTokenStream countSelection = QueryTransformers.filterCountSelection(selectionListbuilder);
CountSelectionTokenStream countSelection = CountSelectionTokenStream.create(selectionListbuilder);

if (countSelection.requiresPrimaryAlias()) {
// constructor
nested.append(QueryTokens.token(primaryFromAlias));
} else {
// keep all the select items to distinct against
nested.append(countSelection);
nested.append(selectionListbuilder);
}
return nested;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public QueryRendererBuilder visitSelect_clause(JpqlParser.Select_clauseContext c
private QueryRendererBuilder getDistinctCountSelection(QueryTokenStream selectionListbuilder) {

QueryRendererBuilder nested = new QueryRendererBuilder();
CountSelectionTokenStream countSelection = QueryTransformers.filterCountSelection(selectionListbuilder);
CountSelectionTokenStream countSelection = CountSelectionTokenStream.create(selectionListbuilder);

if (countSelection.requiresPrimaryAlias()) {
// constructor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,46 +29,46 @@
*/
class QueryTransformers {

static CountSelectionTokenStream filterCountSelection(QueryTokenStream selection) {
static class CountSelectionTokenStream implements QueryTokenStream {

List<QueryToken> target = new ArrayList<>(selection.size());
boolean skipNext = false;
boolean containsNew = false;
private final List<QueryToken> tokens;
private final boolean requiresPrimaryAlias;

for (QueryToken token : selection) {
CountSelectionTokenStream(List<QueryToken> tokens, boolean requiresPrimaryAlias) {
this.tokens = tokens;
this.requiresPrimaryAlias = requiresPrimaryAlias;
}

if (skipNext) {
skipNext = false;
continue;
}
static CountSelectionTokenStream create(QueryTokenStream selection) {

if (token.equals(TOKEN_AS)) {
skipNext = true;
continue;
}
List<QueryToken> target = new ArrayList<>(selection.size());
boolean skipNext = false;
boolean containsNew = false;

if (!token.equals(TOKEN_COMMA) && token.isExpression()) {
token = QueryTokens.token(token.value());
}
for (QueryToken token : selection) {

if (!containsNew && token.value().contains("new")) {
containsNew = true;
}
if (skipNext) {
skipNext = false;
continue;
}

target.add(token);
}
if (token.equals(TOKEN_AS)) {
skipNext = true;
continue;
}

return new CountSelectionTokenStream(target, containsNew);
}
if (!token.equals(TOKEN_COMMA) && token.isExpression()) {
token = QueryTokens.token(token.value());
}

static class CountSelectionTokenStream implements QueryTokenStream {
if (!containsNew && token.value().contains("new")) {
containsNew = true;
}

private final List<QueryToken> tokens;
private final boolean requiresPrimaryAlias;
target.add(token);
}

public CountSelectionTokenStream(List<QueryToken> tokens, boolean requiresPrimaryAlias) {
this.tokens = tokens;
this.requiresPrimaryAlias = requiresPrimaryAlias;
return new CountSelectionTokenStream(target, containsNew);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ void applyCountToMoreComplexQuery() {
}

@Test
void applyCountToAlreadySorteQuery() {
void applyCountToAlreadySortedQuery() {

// given
var original = "SELECT e FROM Employee e where e.name = :name ORDER BY e.modified_date";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,23 @@ void applyCountToSimpleQuery() {
assertThat(results).isEqualTo("select count(e) FROM Employee e where e.name = :name");
}

@Test // GH-3536
void shouldCreateCountQueryForDistinctCount() {

// given
var original = """
select distinct cast(e.timestampField as date) as foo
from ExampleEntity e
order by cast(e.timestampField as date) desc
""";

// when
var results = createCountQueryFor(original);

// then
assertThat(results).isEqualTo("select count(distinct cast(e.timestampField as date)) from ExampleEntity e");
}

@Test
void applyCountToMoreComplexQuery() {

Expand Down Expand Up @@ -701,11 +718,9 @@ void applySortingAccountsForNativeWindowFunction() {
.isEqualTo("select dense_rank() over (order by lastname) from user u order by u.lastname, u.age desc");

// partition by + order by in over clause
assertThat(
createQueryFor(
"select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u",
sort))
.isEqualTo(
assertThat(createQueryFor(
"select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u",
sort)).isEqualTo(
"select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u order by u.age desc");

// partition by + order by in over clause + order by at the end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ void applyCountToMoreComplexQuery() {
}

@Test
void applyCountToAlreadySorteQuery() {
void applyCountToAlreadySortedQuery() {

// given
var original = "SELECT e FROM Employee e where e.name = :name ORDER BY e.modified_date";
Expand Down
Loading