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

Direct access to protected or package-private entity fields does not trigger SQL updates in Hibernate ORM #32735

Closed
mensinda opened this issue Apr 18, 2023 · 5 comments · Fixed by #32876
Assignees
Labels
area/hibernate-orm Hibernate ORM kind/bug Something isn't working
Milestone

Comments

@mensinda
Copy link

Describe the bug

When updating only certain fields from certain locations, the changes are not committed. See the reproducer here.

Part of this issue might be related to the issue in Hibernate 5.x where the generic is not updated in #32532.

Expected behavior

All tests pass.

Actual behavior

Test failures because the changes to the entity are not written to the DB when:

  • updating a protected field from the base class in the Entity subclass
  • updating a package private field outside the Entity

What is surprising to me is that updating public fields outside the Entity works apparently...

How to Reproduce?

To reproduce the error:

  1. Check out the stringBaseDirty branch of this repository: https://github.com/mensinda/quarkus-stuff/tree/stringBaseDirty
  2. run mvn clean verify

To see that it is also broken in Quarkus 2.x:

  1. run ./downgrade.sh
  2. run mvn clean verify

Output of uname -a or ver

Linux 5.15.0-56-generic #62-Ubuntu SMP Tue Nov 22 19:54:14 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "17.0.6" 2023-01-17

GraalVM version (if different from Java)

No response

Quarkus version or git rev

3.0.0.CR2 / 2.16.6.Final

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

Apache Maven 3.8.2 (ea98e05a04480131370aa0c110b8c54cf726c06f)

Additional information

No response

@mensinda mensinda added the kind/bug Something isn't working label Apr 18, 2023
@geoand geoand added area/hibernate-orm Hibernate ORM and removed triage/needs-triage labels Apr 19, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 19, 2023

/cc @Sanne (hibernate-orm), @gsmet (hibernate-orm), @yrodiere (hibernate-orm)

@yrodiere yrodiere self-assigned this Apr 21, 2023
@yrodiere
Copy link
Member

Thanks for the report, I confirmed this locally and have reproducers that I can add to the Quarkus test suite.

It seems caused by the fact we don't replace direct access to fields by calls to accessors:

  • for non-public fields
  • for fields of superclasses

I'll try to see if I can change that.

I doubt we will backport such a change to 2.x though, as it has potential to break applications in unexpected ways.

@yrodiere yrodiere changed the title Entities are not always updated under certain circumstances Direct access to protected or package-private entity fields does not trigger SQL updates in Hibernate ORM Apr 25, 2023
@mensinda
Copy link
Author

I doubt we will backport such a change to 2.x though, as it has potential to break applications in unexpected ways.

This is not an issue for us, since we are going to switch to Quarkus 3 anyway.


We have run some more tests internally and have found another edge case: Accessing private fields from an inner class (via a builder pattern for instance) also does not trigger an SQL update.

I have updated the TestEntity and added a test in my reproduncer repo

@yrodiere
Copy link
Member

Accessing private fields from an inner class (via a builder pattern for instance) also does not trigger an SQL update

I think that's getting into a grey area that I'm not sure we want to support, as instrumenting private field access could potentially come with a high performance cost and very weird edge-cases. Regardless of whether we will support this, I'd recommend to stay away from such access patterns.

@mensinda
Copy link
Author

I agree that building such Constructs is in 99% of all cases a bad Idea. Builder patterns are the only "valid" use-case I could come up with (and even then, this is only an issue if the persist is called before the fields are set...).

However, if this is not supported, I would recommend documenting this behavior since bugs resulting from this are very hard to track down otherwise (basically a warning in the docs that accessing private fields outside the entity is not supported and that such code will break).

@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 14, 2023
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 Something isn't working
Projects
None yet
3 participants