quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.75k stars 2.67k forks source link

null pointer in panache 'list' query with like clause #33310

Closed sjaakd closed 1 year ago

sjaakd commented 1 year ago

Describe the bug

@Entity
@Table(schema = "DINO_DBA", name = "GDW_LKS_CPT_RD")
@NamedQuery( name = "GdwLksCptRd.search", query = "from GdwLksCptRd p where p.broId like :criterion" )
public class GdwLksCptRd extends PanacheEntityBase implements Serializable {

    private static final long serialVersionUID = -2816021106611026573L;

    @Id
    @Column(name = "OBJECT_ID")
    private Long objectId;

    @Column( name = "BRO_ID" )
    private String broId;
}
    List<GdwLksCptRd> rs1 = GdwLksCptRd.list( "broId like ?1", criterion + "%");

Leads to:

16:02:40 WARN  [nl.bro.mic.com.rs.pro.DefaultExceptionProvider] (executor-thread-4) null: java.lang.NullPointerException
    at java.base/java.lang.String$CaseInsensitiveComparator.compare(String.java:1224)
    at java.base/java.lang.String$CaseInsensitiveComparator.compare(String.java:1218)
    at java.base/java.util.TreeMap.getEntryUsingComparator(TreeMap.java:374)
    at java.base/java.util.TreeMap.getEntry(TreeMap.java:343)
    at java.base/java.util.TreeMap.get(TreeMap.java:277)
    at org.hibernate.dialect.function.SQLFunctionRegistry.findSQLFunction(SQLFunctionRegistry.java:45)
    at org.hibernate.hql.internal.ast.util.SessionFactoryHelper.findSQLFunction(SessionFactoryHelper.java:386)
    at org.hibernate.hql.internal.ast.tree.IdentNode.getDataType(IdentNode.java:371)
    at org.hibernate.hql.internal.ast.HqlSqlWalker.lookupProperty(HqlSqlWalker.java:680)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.addrExpr(HqlSqlBaseWalker.java:5097)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.expr(HqlSqlBaseWalker.java:1315)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.exprOrSubquery(HqlSqlBaseWalker.java:4797)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.comparisonExpr(HqlSqlBaseWalker.java:4261)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.logicalExpr(HqlSqlBaseWalker.java:2180)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.whereClause(HqlSqlBaseWalker.java:841)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.query(HqlSqlBaseWalker.java:635)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.selectStatement(HqlSqlBaseWalker.java:339)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.statement(HqlSqlBaseWalker.java:287)
    at org.hibernate.hql.internal.ast.QueryTranslatorImpl.analyze(QueryTranslatorImpl.java:276)
    at org.hibernate.hql.internal.ast.QueryTranslatorImpl.doCompile(QueryTranslatorImpl.java:192)
    at org.hibernate.hql.internal.ast.QueryTranslatorImpl.compile(QueryTranslatorImpl.java:144)
    at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:112)
    at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:73)
    at org.hibernate.engine.query.spi.QueryPlanCache.getHQLQueryPlan(QueryPlanCache.java:162)
    at org.hibernate.internal.AbstractSharedSessionContract.getQueryPlan(AbstractSharedSessionContract.java:636)
    at org.hibernate.internal.AbstractSharedSessionContract.createQuery(AbstractSharedSessionContract.java:748)
    at org.hibernate.internal.AbstractSharedSessionContract.createQuery(AbstractSharedSessionContract.java:114)
    at io.quarkus.hibernate.orm.runtime.session.TransactionScopedSession.createQuery(TransactionScopedSession.java:357)
    at org.hibernate.engine.spi.SessionLazyDelegator.createQuery(SessionLazyDelegator.java:547)
    at org.hibernate.engine.spi.SessionLazyDelegator.createQuery(SessionLazyDelegator.java:65)
    at org.hibernate.Session_5b93bee577ae2f8d76647de04cfab36afbf52958_Synthetic_ClientProxy.createQuery(Unknown Source)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.createBaseQuery(CommonPanacheQueryImpl.java:361)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.createQuery(CommonPanacheQueryImpl.java:316)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.list(CommonPanacheQueryImpl.java:267)
    at io.quarkus.hibernate.orm.panache.runtime.PanacheQueryImpl.list(PanacheQueryImpl.java:149)

Same result for executing the query with @NamedQuery as arg:

GdwLksCptRd.find( "GdwLksCptRd.search", Parameters.with( "criterion", criterion + "%" ) ).range( 0, 29 ).list()

However executing a normal JPA query works as expected.

TypedQuery<GdwLksCptRd> query1 = em.createNamedQuery( "GdwLksCptRd.search", GdwLksCptRd.class ); query1.setParameter( "criterion", criterion + "%" );
List<GdwLksCptRd> rs1 = query1.getResultList();

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Output of uname -a or ver

No response

Output of java -version

openjdk 11

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.16.6

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

mvn

Additional information

No response

quarkus-bot[bot] commented 1 year ago

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

gsmet commented 1 year ago

Wondering if this is a Hibernate ORM issue. /cc @yrodiere @Sanne

marko-bekhta commented 1 year ago

hey, it seems that in the

GdwLksCptRd.find( "GdwLksCptRd.search", Parameters.with( "criterion", criterion + "%" ) ).range( 0, 29 ).list()

The named query is missing a # at the beginning of the string. According to the docs :

You can reference a named query instead of a (simplified) HQL query by prefixing its name with the '#' character. You can also use named queries for count, update and delete queries.

so something like this:

GdwLksCptRd.find( "#GdwLksCptRd.search", Parameters.with( "criterion", criterion + "%" ) ).range( 0, 29 ).list()

should work (at least I've tested it without the # and got the same error as in the report, and after adding a # all is ok)

loicmathieu commented 1 year ago

Yes, named query in a PanacheQL query (the simplified JPQL that is accepted by all Panache methods) must start with # so we can distinguished them from simplified queries like Person.find("name", "John Doe").

@sjaakd can you confirm that it's working ?

gsmet commented 1 year ago

Hmmm, if it fixes it, we definitely need to improve and provide a proper error message and not a NPE.

sjaakd commented 1 year ago

Thanks guys. I'll check that one asap.. Need to rewrite quite some code to test again, so will need some time. Could it be so simple?

I tried querying directly: List<GdwLksCptRd> rs1 = GdwLksCptRd.list( "broId like ?1", criterion + "%"); like is suggested here, although that's a repo example). It doesn't use the named query, but a query string leading to the same result.

Yes.. the NPE put me on a wrong track 😄 ..

Sanne commented 1 year ago

Hmmm, if it fixes it, we definitely need to improve and provide a proper error message and not a NPE.

Definitely. I'll track the ORM side - however this issue only applies to ORM5, as ORM6 treats functions in an entirely different way.

On the Panache side, it would be nice to detect that the simplified query is invalid and it happens to refer to a named query - not sure if we can do better.

sjaakd commented 1 year ago

@Sanne thanks..

In my case I have a number of quite similar objects that share all the same MappedSuperclass. This works quite nice for named queries. I tried to rewrite this to Panache to test this issue again .

So I've got a number of maps handling a function that calls this named query. This is how it looked like:

  private static final List<BiFunction<EntityManager, String, List<ObjectDto>>> DINO_OBJECT_QUERIES = List.of(
        (e, criterion) -> objectQuery( e, "GdwLksGboRd.search", criterion ),
        (e, criterion) -> objectQuery( e, "GdwLksAboRd.search", criterion ),
        (e, criterion) -> objectQuery( e, "GdwLksGsoRd.search", criterion )
        // etc
      );

and then:

    private static List<ObjectDto> objectQuery(EntityManager em, String queryName, String criterion ) {
        TypedQuery<ObjectDto> query = em.createNamedQuery( queryName, ObjectDto.class );
        query.setParameter( "criterion", criterion + "%" );
        query.setMaxResults( MAX_SEARCH_RESULTS_PER_DATA_TYPE );
        return query.getResultList();
    }
@NamedQuery(
    name = "GdwLksBhrGRd.search",
    query = "select new nl.bro.microservices.dlp.model.common.ObjectDto( p.broId, p.geometry, 'SOIL', 'BRO', 'bhrg' ) "
        + "from GdwLksBhrGRd p where p.broId like :criterion order by p.broId"
)

Now.. I want to rewrite this using panache and panache projection

So:

    private static final List<Function< String, List<ObjectDto>>> DINO_OBJECT_QUERIES = List.of(
        criterion -> objectQuery( GdwLksGboRd.find("#GdwLksGboRd.search",  Parameters.with( "criterion", criterion + "%" ) ) ),
        criterion -> objectQuery( GdwLksAboRd.find("#GdwLksAboRd.search",  Parameters.with( "criterion", criterion + "%" ) ) ),
        criterion -> objectQuery( GdwLksGsoRd.find("#GdwLksGsoRd.search",  Parameters.with( "criterion", criterion + "%" ) ) ),
   //: 
    private static <T extends PanacheEntityBase>  List<ObjectDto> objectQuery( PanacheQuery<T> query  ) {
        return query.range( 0, MAX_SEARCH_RESULTS_PER_DATA_TYPE ).project( ObjectDto.class ).list();
    }

example query:

@NamedQuery(
    name = "GdwLksBhrGRd.search",
    query = "select  p.broId, p.geometry, 'SOIL', 'BRO', 'bhrg'  "
        + "from GdwLksBhrGRd p where p.broId like :criterion order by p.broId"
)

Only to find out that Panache does not support @NamedQuery icw projection 😢 .. Note: cannot derive this from the documentation, but that could be me (not such a good reader). I always assumed there would be a performance gain by using named queries. Guess I've got to rewrite stuff a bit further..

Sanne commented 1 year ago

@FroMage ^

sjaakd commented 1 year ago

Ok.. I'm going to stop rewriting the named queries to Panache. I've found some more issues. I'll try to summarize them.

I've removed the named queries in favor of simple queries like this:

"CPT", criterion -> objectQuery( GdwLksCptRd.find( "broId like ?1", criterion + "%" ), BRO, SOIL, "cpt" ). However, i need different constructors in my target dto. When I do so, I run into NPE's.


    public ObjectDto(@ProjectedFieldName( "broId" ) String objectId, @ProjectedFieldName( "geometry" ) Geometry geometry) {
        this.objectId = objectId;
        if ( geometry != null ) {
            this.xCoordinate = geometry.getCoordinate().x;
            this.yCoordinate = geometry.getCoordinate().y;
        }
    }

   // adding this constructor gives the NPE
    public ObjectDto(@ProjectedFieldName( "dinoNumber" ) String objectId, @ProjectedFieldName( "xRdCrd" ) BigDecimal xCoordinate, @ProjectedFieldName( "yRdCrd" ) BigDecimal yCoordinate) {
        this.objectId = objectId;
        if ( xCoordinate != null ) {
            this.xCoordinate = xCoordinate.doubleValue();
        }
        if ( yCoordinate != null ) {
            this.yCoordinate = yCoordinate.doubleValue();
        }
    }
17:15:42 WARN  [nl.bro.mic.com.rs.pro.DefaultExceptionProvider] (executor-thread-0) null: java.lang.NullPointerException
    at org.hibernate.hql.internal.NameGenerator.generateColumnNames(NameGenerator.java:27)
    at org.hibernate.hql.internal.ast.util.SessionFactoryHelper.generateColumnNames(SessionFactoryHelper.java:435)
    at org.hibernate.hql.internal.ast.tree.SelectClause.initializeColumnNames(SelectClause.java:308)
    at org.hibernate.hql.internal.ast.tree.SelectClause.finishInitialization(SelectClause.java:298)
    at org.hibernate.hql.internal.ast.tree.SelectClause.initializeExplicitSelectClause(SelectClause.java:268)
    at org.hibernate.hql.internal.ast.HqlSqlWalker.useSelectClause(HqlSqlWalker.java:1039)
    at org.hibernate.hql.internal.ast.HqlSqlWalker.processQuery(HqlSqlWalker.java:807)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.query(HqlSqlBaseWalker.java:703)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.selectStatement(HqlSqlBaseWalker.java:339)
    at org.hibernate.hql.internal.antlr.HqlSqlBaseWalker.statement(HqlSqlBaseWalker.java:287)
    at org.hibernate.hql.internal.ast.QueryTranslatorImpl.analyze(QueryTranslatorImpl.java:276)
    at org.hibernate.hql.internal.ast.QueryTranslatorImpl.doCompile(QueryTranslatorImpl.java:192)
    at org.hibernate.hql.internal.ast.QueryTranslatorImpl.compile(QueryTranslatorImpl.java:144)
    at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:112)
    at org.hibernate.engine.query.spi.HQLQueryPlan.<init>(HQLQueryPlan.java:73)
    at org.hibernate.engine.query.spi.QueryPlanCache.getHQLQueryPlan(QueryPlanCache.java:162)
    at org.hibernate.internal.AbstractSharedSessionContract.getQueryPlan(AbstractSharedSessionContract.java:636)
    at org.hibernate.internal.AbstractSharedSessionContract.createQuery(AbstractSharedSessionContract.java:748)
    at org.hibernate.internal.AbstractSharedSessionContract.createQuery(AbstractSharedSessionContract.java:114)
    at io.quarkus.hibernate.orm.runtime.session.TransactionScopedSession.createQuery(TransactionScopedSession.java:357)
    at org.hibernate.engine.spi.SessionLazyDelegator.createQuery(SessionLazyDelegator.java:547)
    at org.hibernate.engine.spi.SessionLazyDelegator.createQuery(SessionLazyDelegator.java:65)
    at org.hibernate.Session_5b93bee577ae2f8d76647de04cfab36afbf52958_Synthetic_ClientProxy.createQuery(Unknown Source)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.createBaseQuery(CommonPanacheQueryImpl.java:361)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.createQuery(CommonPanacheQueryImpl.java:316)
    at io.quarkus.hibernate.orm.panache.common.runtime.CommonPanacheQueryImpl.list(CommonPanacheQueryImpl.java:267)
    at io.quarkus.hibernate.orm.panache.runtime.PanacheQueryImpl.list(PanacheQueryImpl.java:149)

The next point is that Panache will not allow me to use string literals in the "new Object" in projection, like this:

@NamedQuery(
    name = "GdwLksCptRd.search",
    query = "select new nl.bro.microservices.dlp.model.common.ObjectDto( p.broId, p.geometry, 'SOIL', 'BRO', 'cpt' ) "
        + "from GdwLksCptRd p where p.broId like :criterion order by p.broId"
)

this means that I've got to iterate over the result list and set the proper parameters afterwards.

I cannot use the flexibility that the @NamedQuery offers assigning different query parameters: sometimes I need a geometry from the entity itself, sometimes from its related parent (related entity). The @ProjectedFieldName( "dinoNumber" ) does not always offer me that flexibility. Besides: I'd like to have my queries close to the entities (maintainability perspective).

Last, but not least for you guys: I can confirm that the issue was indeed the missing #.

I hope you can somehow use my feedback above to improve on the Panache library. I really would like to use it. Especially when combining it with its accompanying unit test library. Now I've somehow got the figure out how to mock an Oracle DB in unit test (with native types in H2) or mock the entity manager.

Sanne commented 1 year ago

Great feedback, thanks! Regarding the Oracle DB specifically.. why not use the dev-services facility?

I believe there's no better way to test than to test on the real thing.. it will start a real Oracle container instance for you during tests; if you configure testcontainers for container reuse, it will also be very fast as it won't have to start the DB container often.

sjaakd commented 1 year ago

Happy to be useful 😄 .

Wrt to Oracle: we are possibly moving away from Oracle in the future. License costs / opensource vs closed source etc.

In short, it comes down what you consider to be abstraction. I think an ORM should provide database abstraction, not needing a dao, but my experience is otherwise. I try to encourage to use as much as standard SQL as possible in the teams. I just recently discovered rider-cdi and I really like it in combination with h2 and Quarkus. With a few annotations you setup your test data.

Keep up the good work here.. Its appreciated 👍 .

BTW: If you want to close the issue that's ok for me. I leave that up to you. My issues are handled.

FroMage commented 1 year ago

So, this issue turns out to have many parts:

Thanks for your feedback, this is very helpful.

yrodiere commented 1 year ago

FWIW https://hibernate.atlassian.net/browse/HHH-16609 has been fixed upstream, so the originally reported problems will be fixed in Quarkus as soon as Hibernate ORM 5.6.16 gets released and we upgrade in Quarkus 2.16.

@FroMage can you confirm I can close this ticket as soon as the upstream issue in ORM gets fixed in Quarkus, or do you need to keep it open to track those other problems you listed? I'd recommend opening another issue, but it's up to you.

FroMage commented 1 year ago

Most of these points I summarised are taken care of by https://github.com/quarkusio/quarkus/pull/33902 if you want to review it.

sjaakd commented 1 year ago

Hi guys,

Although closed I see I missed some questions. Sorry. For that

"The next point is that Panache will not allow me to use string literals in the "new Object" in projection, like this:" what's your error? I don't see why Panache would be involved in that one.

-> Panache is not involved here. It's just a feature that you get with standard named queries.. Just like this

@Entity
@Table(schema = "DINO_DBA", name = "GDW_LKS_SFR_RD")
@NamedQuery(
    name = "GdwLksSfrRd.search",
    query = "select new nl.bro.microservices.dlp.model.common.ObjectDto( p.broId, p.geometry, 'SOIL', 'BRO', 'sfr' ) "
        + "from GdwLksSfrRd p where p.broId like :criterion order by p.broId"
)

Note that you can set the constructor arguments with a literal? I miss that kind of functionality in Panache.

"I cannot use the flexibility that the @NamedQuery offers assigning different query parameters". Can you clarify? I don't understand the issue.

-> I'm lost here myself as well 😢

FroMage commented 1 year ago

Oh, OK, so you'd like to use literal values for some of the parameters of your projection constructor, that don't correspond to selected columns?

Perhaps some extra optional annotation parameter on @ProjectedFieldName(defaultValue = 'SOIL') or literal or something? I'm not sure if we want to support Java literals or DB literals (which are probably of a different set of types).

That could be useful, could you open an issue for that please? This way we can see if more people would find this useful.

Thanks.

yrodiere commented 1 year ago

Perhaps some extra optional annotation parameter on @ProjectedFieldName(defaultValue = 'SOIL') or literal or something? I'm not sure if we want to support Java literals or DB literals (which are probably of a different set of types).

Just an idea, but at that point you could just allow calling a static factory method instead of a constructor, and let users define that method and handle the default values. It could be even more powerful than annotations.

FroMage commented 1 year ago

Yes, true. Though this would not use HQL projection, right?

yrodiere commented 1 year ago

Yes, true. Though this would not use HQL projection, right?

I don't think HQL supports calling factory methods at the moment, no.