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

Explicit flush needed in 3.13.0 spring-data-jpa when running @Query that updates foreign key column with newly saved associated entity #42322

Closed
famod opened this issue Aug 5, 2024 · 15 comments
Labels
area/hibernate-orm Hibernate ORM area/spring Issues relating to the Spring integration kind/bug Something isn't working

Comments

@famod
Copy link
Member

famod commented Aug 5, 2024

Describe the bug

Starting from 3.13.0.CR1 (#41284 respectively), an explicit flush is needed to make sure that a @Query('update A set associatedB where ...') repository method does not run into a foreign key constraint violation when the other entity (here: B) was just created in the same transaction (shortly before the update).

In other words, the following fails in 3.13.0 (but works in 3.12):

  • B b = new B(...)
  • BRepository.save(b)
  • ARepository.updateAssociatedB(b) <- foreign key constraint violations (b doesn't exist)

Using BRepository.saveAndFlush(b) works around the issue.

Expected behavior

No explicit flush needed; persistence layer/Hibernate should detect on its own that a flush is needed.

Actual behavior

Explicit flush required.

How to Reproduce?

quarkusio/quarkus-quickstarts@main...famod:quarkus-quickstarts:spring-jpa-flush

  1. check out the spring-jpa-flush branch
  2. fails: ./mvnw clean install -f spring-data-jpa-quickstart
  3. works: ./mvnw clean install -f spring-data-jpa-quickstart -Dquarkus.platform.version=3.12.3

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

3.13.0

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

Maven 3.9.8

Additional information

First reported here: #41284 (comment)

I haven't had the time yet to report the issue upstream via QuarkusLikeORMUnitTestCase.

@famod famod added kind/bug Something isn't working triage/upstream area/hibernate-orm Hibernate ORM area/spring Issues relating to the Spring integration labels Aug 5, 2024
Copy link

quarkus-bot bot commented Aug 5, 2024

/cc @geoand (spring)

@famod
Copy link
Member Author

famod commented Aug 5, 2024

/cc @yrodiere

@gsmet
Copy link
Member

gsmet commented Aug 7, 2024

@beikov @dreab8 any chance one of you could have a look at this one so that we understand why the behavior changed?

@dreab8
Copy link
Contributor

dreab8 commented Aug 7, 2024

@gsmet looking into it

@dreab8
Copy link
Contributor

dreab8 commented Aug 7, 2024

The update query was triggering a flush only because the default flush mode was set to ALWAYS but this has been changed by #41284 so Hibernate executes a flush after a query only when the result of the query may be affected by dirty state not synchronised with the db flush on JPQL/HQL query and in this case

var fruit = fruitRepository.findByName("Apple").orElseThrow(NotFoundException::new);
var logEntry = new FruitLogEntry(fruit, LocalDateTime.now() , "foo");
fruitLogEntryRepository.save(logEntry);
fruitRepository.updateLastLogEntry(fruit.getId(), logEntry);

the executed query update Fruit set lastLogEntry = :logEntry where id = :fruitId affects only the Fruit entity that is not dirty, so no flush is expected to happen.

@dreab8
Copy link
Contributor

dreab8 commented Aug 7, 2024

I noticed that spring offers this annotation @Modifying(flushAutomatically = true) but is seems not working as it's not working
@QueryHints({@QueryHint( name = "org.hibernate.flushMode", value ="ALWAYS" )}) while

var fruit = em.createQuery( "select f from Fruit f where f.name = :name", Fruit.class )
				.setParameter( "name", "Apple" ).getSingleResult();

var logEntry = new FruitLogEntry(fruit, LocalDateTime.now() , "foo");
em.persist(logEntry);
em.createQuery( "update Fruit set lastLogEntry = :logEntry where id = :fruitId" )
	.setHint( AvailableHints.HINT_FLUSH_MODE, "ALWAYS" )
	.setParameter( "logEntry", logEntry )
	.setParameter( "fruitId", fruit.getId() ).executeUpdate();

works

@famod
Copy link
Member Author

famod commented Aug 7, 2024

@dreab8 thanks for your analysis.

So basically you are saying that Hibernate ORM is not meant to detect this case, hence no bug in Hibernate, right?

@dreab8
Copy link
Contributor

dreab8 commented Aug 7, 2024

yes this is not a bug, hibernate flushes the session only if fruit entity is dirty (e.g fruit.setColor( "Green" ); ) when the query is executed.

@famod
Copy link
Member Author

famod commented Aug 7, 2024

I noticed that spring offers this annotation @Modifying(flushAutomatically = true) but is seems not working

I think I know why. I'll try to come up with a PR soonish.

@famod
Copy link
Member Author

famod commented Aug 7, 2024

So, I'm under the impression that this issue is to be closed as won't fix. Any objections?

@yrodiere
Copy link
Member

Agreed, since the problem was not quite the one described in the issue. Thanks for the fix.

@yrodiere yrodiere closed this as not planned Won't fix, can't repro, duplicate, stale Aug 12, 2024
@famod
Copy link
Member Author

famod commented Aug 12, 2024

Hi @yrodiere,

Agreed, since the problem was not quite the one described in the issue. Thanks for the fix.

To avoid confusion: I've never used flushAutomatically and even after the fix (#42376 / Quarkus 3.13.2) I won't be using it for the cases that lead to this issue.
When @dreab8 mentioned flushAutomatically (and reporting it as broken) I was just interested in why it didn't work and had a quick look.

So what I'm trying to say is that with Quarkus 3.13.0 I need an explicit flush, in one way or the other.
And as @dreab8 described, Hibernate is not supposed to detect the need for a flush in the cases I described.

@yrodiere
Copy link
Member

yrodiere commented Aug 12, 2024

Thanks for the clarification.

So your patch addresses flushAutomatically, which you don't personally use but were kind enough to fix anyway (thanks!).

Can you confirm you don't need any other change? I.e. that migration notes sort of address your problem, which was only that your application was relying on flushes that were never meant to happen?

@famod
Copy link
Member Author

famod commented Aug 12, 2024

Can you confirm you don't need any other change? I.e. that migration notes sort of address your problem, which was only that your application was relying on flushes that were never meant to happen?

Of course it would be nice if Hibernate would detect that special case automatically, but I'm ok with the explicit flushes. :-)

@yrodiere
Copy link
Member

yrodiere commented Aug 12, 2024

Right, by "other changes" I meant "changes in Quarkus or bugfixes in Hibernate".

Filing feature requests to Hibernate ORM is still an option, of course :)

Anyway, thank you for confirming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hibernate-orm Hibernate ORM area/spring Issues relating to the Spring integration kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants