Update Panache docs to mention that `Sort` requires users to disable escaping when using functions

pedropds commented 2 months ago

Describe the bug

I'm updating Quarkus from 3.8.4 to the latest version, 3.14.1. (using gradle, btw) When doing this upgrade, some of our integration tests started failing, specifically some of the ones using Panache queries, with Sort options, when trying to sort with case insensitive, by using the lower() or upper() functions.

Let's take a simple example, from my codebase:

@Table(name = "user")
public class User extends PanacheEntityBase { 
     .....
     String name;
}

public class UserRepository implements PanacheRepository<User> {

    ......
    public List<User> getUsers() {
        Sort sortCaseInsensitive = Sort.by("lower(name)").direction(Sort.Direction.Descending);
        List<User> users = find("SELECT u FROM User u", sortCaseInsensitive);
        return users;
    }  
}

Expected behavior

In Quarkus 3.8.4, this should allow the query to execute correctly without throwing any exceptions, enabling a case-insensitive sort.

Actual behavior

The following exception is now thrown:

Could not interpret path expression 'lower(name)'
org.hibernate.query.SemanticException: Could not interpret path expression 'lower(name)'
    at org.hibernate.query.hql.internal.BasicDotIdentifierConsumer$BaseLocalSequencePart.resolvePathPart(BasicDotIdentifierConsumer.java:240)
    at org.hibernate.query.hql.internal.BasicDotIdentifierConsumer.consumeIdentifier(BasicDotIdentifierConsumer.java:92)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.resolveOrderByOrGroupByExpression(SemanticQueryBuilder.java:1707)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitSortExpression(SemanticQueryBuilder.java:1796)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitSortSpecification(SemanticQueryBuilder.java:1755)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitOrderByClause(SemanticQueryBuilder.java:1738)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitOrderByClause(SemanticQueryBuilder.java:1727)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitQueryOrder(SemanticQueryBuilder.java:1194)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitQuerySpecExpression(SemanticQueryBuilder.java:1043)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitQuerySpecExpression(SemanticQueryBuilder.java:277)
    at org.hibernate.grammars.hql.HqlParser$QuerySpecExpressionContext.accept(HqlParser.java:2134)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitSimpleQueryGroup(SemanticQueryBuilder.java:1025)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitSimpleQueryGroup(SemanticQueryBuilder.java:277)
    at org.hibernate.grammars.hql.HqlParser$SimpleQueryGroupContext.accept(HqlParser.java:2005)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitSelectStatement(SemanticQueryBuilder.java:492)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.visitStatement(SemanticQueryBuilder.java:451)
    at org.hibernate.query.hql.internal.SemanticQueryBuilder.buildSemanticModel(SemanticQueryBuilder.java:324)
    at org.hibernate.query.hql.internal.StandardHqlTranslator.translate(StandardHqlTranslator.java:71)
    at org.hibernate.query.internal.QueryInterpretationCacheStandardImpl.createHqlInterpretation(QueryInterpretationCacheStandardImpl.java:145)
    at org.hibernate.query.internal.QueryInterpretationCacheStandardImpl.resolveHqlInterpretation(QueryInterpretationCacheStandardImpl.java:132)
    at org.hibernate.query.spi.QueryEngine.interpretHql(QueryEngine.java:54)
    at org.hibernate.internal.AbstractSharedSessionContract.interpretHql(AbstractSharedSessionContract.java:831)
    at org.hibernate.internal.AbstractSharedSessionContract.interpretAndCreateSelectionQuery(AbstractSharedSessionContract.java:809)
    at org.hibernate.internal.AbstractSharedSessionContract.createSelectionQuery(AbstractSharedSessionContract.java:856)
    at io.quarkus.hibernate.orm.runtime.session.TransactionScopedSession.createSelectionQuery(TransactionScopedSession.java:1277)
    at org.hibernate.engine.spi.SessionLazyDelegator.createSelectionQuery(SessionLazyDelegator.java:749)
    at org.hibernate.Session_Su2omAGHZ_IxR-fCiSpnKyfZVSw_Synthetic_ClientProxy.createSelectionQuery(Unknown Source)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.createBaseQuery(CommonPanacheQueryImpl.java:387)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.createQuery(CommonPanacheQueryImpl.java:348)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.list(CommonPanacheQueryImpl.java:301)
    at io.quarkus.hibernate.orm.panache.runtime.PanacheQueryImpl.list(PanacheQueryImpl.java:150)
    at my.packages.here.UserRepository.getUsers(UserRepository.java:58)

How to Reproduce?

Just reproduce this with the simple scenario I provided above, or any other by your choice, and change your gradle.properties file (not sure the process for maven):

quarkusPluginVersion=3.8.4
quarkusPlatformVersion=3.8.4

to

quarkusPluginVersion=3.14.1
quarkusPlatformVersion=3.14.1

Quarkus version or git rev

3.14.1

Build tool (ie. output of `mvnw --version` or `gradlew --version`)

Gradle 8.5

Additional information

Im using Postgres for the database, and then just using the default libraries from Quarkus, the main one for this case being the Hibernate:

implementation 'io.quarkus:quarkus-hibernate-orm-panache'

quarkus-bot[bot] commented 2 months ago

/cc @FroMage (panache), @loicmathieu (panache)

FroMage commented 2 months ago

This sounds like an ORM change, no @yrodiere ?

yrodiere commented 2 months ago

This sounds like an ORM change, no @yrodiere ?

Unless Panache is introducing non-printable characters somewhere in lower(name), which sounds unlikely, yes this looks like a bug in Hibernate ORM.

yrodiere commented 2 months ago

This sounds like an ORM change, no @yrodiere ?

Unless Panache is introducing non-printable characters somewhere in lower(name), which sounds unlikely, yes this looks like a bug in Hibernate ORM.

Ah, that was wrong @FroMage -- the problem is in Panache.

I debugged it locally and this is what gets passed to Hibernate ORM:

FROM `org.acme.hibernate.orm.panache.entity.FruitEntity` ORDER BY `lower(name)`

So essentially I think it's a result from that patch that added escapes everywhere to avoid HQL injection.

@pedropds you need to use Sort.by("lower(name)").disableEscaping() @FroMage you need to make sure migration guides (and the Panache documentation) mention all this :)

pedropds commented 2 months ago

Was about to send ORDER BY 'lower(name)' as well as I was debugging to see if i could provide more info.

Thank you both for the help!

FroMage commented 2 months ago

OH, that makes sense, thanks @yrodiere :)

I'll rewrite this issue's topic.

quarkusio / quarkus