spring-projects / spring-data-jpa

Simplifies the development of creating a JPA-based data access layer.
https://spring.io/projects/spring-data-jpa/
Apache License 2.0
2.98k stars 1.41k forks source link

Allow access to query method parameters from within the query declaration [DATAJPA-723] #1098

Closed spring-projects-issues closed 2 years ago

spring-projects-issues commented 9 years ago

Caleb Cushing opened DATAJPA-723 and commented

So we have some code that uses some complex logic for sorting, in an external if statement we determine which of these 2 implementations to run. I would like to not have to do both of these methods, but instead have the one that takes Pageable and find a way to encode the order by into an Order object as either a jpql snippet or a criteria builder object. That way I could only have the one query and just conditionally choose the order (and have it support the asc/desc parameter).

if this is already possible, documentation would be wonderful

    Page<UserActivityLog> findByActivityTargetUserId( Long id, Pageable pageable );

    @Query( "from UserActivityLog al where al.activityTargetUser.id = ?1 "
                    + " order by case "
                    + " when al.activitySourceUser.primaryOrganization.ownerOrganization = true "
                    + " then al.activitySourceUser.role.roleType "
                    + " else al.activitySourceUser.contact.fullName "
                    + " end ?#{ #pageable.getOrderFor('activitySourceUser.contact.fullName').direction } ") Page<UserActivityLog> findByActivityTargetUserIdOrderByUserForNonAdmin( Long id, Pageable pageable );

No further details from DATAJPA-723

spring-projects-issues commented 9 years ago

Thomas Darimont commented

Hi Caleb,

what check do you do to decide which repository method to use? We have some means to express something similar of what you are trying to do via SpEL expressions in @Query annotations. See: https://github.com/spring-projects/spring-data-examples/blob/8ce6ac38c0bc53bb52b69390a6881a6a7d2800f5/jpa/security/src/main/java/example/springdata/jpa/security/SecureBusinessObjectRepository.java#L44

Cheers, Thomas

spring-projects-issues commented 9 years ago

Caleb Cushing commented

it's an if condition based around some existing business logic (a service method ), however it doesn't look like SPeL works for me in 1.6.5 (we're stuck on Spring 3 atm) . obviously I wouldn't expect this feature to be added to 1.6, it just looks like a useful feature to add based on this experience. Looking at the logic behind that method suggests to me that it would be possible to do it in a query, though I still think it might be nice to just segment off this bit in a compiletime safe way that didn't involve me writing the @Query, and now messing around with the sort direction because that's static.

Caused by: org.hibernate.QueryException: unexpected char: '#' [from com.mycorp.model.principal.UserActivityLog al where al.activityTargetUser.id = :?1  order by case  when al.activitySourceUser.primaryOrganization.ownerOrganization = true  then al.activitySourceUser.role.roleType  else al.activitySourceUser.contact.fullName  end ?#{ #pageable.getOrderFor('activitySourceUser.contact.fullName').direction } ]
    at org.hibernate.hql.internal.ast.QueryTranslatorImpl.doCompile(QueryTranslatorImpl.java:224)
    at org.hibernate.hql.internal.ast.QueryTranslatorImpl.compile(QueryTranslatorImpl.java:137)
    at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:105)
    at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:80)
    at org.hibernate.engine.query.spi.QueryPlanCache.getHQLQueryPlan(QueryPlanCache.java:168)
    at org.hibernate.internal.AbstractSessionImpl.getHQLQueryPlan(AbstractSessionImpl.java:221)
    at org.hibernate.internal.AbstractSessionImpl.createQuery(AbstractSessionImpl.java:199)
    at org.hibernate.internal.SessionImpl.createQuery(SessionImpl.java:1778)
    at org.hibernate.ejb.AbstractEntityManagerImpl.createQuery(AbstractEntityManagerImpl.java:291)
spring-projects-issues commented 9 years ago

Caleb Cushing commented

noticed I didn't say that I kind of imagined this working like a Specification, but for Sort/Order, instead of the "where"

spring-projects-issues commented 9 years ago

Oliver Drotbohm commented

We're not going to blur the line between declared queries and the assembly of queries from dynamic predicates. Also, the Dijkstra release train (the last Spring 3 compatible version) is in pure maintenance mode. So I suggest to upgrade to a recent release train (incl. Spring 4) and see how far you get with SpEL as the essence of what you present in the original description is available through it.

This basically seems to run into the "works as designed" direction as it doesn't seem to be about a feature missing but you not being able to upgrade to a version that supports what you ask for.

spring-projects-issues commented 9 years ago

Caleb Cushing commented

just in case someone else runs into this, here's the solution I came up with, personally I don't think it's unreasonable to ask for an OrderSpecification that returns an Order instead of a Predicate, delegating to another predicate that was only written due to lack of this feature seems dirty, SpEL might be able to solve the problems, but I think it's questionable as to whether it's a good idea to use that for my case, it lacks the potential compiletime safety of criteria builder.

I did try to see if there was some alternative to me writing this static direction method but didn't find one.

public class FilterByIdForUserSpecification<AL extends AbstractActivityLogWithSiteUser> implements Specification<AL>
{
    private final Sort.Direction direction;
    private final Specification<AL> specification;

    public FilterByIdForUserSpecification(
            final Specification<AL> specification,
            final Sort.Direction direction )
    {
        this.specification = specification;
        this.direction = direction;
    }

    @Override
    public Predicate toPredicate(
            final Root<AL> root,
            final CriteriaQuery<?> query,
            final CriteriaBuilder cb )
    {
        Path<Object> siteUser = root.get( AbstractActivityLogWithSiteUser.Index.USER );
        Path<Object> ownerOrg = siteUser.get( SiteUser.Index.PRI_ORG ).get( Organization.Index.OWNER );
        Path<Object> fullName = siteUser.get( SiteUser.Index.CONTACT ).get( Contact.Index.FULL_NAME );
        Path<Object> roleDisp = siteUser.get( SiteUser.Index.ROLE ).get( Role.Index.DISPLAY );

        Expression<Object> queryCase = cb.selectCase()
                .when( cb.equal( ownerOrg, true ), roleDisp )
                .otherwise( fullName );

        query.orderBy( direction( cb, queryCase, direction ) );
        return specification.toPredicate( root, query, cb );
    }

    static Order direction( final CriteriaBuilder cb, final Expression<?> e, final Sort.Direction direction )
    {
        if ( direction == Direction.ASC )
        {
            return cb.asc( e );
        }
        return cb.desc( e );
    }
}
spring-projects-issues commented 9 years ago

Caleb Cushing commented

so friday on my path to converting 6 db tables from being lucene paged and sorted, we discovered that 2 of the tables have a column that we sort by that is a CLOB, of course oracle can't sort clobs (for some reason, would seems like they could do the good enough approach (speculate ours can be sorted by the first 20 characters )). We use vaadin to display the data, and vaadin, like any good dynamic table allows you to click a column header and sort it. At this point that means 2 of 4 displayed columns needs a custom order by (because I think I can do an order by substring) and I need some way of mapping my sort key to my custom order by, because I may need a different order by for each of the 4 columns displayed and asc/desc must be dynamic. Most of these have a very simple where id = :long clause. The default sort may sort more than one column and need more than one custom sort.

here is how I think I'm going to solve this problem (pseudocode)

public interface JpaOrderSpecification {
      Order toOrder( ... same params as Predicate spec );
}
public class MySpec implements Specification {

        public MySpec( Direction dir, Iterable<JPaOrderSpecification> specOrders ) {
        ...
        }

     Predicate toPredicate( ...args ) {
        ...
      cb.orderBy( specOrders.stream().map( (o) -> { direction( o.toOrder( ...args ) ) });

       return cb.where( id... ); // or whatever
     }
}

what I'd like to write however is this

MyRepository extends PagingAndSorting {
   Page<> findById( Long id, OrderSpecification os, Pageable page );
}

of course one quandry with the above is how best to map pageable getSort() sort keys to custom specifcations. In my case I think an alternative would be that it'd be ok to have a JpaPageable that has it's own JpaSortable that could do this.

new JpaPageRequest( 1, 15,
 new JpaSort(
    new JpaOrder(   new MyJpaOrderSpec(), Direction.ASC )
    new JpaOrder(   new MyOtherOrder(), Direction.Desc )
));

to reiterate the problem is that I may need a different completely custom order criteria for ever displayed column, the where clause doesn't change, and that order by needs to change to asc/desc based on which way the displayed column click specified

spring-projects-issues commented 8 years ago

Caleb Cushing commented

recently discovered that the ordering in the specification causes h2 to throw exceptions because it's not ignored by the count like jpql is. This does seem to be dependent on the SQL engine, as the problem doesn't occur on Oracle.

http://stackoverflow.com/q/33192872/206466

Hibernate: select count(log0_.id) as col_0_0_ from log log0_ group by log0_.created order by log0_.created desc, log0_.entry asc
2015-10-17 20:49:26.335  WARN 11919 --- [io-8080-exec-10] o.h.engine.jdbc.spi.SqlExceptionHelper   : SQL Error: 90016, SQLState: 90016
2015-10-17 20:49:26.335 ERROR 11919 --- [io-8080-exec-10] o.h.engine.jdbc.spi.SqlExceptionHelper   : Column "LOG0_.ENTRY" must be in the GROUP BY list; SQL statement:
select count(log0_.id) as col_0_0_ from log log0_ group by log0_.created order by log0_.created desc, log0_.entry asc [90016-188]
2015-10-17 20:49:26.337 ERROR 11919 --- [io-8080-exec-10] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is org.springframework.orm.jpa.JpaSystemException: could not extract ResultSet; nested exception is org.hibernate.exception.GenericJDBCException: could not extract ResultSet] with root cause
org.h2.jdbc.JdbcSQLException: Column "LOG0_.ENTRY" must be in the GROUP BY list; SQL statement:
select count(log0_.id) as col_0_0_ from log log0_ group by log0_.created order by log0_.created desc, log0_.entry asc [90016-188]
    at org.h2.message.DbException.getJdbcSQLException(DbException.java:345) ~[h2-1.4.188.jar:1.4.188]
    at org.h2.message.DbException.get(DbException.java:179) ~[h2-1.4.188.jar:1.4.188]
    at org.h2.message.DbException.get(DbException.java:155) ~[h2-1.4.188.jar:1.4.188]
spring-projects-issues commented 8 years ago

Caleb Cushing commented

maybe another solution to improve the robustness of specifications would be to give them some sort of query context parameter, tell me that I'm running a count, give me access to any additional parameters to the repository.

It seems weird that specifications may actually be less powerful than JPQL + SpEL

gregturn commented 2 years ago

I don't see anything specific here that needs to be addressed inside Spring Data JPA.

Either work with one of our currently supported options: Specification, QueryDSL, SpEL, native or JPQL.

Veering into the range of CLOBs further pushes this out of range. CLOB columns are really data store specific and require custom handling.