Skip to content

Commit

Permalink
Properly handle Sort's that start with a join alias.
Browse files Browse the repository at this point in the history
JOIN clauses can have aliases as well, despite not using an AS reserved word. The HQL query parser needs to handle this.

See #2960, #1066, #664
Original Pull Request: 2967
  • Loading branch information
gregturn committed Jun 12, 2023
1 parent a7fe004 commit 864edda
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,30 @@ public List<JpaQueryParsingToken> visitJoin(HqlParser.JoinContext ctx) {
return tokens;
}

@Override
public List<JpaQueryParsingToken> visitJoinPath(HqlParser.JoinPathContext ctx) {

List<JpaQueryParsingToken> tokens = super.visitJoinPath(ctx);

if (ctx.variable() != null) {
transformerSupport.registerAlias(tokens.get(tokens.size() - 1).getToken());
}

return tokens;
}

@Override
public List<JpaQueryParsingToken> visitJoinSubquery(HqlParser.JoinSubqueryContext ctx) {

List<JpaQueryParsingToken> tokens = super.visitJoinSubquery(ctx);

if (ctx.variable() != null) {
transformerSupport.registerAlias(tokens.get(tokens.size() - 1).getToken());
}

return tokens;
}

@Override
public List<JpaQueryParsingToken> visitAlias(HqlParser.AliasContext ctx) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,16 @@ private boolean shouldPrefixWithAlias(Sort.Order order, String primaryFromAlias)
return false;
}

// If the Sort references an alias
// If the Sort references an alias directly
if (projectionAliases.contains(order.getProperty())) {
return false;
}

// If the Sort property starts with an alias
if (projectionAliases.stream().anyMatch(alias -> order.getProperty().startsWith(alias))) {
return false;
}

return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,16 @@ public List<JpaQueryParsingToken> visitRange_variable_declaration(JpqlParser.Ran
return tokens;
}

@Override
public List<JpaQueryParsingToken> visitJoin(JpqlParser.JoinContext ctx) {

List<JpaQueryParsingToken> tokens = super.visitJoin(ctx);

transformerSupport.registerAlias(tokens.get(tokens.size() - 1).getToken());

return tokens;
}

@Override
public List<JpaQueryParsingToken> visitConstructor_expression(JpqlParser.Constructor_expressionContext ctx) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,12 @@ AND LOWER(COALESCE(vehicle.make, '')) LIKE :query)
""")).isEqualTo("o");
}

@Test // DATAJPA-252
@Test // DATAJPA-252, GH-664, GH-1066, GH-2960
void doesNotPrefixOrderReferenceIfOuterJoinAliasDetected() {

String query = "select p from Person p left join p.address address";
Sort sort = Sort.by("address.city");
assertThat(createQueryFor(query, sort)).endsWith("order by p.address.city asc");
assertThat(createQueryFor(query, sort)).endsWith("order by address.city asc");
}

@Test // DATAJPA-252
Expand Down Expand Up @@ -986,6 +986,26 @@ select max(id), col
assertThat(createQueryFor(query, Sort.unsorted())).isEqualToIgnoringWhitespace(query);
}

@Test // GH-664, GH-1066, GH-2960
void sortingRecognizesJoinAliases() {

String query = "select p from Customer c join c.productOrder p where p.delayed = true";

assertThat(createQueryFor(query, Sort.by(Sort.Order.desc("lastName")))).isEqualToIgnoringWhitespace("""
select p from Customer c
join c.productOrder p
where p.delayed = true
order by c.lastName desc
""");

assertThat(createQueryFor(query, Sort.by(Sort.Order.desc("p.lineItems")))).isEqualToIgnoringWhitespace("""
select p from Customer c
join c.productOrder p
where p.delayed = true
order by p.lineItems desc
""");
}

private void assertCountQuery(String originalQuery, String countQuery) {
assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,12 @@ AND LOWER(COALESCE(vehicle.make, '')) LIKE :query)
""")).isEqualTo("o");
}

@Test // DATAJPA-252
@Test // DATAJPA-252, GH-664, GH-1066, GH-2960
void doesNotPrefixOrderReferenceIfOuterJoinAliasDetected() {

String query = "select p from Person p left join p.address address";
Sort sort = Sort.by("address.city");
assertThat(createQueryFor(query, sort)).endsWith("order by p.address.city asc");
assertThat(createQueryFor(query, sort)).endsWith("order by address.city asc");
}

@Test // DATAJPA-252
Expand Down Expand Up @@ -742,6 +742,26 @@ where exists (
""", relationshipName, joinAlias, joinAlias));
}

@Test // GH-664, GH-1066, GH-2960
void sortingRecognizesJoinAliases() {

String query = "select p from Customer c join c.productOrder p where p.delayed = true";

assertThat(createQueryFor(query, Sort.by(Sort.Order.desc("lastName")))).isEqualToIgnoringWhitespace("""
select p from Customer c
join c.productOrder p
where p.delayed = true
order by c.lastName desc
""");

assertThat(createQueryFor(query, Sort.by(Sort.Order.desc("p.lineItems")))).isEqualToIgnoringWhitespace("""
select p from Customer c
join c.productOrder p
where p.delayed = true
order by p.lineItems desc
""");
}

static Stream<Arguments> queriesWithReservedWordsAsIdentifiers() {

return Stream.of( //
Expand Down

0 comments on commit 864edda

Please sign in to comment.