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.

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

NativeQuery with Pagination #2260

Closed
Johnczek opened this issue Jul 14, 2021 · 29 comments
Closed

NativeQuery with Pagination #2260

Johnczek opened this issue Jul 14, 2021 · 29 comments
Assignees
Labels
in: query-parser Everything related to parsing JPQL or SQL type: bug A general bug

Comments

@Johnczek
Copy link

Hello, I am facing issue similiar with #1282.

I am trying to run this native query with spring-data on version 2.5.0

@Query(value = "SELECT dense_rank() OVER (ORDER BY rank DESC)," + "t.id, " + "FROM table t WHERE t.x > 0 ", nativeQuery = true) Page<Object[]> getT(Pageable pageable);

But I am getting SQLGrammarException because the spring-data engine will translate this query into

SELECT dense_rank() OVER (ORDER BY rank DESC),t.id FROM table t WHERE t.x > 0 , t.y desc limit 20

Where the problem is pretty obvious. There is ORDER BY clause missing, probably because of presence of ORDER BY clause in OVER subquery. Is this a known bug? And is it possible to bypass this somehow? I need to use whole pageable object with native query with this dense_rank() calling.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 14, 2021
@schauder
Copy link
Contributor

I suspect this is a bug in the underlying JPA implementation. Spring Data JPA uses Query.setFirstResult and Query.setMaxResult to create the queries for pagination.

Could you confirm that you seeing this behaviour as well if you use an EntityManager directly, as described in https://www.baeldung.com/jpa-pagination

@schauder schauder added the status: waiting-for-feedback We need additional information before we can continue label Jul 14, 2021
@Johnczek
Copy link
Author

I suspect this is a bug in the underlying JPA implementation. Spring Data JPA uses Query.setFirstResult and Query.setMaxResult to create the queries for pagination.

Could you confirm that you seeing this behaviour as well if you use an EntityManager directly, as described in https://www.baeldung.com/jpa-pagination

Hello, I´ve tried many ways but entity manager did not worked for me cause native query has no option to pass sort into query.

But after some investigation I´ve found a workaround with adding ORDER 1 at the end of query and now It is working fine.
I assume that spring will look for ORDER BY substring and if its found, there is (from his point of view) no need to add another ORDER BY and therefore spring appends just , ordering limiting offsetting. But in this case the ORDER BY clause should be added because the found ORDER BY property is not the one that framework is looking for.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jul 14, 2021
@schauder schauder added in: query-parser Everything related to parsing JPQL or SQL type: bug A general bug and removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged labels Jul 19, 2021
@TAndronicus
Copy link
Contributor

I'll try to tackle this issue

TAndronicus added a commit to TAndronicus/spring-data-jpa that referenced this issue Sep 28, 2021
The test cannot be reproduced using hsqldb - the syntax causing the bug is not supported.
Closes spring-projects#2260
@TAndronicus TAndronicus mentioned this issue Sep 28, 2021
4 tasks
@TAndronicus
Copy link
Contributor

TAndronicus commented Sep 28, 2021

I have added a solution where I'm counting the number of occurrences of order by and order by used in window functions. If the former count is greater than the latter, I'm assuming the order by is already present.
Although this fixes the reported issue, I can think of an example where it won't work - when order by is used in nested query (as in @shu1997 comment). Any suggestions are welcome.
Maybe it would be better to exclude all order by matches surrounded by parentheses - this would solve all the issues provided the entire query is not surrounded with parentheses.

TAndronicus added a commit to TAndronicus/spring-data-jpa that referenced this issue Oct 8, 2021
Fix alias detection bug occurring when using subqueries.
Closes spring-projects#2260
@TAndronicus
Copy link
Contributor

I've changed the pattern to match subqueries.
At the same time there was a bug in alias detection - when using subqueries, it would take the first alias - used in subquery.

@somayaj
Copy link

somayaj commented Nov 21, 2021

Hi, can this be worked?

gregturn pushed a commit that referenced this issue Mar 28, 2022
In several ways, it's possible to have various ORDER BY clauses, including SQL OVER (windowing) clauses.

See #2260.
gregturn pushed a commit that referenced this issue Mar 28, 2022
In several ways, it's possible to have various ORDER BY clauses, including SQL OVER (windowing) clauses. Need to improve handling of ORDER BY.

See #2260.
gregturn pushed a commit that referenced this issue Mar 28, 2022
In several ways, it's possible to have various ORDER BY clauses, including SQL OVER (windowing) clauses. Need to improve handling of ORDER BY.

See #2260.
gregturn pushed a commit that referenced this issue Mar 28, 2022
In several ways, it's possible to have various ORDER BY clauses, including SQL OVER (windowing) clauses. Need to improve handling of ORDER BY.

See #2260.
gregturn pushed a commit that referenced this issue Mar 28, 2022
In several ways, it's possible to have various ORDER BY clauses, including SQL OVER (windowing) clauses. Need to improve handling of ORDER BY.

See #2260.
@gregturn gregturn added this to the 3.0 M4 (2022.0.0) milestone Mar 28, 2022
@gregturn
Copy link
Contributor

Thanks @TAndronicus!

@Persi
Copy link

Persi commented Apr 25, 2022

Hi @TAndronicus, you mentioned a bug concerning alias detection. After upgrading from Spring Boot 2.5.12 to 2.5.13 (which implicitely upgrades spring-data-jpa from 2.5.10 to 2.5.11), some of our JPA queries did not work anymore because of false aliases used in queries. The problem only seem to occur in functions using @Query annotated repository methods, which use generic Sort or Paging mechanics of Spring Data JPA.

Example showing code which works with 2.5.10 of spring-data-jpa, but fails with 2.5.11:

@Query("SELECT r FROM Rolle r where " +
	"(lower(r.name) like concat('%', lower(:name), '%')) AND " +
	"(:deleted is null or r.deleted = :deleted) AND " +
	"(:systemrolle is null or r.systemRolle = :systemrolle) AND " +
	"(COALESCE(:rechtIds, null) is null or (" +
	"select count(r2) from Rolle r2 join r2.rechtIds rIds " +
	"where rIds in (:rechtIds) " +
	"AND r.id = r2.id " +
	"group by r2.id having count(r2.id) = :minNumberOfMatches ) > 0)")
Page<Rolle> findAllByNameContainsIgnoreCaseAndDeletedAndSystemrolleAndRechtIdsIn(
	@Param("name") @NotNull String name,
	@Param("deleted") Boolean deleted,
	@Param("systemrolle") Boolean systemrolle,
	@Param("rechtIds") Set<RechtId> rechtIds,
	@NotNull Long minNumberOfMatches, Pageable pageable);

The occurring error is as follows:
... Caused by: org.hibernate.hql.internal.ast.QuerySyntaxException: Invalid path: 'r2.name' [SELECT r FROM rechte_rollen.model.Rolle r where (lower(r.name) like concat('%', lower(:name), '%')) AND (:deleted is null or r.deleted = :deleted) AND (:systemrolle is null or r.systemRolle = :systemrolle) AND (COALESCE(:rechtIds, null) is null or (select count(r2) from rechte_rollen.model.Rolle r2 join r2.rechtIds rIds where rIds in (:rechtIds) AND r.id = r2.id group by r2.id having count(r2.id) = :minNumberOfMatches ) > 0) order by r2.name desc] ...

As you can see in the error, the generated sql is finalized by "order by r2.name desc", but r2 is the alias used in the subquery, not the alias used for the whole query.

Can you please take a look? I am unsure if we do something wrong, or if this is a new bug introduced with 2.5.11.

With best regards,
Marcus

@edudar
Copy link
Contributor

edudar commented Apr 25, 2022

I can confirm, the change in alias detection broke quite a few things:

while (matcher.find()) {
  alias = matcher.group(2);
}

In my case, I have a field that ends with From, alias detection uses from as a starting point, and the query alias moves.

QueryUtils.detectAlias(
"SELECT e FROM DbEvent e 
WHERE (CAST(:modifiedTo AS date) IS NULL OR e.modificationDate >= :modifiedTo)"
); -> "e"

QueryUtils.detectAlias(
"SELECT e FROM DbEvent e 
WHERE (CAST(:modifiedFrom AS date) IS NULL OR e.modificationDate >= :modifiedFrom)"
); -> "date"

@TAndronicus
Copy link
Contributor

Thank you @Persi and @edudar for reporting, I will probably have time to take a look next week. Thank you for minimal example @edudar

@slPerryRhodan
Copy link

I can confirm, the change in alias detection broke quite a few things:

while (matcher.find()) {
  alias = matcher.group(2);
}

In my case, I have a field that ends with From, alias detection uses from as a starting point, and the query alias moves.

QueryUtils.detectAlias(
"SELECT e FROM DbEvent e 
WHERE (CAST(:modifiedTo AS date) IS NULL OR e.modificationDate >= :modifiedTo)"
); -> "e"

QueryUtils.detectAlias(
"SELECT e FROM DbEvent e 
WHERE (CAST(:modifiedFrom AS date) IS NULL OR e.modificationDate >= :modifiedFrom)"
); -> "date"

I can confirm this behavior. We have the same error.

gregturn pushed a commit that referenced this issue May 4, 2022
In commit 3e64d9a a bug got introduced that uses the next symbol after the table name for the count function. With this commit this should be now resolved. The count query will use `*` when there is no alias present nor a variable.

Related tickets #2341, #2177, #2260, #2511
gregturn pushed a commit that referenced this issue May 4, 2022
In commit 3e64d9a a bug got introduced that uses the next symbol after the table name for the count function. With this commit this should be now resolved. The count query will use `*` when there is no alias present nor a variable.

Related tickets #2341, #2177, #2260, #2511
gregturn pushed a commit that referenced this issue May 4, 2022
In order to correctly identify aliases in the order by clause, we cannot process the from clauses left-to-right using regular expressions. These must be removed inner-to-outer. Commit c93aa25 resulted in a bug where the subquery would be incorrectly identified as the alias, as by the following query:

```
from Parent p join p.children c where not c.id not in (select c2.id from Child c2)
```

Passing in a Sort.by("name") would result in "order by c2.name" instead of "order by p.name". Thus, it was using the alias of the inner query instead of the outer query. [This comment](#2260 (comment)) suggests removing the content of the inner query, with the caveat of the entire query being surrounded by parenthesis. This commit does exactly that, by removing the subquery before the alias is identified. It also handles the case when the entire query is surrounded by parenthesis. Unit tests illustrate this along with several examples of removing the subquery to correctly identify the alias for the order by clause.

See #2260 (c93aa25), #2500, #2518.
gregturn added a commit that referenced this issue May 4, 2022
gregturn pushed a commit that referenced this issue May 4, 2022
In order to correctly identify aliases in the order by clause, we cannot process the from clauses left-to-right using regular expressions. These must be removed inner-to-outer. Commit c93aa25 resulted in a bug where the subquery would be incorrectly identified as the alias, as by the following query:

```
from Parent p join p.children c where not c.id not in (select c2.id from Child c2)
```

Passing in a Sort.by("name") would result in "order by c2.name" instead of "order by p.name". Thus, it was using the alias of the inner query instead of the outer query. [This comment](#2260 (comment)) suggests removing the content of the inner query, with the caveat of the entire query being surrounded by parenthesis. This commit does exactly that, by removing the subquery before the alias is identified. It also handles the case when the entire query is surrounded by parenthesis. Unit tests illustrate this along with several examples of removing the subquery to correctly identify the alias for the order by clause.

See #2260 (c93aa25), #2500, #2518.
gregturn added a commit that referenced this issue May 4, 2022
gregturn pushed a commit that referenced this issue May 4, 2022
In order to correctly identify aliases in the order by clause, we cannot process the from clauses left-to-right using regular expressions. These must be removed inner-to-outer. Commit c93aa25 resulted in a bug where the subquery would be incorrectly identified as the alias, as by the following query:

```
from Parent p join p.children c where not c.id not in (select c2.id from Child c2)
```

Passing in a Sort.by("name") would result in "order by c2.name" instead of "order by p.name". Thus, it was using the alias of the inner query instead of the outer query. [This comment](#2260 (comment)) suggests removing the content of the inner query, with the caveat of the entire query being surrounded by parenthesis. This commit does exactly that, by removing the subquery before the alias is identified. It also handles the case when the entire query is surrounded by parenthesis. Unit tests illustrate this along with several examples of removing the subquery to correctly identify the alias for the order by clause.

See #2260 (c93aa25), #2500, #2518.
gregturn added a commit that referenced this issue May 4, 2022
gregturn pushed a commit that referenced this issue May 4, 2022
In order to correctly identify aliases in the order by clause, we cannot process the from clauses left-to-right using regular expressions. These must be removed inner-to-outer. Commit c93aa25 resulted in a bug where the subquery would be incorrectly identified as the alias, as by the following query:

```
from Parent p join p.children c where not c.id not in (select c2.id from Child c2)
```

Passing in a Sort.by("name") would result in "order by c2.name" instead of "order by p.name". Thus, it was using the alias of the inner query instead of the outer query. [This comment](#2260 (comment)) suggests removing the content of the inner query, with the caveat of the entire query being surrounded by parenthesis. This commit does exactly that, by removing the subquery before the alias is identified. It also handles the case when the entire query is surrounded by parenthesis. Unit tests illustrate this along with several examples of removing the subquery to correctly identify the alias for the order by clause.

See #2260 (c93aa25), #2500, #2518.
gregturn added a commit that referenced this issue May 4, 2022
@edudar
Copy link
Contributor

edudar commented May 5, 2022

@gregturn does not look like #2516 fixes this case: #2260 (comment) as there are no sub-queries yet alias detection is affected by the change in this ticket.

darinmanica added a commit to darinmanica/spring-data-jpa that referenced this issue May 5, 2022
fixes spring-projects#2260

Added a zero-width word boundary to the regex that identifies the from clause.  This is used in alias detection for order by clauses.
@darinmanica
Copy link
Contributor

@edudar this is due to the lack of a zero-width word boundary in a regex that predates #2216; however, it is revealed by the PR. I've added the word boundary and unit test in a new PR, #2521.

gregturn pushed a commit that referenced this issue May 5, 2022
Add a zero-width word boundary to the regex that identifies the from clause. This is used in alias detection.

See #2508, #2260.
gregturn pushed a commit that referenced this issue May 5, 2022
Add a zero-width word boundary to the regex that identifies the from clause. This is used in alias detection.

See #2508, #2260.
gregturn pushed a commit that referenced this issue May 5, 2022
Add a zero-width word boundary to the regex that identifies the from clause. This is used in alias detection.

See #2508, #2260.
gregturn pushed a commit that referenced this issue May 5, 2022
Add a zero-width word boundary to the regex that identifies the from clause. This is used in alias detection.

See #2508, #2260.
@gregturn gregturn self-assigned this May 5, 2022
@gregturn
Copy link
Contributor

gregturn commented May 5, 2022

Fixed on main.

Backported to 2.7.x, 2.6.x, and 2.5.x.

Thanks @darinmanica!

@gregturn
Copy link
Contributor

gregturn commented May 5, 2022

Thanks to everyone else that helped tackle this issue with feedback and links.

@ronghuiye-agi
Copy link

hi @gregturn, does this issue get fixed? I am using 2.7.1 but still seeing the sorting with wrong table

@PAX523
Copy link

PAX523 commented Jul 14, 2022

I watched the same issue with 2.7.1, even if it should already be fixed

@schauder
Copy link
Contributor

@PAX523 please create a new issue and include a reproducer.

@darinmanica
Copy link
Contributor

@PAX523 @ronghuiye-agi There is an issue in #2563 where the alias is incorrectly detected in multi-line queries. I just submitted #2603 to address this. Does your query contain EOL characters, such as a java text block?

@PAX523
Copy link

PAX523 commented Jul 23, 2022

Yes, that's the case! I cannot inspect the used EOL characters right now, but basically the query consists of Java's multi-line string feature:

 """
  SELECT * FROM ...
   WHERE ...
  """

Thank you for working on it. Cheers PAX

trayanus1026 pushed a commit to trayanus1026/spring-data-jpa-java that referenced this issue Aug 5, 2023
In order to correctly identify aliases in the order by clause, we cannot process the from clauses left-to-right using regular expressions. These must be removed inner-to-outer. Commit c93aa25 resulted in a bug where the subquery would be incorrectly identified as the alias, as by the following query:

```
from Parent p join p.children c where not c.id not in (select c2.id from Child c2)
```

Passing in a Sort.by("name") would result in "order by c2.name" instead of "order by p.name". Thus, it was using the alias of the inner query instead of the outer query. [This comment](spring-projects/spring-data-jpa#2260 (comment)) suggests removing the content of the inner query, with the caveat of the entire query being surrounded by parenthesis. This commit does exactly that, by removing the subquery before the alias is identified. It also handles the case when the entire query is surrounded by parenthesis. Unit tests illustrate this along with several examples of removing the subquery to correctly identify the alias for the order by clause.

See #2260 (c93aa25), #2500, #2518.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: query-parser Everything related to parsing JPQL or SQL type: bug A general bug
Projects
None yet
Development

No branches or pull requests