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

Create the possibility to firstresult and maxresult in @Query or a new annotation [DATAJPA-147] #573

Closed
spring-projects-issues opened this issue Jan 3, 2012 · 11 comments

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Jan 3, 2012

Donovan opened DATAJPA-147 and commented

Create the possibility to firstresult and maxresult in @Query or a new annotation

To make it possible to have a Top 10 firstresult=0 maxresult=10

@Query(value="select pt from PersonTraining pt where pt.person = :person and pt.traing = :training order by pt.dueDate")

return a list

and even

To have a unique or null result having firstresult=0 maxresult=1

@Query(value="select pt from PersonTraining pt where pt.person = :person and pt.traing = :training order by pt.dueDate")

returning a single object


Issue Links:

Referenced from: pull request #116

4 votes, 9 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jan 11, 2012

Oliver Drotbohm commented

The former is already possible simply using:

@Query("your query")
List<Person> findPersons(String parameter, Pageable pageable);

Regarding the second idea: how do you think should we react on a Pageable with a page size greater than one being passed into the method?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jan 16, 2012

Donovan commented

This is my simple and working and dirty solution for now.

@Query(value="select pt from PersonTraining pt where pt.person = :personProxy and pt.training = :training  order by pt.dueDate desc")
@QueryHints({@QueryHint(name="TOP",value="1")})
PersonTraining getLastTraining(@Param("personProxy")PersonProxy personProxy,
			@Param("training")Training training);
@Query(value="select pt from PersonTraining pt where pt.person = :personProxy order by pt.dueDate desc")
@QueryHints({@QueryHint(name="TOP",value="10")})
List<PersonTraining> getLast10Trainings(@Param("personProxy")PersonProxy personProxy);


.............
	protected JpaQueryExecution getExecution() {
		List<QueryHint> x = method.getHints();
		for (QueryHint queryHint : x) {
			if(queryHint.name().equalsIgnoreCase("TOP")){
				final Integer top = Integer.valueOf(queryHint.value());
				if(top==1 && method.getType() == Type.SINGLE_ENTITY){
					return new JpaQueryExecution() {
						@Override
						protected Object doExecute(AbstractJpaQuery query, Object[] values) {
							return query.createQuery(values).setFirstResult(0).setMaxResults(top).getSingleResult();
						}
					};
				}else{
					return new JpaQueryExecution() {
						@Override
						protected Object doExecute(AbstractJpaQuery query, Object[] values) {
							return query.createQuery(values).setFirstResult(0).setMaxResults(top).getResultList();
						}
					};
				}
			}
		}
		
		switch (method.getType()) {

		case COLLECTION:
			return new CollectionExecution();
		case PAGING:
			return new PagedExecution(method.getParameters());
		case MODIFYING:
			return method.getClearAutomatically() ? new ModifyingExecution(method, em) : new ModifyingExecution(method, null);
		default:
			return new SingleEntityExecution();
		}
	}
..............
@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 2, 2013

Dejan Predovic commented

It's not my issue, but I'd like similar functionality. The problem with paging is that (as it is) it's always going to execute the counting query, and in many cases the total count is irrelevant and/or expensive. I guess we could add another param to PageRequest - something like boolean noCount, that would skip the counting query and fill the corresponding Page values with dummies

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 3, 2013

Oliver Drotbohm commented

Are you aware of using List as return value in query methods avoids the execution of the additional count query? You also might wanna watch DATAJPA-306 for an additional approach avoiding the count query entirely

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 3, 2013

Dejan Predovic commented

Great, didn't know that. The only problem I can see with that solution is that it's finder-method-based and not parameter-based, so that if I want to execute the same query as both counting and non-counting, I need to define two finders that differ only in the result type

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 6, 2013

Piotr Betkier commented

In my opinion, the advantage of the proposed solution with firstresult and maxresult over using PageRequest is the resulting API clarity. As a user of some repository I would prefer a method for finding e.g. last Event to be called findLast() over findAll(new PageRequest(0, 1, Sort.Direction.DESC, "eventDate")). Especially if I plan use this method in a lot of places, creating this PageRequest each time is a DRY violation

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 11, 2018

Jens Schauder commented

Feedback has been given in the previous comments

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 23, 2019

Jens Schauder commented

It seems there are the following concerns:

  1. adding a Pageable argument is a valid workaround, but it does make the API more flexible than it should be.
  2. what happens if limits are given in the annotation and also as a pageable argument?
  3. it is considered desirable that a return type that is not a collection or similar signals an implicit maxResults=1

I propose the following solution:

  1. do introduce the requested attributes
  2. if the attributes are set and a Pageable argument is present fail validation.
  3. Do not implement number 3 from above. It would change existing behavior. See DATAJPA-666
  4. An implementation should limit the query and not just execute the query and then throw away part of the results. See Query.setFirstResult and Query.setMaxResults.

Since there is a workaround I consider this low priority, but will consider PRs matching the requirements

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 23, 2019

Jens Schauder commented

The single entity approach is covered by DATAJPA-666 which currently has an unfinished discussion if loosing the exception when multiple rows are returned is acceptable.

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 28, 2019

Jens Schauder commented

I updated my comment above. A change breaking existing code is not desirable

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Feb 10, 2020

Jens Schauder commented

There are various options available:

  • Use a Pageable as additional parameter
  • Use a native query.
  • Use a custom implementation.

We don't think that the effort of implementing an maintaining is worth it in this case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants