quarkusio / quarkus

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

Hibernate generates wrong SQL #20264

Closed jw941 closed 3 years ago

jw941 commented 3 years ago

Describe the bug

The update SQL Hibernate generated was wrong which caused exception.

HQL: update Document d set d.processed=false, d.error=false where d.documentGroup.domain=:domain and d.documentTypeDefinition=:documentTypeDefinition

SQL: update recognition_document cross join set processed=false, error=false where domain=? and document_type=?

Expected behavior

update recognition_document set processed=false, error=false where domain=? and document_type=?

Actual behavior

Caused by: org.postgresql.util.PSQLException: ERROR: syntax error at or near "cross"

How to Reproduce?

Here is the reproducer (Docker is required to run the test case). https://github.com/jw941/hibernate-bug

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.2.3.Final

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

No response

Additional information

No response

quarkus-bot[bot] commented 3 years ago

/cc @Sanne, @gsmet, @yrodiere

gastaldi commented 3 years ago

Sounds like an Hibernate Core bug?

jw941 commented 3 years ago

Yea, seems Hibernate doesn't convert HQL to SQL correctly. As far as I am aware, this happens to update only. Not sure if select or others are affected.

Sanne commented 3 years ago

Let's keep in mind that a cross join might be deemed necessary by Hibernate ORM depending on how the relations are modelled.

So, yes this could be an issue in Hibernate, but it could also be an issue with your mapping.

@beikov could you have a look please, and try to classify & prioritize this issue?

beikov commented 3 years ago

Thanks for the issue report, but this is not a "bug" but a feature request. You can't access non-FK attributes of an association directly in a DML statement as that would require inner join semantics as per the JPA specification. We currently wrongly "allow" this and render a join. The "workaround" is to use an exists subquery predicate:

update Document d 
set d.processed=false, d.error=false 
where d.documentTypeDefinition=:documentTypeDefinition
and exists (
  select 1
  from d.documentGroup dg
  where dg.domain=:domain
)
Sanne commented 3 years ago

Ok right let's close this then. @jw941 you're right that we might need to improve the feedback (such as disallowing this explicitly like @beikov suggests), but that belongs on the Hibernate issue tracker, and I should say it's unlikely to be fixed in Hibernate ORM 5. Let's track it for version 6!

Thanks

jw941 commented 3 years ago

@beikov "documentGroup.domain" is accessed via a FK and it used to work. Not sure from which version, it stops working. And yes, it belongs to Hibernate issue rather than Quarkus.

beikov commented 3 years ago

@jw941 accessing the FK is when you would use d.documentGroup.id = :someId. You are specifying that the FK on the document table points to the PK of the document group table: https://github.com/jw941/hibernate-bug/blob/main/src/main/java/com/example/entity/Document.java#L36

This kind of access never worked for DML queries, unless your databases supported the join syntax in update/delete.