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

Panache Query projection doesn't work as expected when entity has an optional one-to-one field #36497

Open
ha-shine opened this issue Oct 16, 2023 · 16 comments
Labels
area/hibernate-orm Hibernate ORM kind/bug-thirdparty Bugs that are caused by third-party components and not causing a major dysfunction of core Quarkus.

Comments

@ha-shine
Copy link

Describe the bug

If the entity has an optional one-to-one field, and use project, the find query will only return rows where the referenced column exist. My current work around is to skip the project and just do manual mapping in Kotlin.

Expected behavior

It should return every rows including those with optional field empty.

Actual behavior

No response

How to Reproduce?

Say you have these entities -

class Person: PanacheEntity() {
    @OneToOne(mappedBy = "job", fetch = FetchType.EAGER, optional = true)
     open var job: Job? = null
}

class Job: PanacheEntity() {
    @OnetoOne(fetch = FetchType.EAGER)
    @JoinColumn(name = "person_id")
    open lateinit var person: Person
}

And you have a DTO like this to project Person entity -

class PersonDto(val job: Job?)

And you use the query like this

repository.find("id = ?1", id).project(PersonDto::class.java)

In the result, you will only see Person object who has a corresponding Job even though Person rows without a job should also be included in the result.

Output of uname -a or ver

Darwin Htets-Mac-Studio.local 23.0.0 Darwin Kernel Version 23.0.0: Fri Sep 15 14:41:43 PDT 2023; root:xnu-10002.1.13~1/RELEASE_ARM64_T6000 arm64

Output of java -version

17.0.8

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.4.3

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

Gradle 8.1.

Additional information

No response

@ha-shine ha-shine added the kind/bug Something isn't working label Oct 16, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 16, 2023

/cc @FroMage (panache), @evanchooly (kotlin), @geoand (kotlin), @loicmathieu (panache)

@BlackSharkCZE
Copy link

I have the same problem. Result sql is using JOIN, so, when missing releation in database, whole result is empty. There is no way, how to change join to 'left/right join'.

@kosti31
Copy link

kosti31 commented Apr 25, 2024

facing the same issue even on @manytoone relationships where the field (foreign key) may be null. Seems like Panache ignores optional=true, nullable=true and even jakarta @nullable
the problem is evident when using project(). Query works fine when not using project.

@darmanovic
Copy link

Any update on this?

I don't understand why LEFT JOIN isn't the default option instead of INNER JOIN in this case.

@darmanovic
Copy link

darmanovic commented Jun 6, 2024

Small update, manual joining may do the trick. Interestingly, relation MUST be named same as ProjectedFieldName value or relation name itself!

This would work. Notice relation - without it it would build SQL query using JOIN instead of LEFT JOIN.

repository.find("FROM MyEntity me LEFT JOIN me.relation relation").project(EntityDTO.class).list()

Changing me.relation relation to me.relation or me.relation someOtherName negates the effect of this 'fix'.

P.S. Using RIGHT join does joining TWICE.

@darmanovic
Copy link

Another small update, this has nothing to do with projections, but with way how hibernate handles implicit joins. So won't get fixed.

@gsmet
Copy link
Member

gsmet commented Dec 27, 2024

@yrodiere it looks more like an Hibernate ORM behavior than a Quarkus issue?

@gsmet gsmet added the triage/needs-feedback We are waiting for feedback. label Dec 27, 2024
@yrodiere
Copy link
Member

yrodiere commented Jan 2, 2025

Hey,

@yrodiere it looks more like an Hibernate ORM behavior than a Quarkus issue?

It does. Though since it was marked as Panache only, I didn't get notified about it :)

@ha-shine @darmanovic there's been fixes for similar bugs in recent versions of Hibernate ORM; is this still a problem with Quarkus 3.17.5?
If so, can you please provide a reproducer (a small Quarkus project demonstrating the issue)?

@yrodiere yrodiere added area/hibernate-orm Hibernate ORM and removed area/panache labels Jan 2, 2025
@geoand geoand added triage/needs-reproducer We are waiting for a reproducer. and removed triage/needs-feedback We are waiting for feedback. labels Jan 10, 2025
@kosti31
Copy link

kosti31 commented Jan 15, 2025

update: tested again with v 3.17.6, problem remains the same.
I also noticed that running the same query with count() returns the correct number of rows, it's list() that has a problem.
repository.find("id = ?1", id).project(PersonDto::class.java).count() returns correct number of rows
repository.find("id = ?1", id).project(PersonDto::class.java).list() excludes rows with job null
same happens when using active record pattern.

@yrodiere
Copy link
Member

Thanks for checking! Can you please provide a reproducer?

@darmanovic
Copy link

@yrodiere

Sorry for late reply, please check it here.

./mvnw test

yrodiere added a commit to yrodiere/hibernate-test-case-templates that referenced this issue Jan 23, 2025
@yrodiere
Copy link
Member

yrodiere commented Jan 23, 2025

Thanks @darmanovic for the reproducer.

First, I agree this has nothing to do with projections, and nothing to do with Quarkus: it's purely related to Hibernate ORM, as you mentioned.

I created a reproducer with vanilla Hibernate ORM: https://github.com/yrodiere/hibernate-test-case-templates/tree/quarkus-36497

The crux of the issue is that in this query:

SELECT document.id, documentType.name
FROM Document document LEFT JOIN document.documentType
WHERE documentType.id IS NULL OR documentType.id > 0

... The left join is actually pointless.

Indeed, we're writing documentType.name, but there is no "root" in the query named documentType, so Hibernate will fall back to looking for a documentType attribute in query roots. It will find one in document, and will substitute documentType.name with document.documentType.name, which means an inner join will be added to the query (that's per the JPA spec: "Path expression navigability is composed using “inner join” semantics")

On the other hand, the following query would work exactly as you expect, because there is a documentType root:

SELECT document.id, documentType.name
FROM Document document LEFT JOIN document.documentType documentType
WHERE documentType.id IS NULL OR documentType.id > 0

There is a second issue in the behavior of getResultCount(): apparently getResultCount() will remove any element from the select clause (which makes sense), along with any implicit inner join (which makes less sense IMO). The inner join being removed, you will end up with the behavior you were initially looking for, akin to a left join.

@gavinking Do you think there's something we could improve in Hibernate?

For the first issue I think the behavior is correct, but dodgy. Maybe pointless joins could be reported with warnings/errors? Or we could just forbid left joins that don't specify a "root name". In either case we'd have to disable these behavior in JPA compliance mode, though...

For the second issue I'd lean towards considering it's a bug, considering the javadoc of getResultCount(), which implies strong consistency with getResultList:

Determine the size of the query result list that would be returned by calling getResultList() with no offset or limit applied to the query.

@yrodiere yrodiere added kind/bug-thirdparty Bugs that are caused by third-party components and not causing a major dysfunction of core Quarkus. and removed kind/bug Something isn't working area/kotlin triage/needs-reproducer We are waiting for a reproducer. labels Jan 23, 2025
@yrodiere
Copy link
Member

For the second issue I'd lean towards considering it's a bug, considering the javadoc of getResultCount(), which implies strong consistency with getResultList:

Determine the size of the query result list that would be returned by calling getResultList() with no offset or limit applied to the query.

I created https://hibernate.atlassian.net/browse/HHH-19065 upstream, feel free to chime in there.

@gavinking
Copy link

For the first issue I think the behavior is correct, but dodgy. Maybe pointless joins could be reported with warnings/errors?

I think you're right, and we might be able to treat a (non-fetch) left join with no identification variable as an error, at least in the case where there's an explicit select clause.

In either case we'd have to disable these behavior in JPA compliance mode, though...

Nope, JPA requires the identification variable, so the spec is not a constraint here.

For the second issue I'd lean towards considering it's a bug

It's definitely a bug.

@darmanovic
Copy link

Thanks @yrodiere and @gavinking for your responses.

As a developer who's been using Panache, I find it unfortunate that Panache doesn’t generate LEFT JOIN with identification variable, but I can understand why that’s the case at the moment.

If it did, working with projections would be so much easier!

@yrodiere
Copy link
Member

FWIW after some discussion I submitted a suggestion to the JPA spec, which may make your use case more straightfoward to address: jakartaee/persistence#697

To anyone getting here by accident (I know @darmanovic understands that, but want to be clear), note this is an improvement aiming at making the syntax less verbose. It does not add any feature, you can do the same currently by specifying left joins in your query explicitly (and naming the result of these joins, see #36497 (comment))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-orm Hibernate ORM kind/bug-thirdparty Bugs that are caused by third-party components and not causing a major dysfunction of core Quarkus.
Projects
None yet
Development

No branches or pull requests

8 participants