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

Delete event not flushed before Save in JpaRepository [DATAJPA-727] #1100

Closed
spring-projects-issues opened this issue May 22, 2015 · 18 comments
Closed
Assignees
Labels
in: core status: declined type: bug

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented May 22, 2015

Tom Matthews opened DATAJPA-727 and commented

I've come across an issue when using a @UniqueConstraint and deleting/inserting entities using JpaRepository within a transaction. If I have a model object with a unique constraint on a property, then within a transaction I delete the original object and save another with the same property value, a constraint violation is thrown. When looking at hibernate logs it appears that the "DELETE" is not flushed before the "INSERT" is executed.

For example I have a model object "UniqueEntity" which has a property "value" with a unique constraint defined:

@Entity
@Table(uniqueConstraints = @UniqueConstraint(columnNames = "value", name = "UK_ENTITY_DATA"))
public class UniqueEntity {
	@Id
	@GeneratedValue(strategy = GenerationType.AUTO)
	Integer id;

	@Column(name = "value")
	Integer value;

	public UniqueEntity() {
	}

	public UniqueEntity(Integer value) {
		this.value = value;
	}

	public Integer getValue() {
		return value;
	}
}

If I call the following method within a transaction a DataIntegrityViolationException is thrown:

@Transactional
public UniqueEntity addSameValue(UniqueEntity originalEntity) {
	UniqueEntity newEntity = new UniqueEntity(originalEntity.getValue());

	repository.delete(originalEntity);

	// no flush here

	newEntity = repository.save(newEntity);

	return newEntity;
}

Exception thrown is:

org.springframework.dao.DataIntegrityViolationException: could not execute statement; SQL [n/a]; constraint ["UK_ENTITY_DATA_INDEX_B ON PUBLIC.UNIQUEENTITY(VALUE) VALUES (1, 1)"; SQL statement:
insert into UniqueEntity (id, value) values (null, ?) [23505-187]]; nested exception is org.hibernate.exception.ConstraintViolationException: could not execute statement

If between the DELETE and INSERT I read from the repository, the DELETE is flushed and the INSERT completes as expected.

This was tested with both mysql and h2.

An example with 2 JUnit tests is attached. One test demonstrates the constraint violation and the other shows the flush with a read between the INSERT and DELETE. Run with "mvn test"


Affects: 1.8 GA (Fowler)

Attachments:

8 votes, 15 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 21, 2016

Mauro Molinari commented

I think I have a similar problem.
After weeks/months of investigation, it seems like I was able to make my code work correctly by replacing a MyEntityARepository.delete(EntityA) call with an EntityManager.remove(Object) on the same entity.

In my case I have two entities, EntityA and EntityB, the first one which references (with a 1-to-1 1-way relationship) the second one. I first delete an instance of EntityA, then the related instance of EntityB (immediately after). Some lines of code later, within the same transaction, however, a flush is issued and often (but not always!) happens that a foreign key constraint violation occurs because the "DELETE FROM" statement for the EntityB instance is issued, while the one for the EntityA instance is not.
I even tried to place an EntityManager.flush() call between the two deletions, but it did not solve the problem.
After replacing the MyEntityARepository.delete(EntityA) call with an EntityManager.remove(Object) call, it seems like it's working fine now.

I don't know if it's relevant, but in my case the call to MyEntityARepository.delete(EntityA) is made from a custom implementation MyEntityARepositoryImpl into which the generated default MyEntityARepository instance is injected.

I'm using EclipseLink and MySQL, with Spring DATA JPA 1.9.1

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 29, 2016

Le Thanh NGUYEN commented

Hi,

I'm facing this problem too, with delete and save in the same transaction, Spring DATA JPA 1.9.2.
I work around by separating the delete and the save in two different transactions to force the flush (but it's ugly)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 29, 2016

Mauro Molinari commented

I have to correct myself about what I've written in my previous comment. It's not true that EntityManager.remove(Object) solved my problem. In my specific case I think there's an EclipseLink bug which is causing my symptom, I'm discussing about it in EclipseLink mailing list: https://dev.eclipse.org/mhonarc/lists/eclipselink-users/msg08618.html
So, at least in my case, I don't think it's a Spring DATA JPA problem, although the fact that Tom attached a test case is interesting because then Oliver may determine that for sure or not.

As an alternative workaround which does not require two different transactions, add an EntityManager.flush() immediately before and another one immediately after the call to the delete call on the repository

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Apr 19, 2016

Valeriy Ak commented

Same problem, delete do not flush SQL and next INSERT in same transaction gets schema constraint error (aka ERROR: duplicate key value violates unique constraint). Hibernate 4.3.10 provider

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 24, 2016

James Green commented

Just spent my entire afternoon debugging an application and it turns out to be this bug. This isn't easy to spot as it requires both queries to trigger leading to developers thinking the first must have been faulty

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Jun 24, 2016

Oliver Drotbohm commented

Probably to all of your discomfort I am not even sure we can consider this is a bug on our side as it's basically the way JPA works: it's the EntityManager's responsibility to record persistence events and flush if necessary. If order is important here, it has to take care of that.

The call to CrudRepository.delete(T entity) basically triggers a EntityManager.remove(…), ….save(…) calls ….merge(…) or ….persist(…) depending on the given entity's is-new state. This means you should be able to reproduce this erroneous behavior with pure JPA interaction. I'd suggest to bring this up with the persistence provider to make sure it runs the necessary SQL commands in the right order, when it eventually flushes the operations

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Mar 12, 2018

Joel commented

I found a solution that worked for me.
Instead of using CrudRepository.delete(T entity) I used CrudRepository.deleteInBatch(Iterable<T> entity)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 9, 2018

mirabilos commented

Even flushing does not make it work: inserting a fooDao.flush(); after…

Set<Kpi> oldKeys = new HashSet<>(mainObject.getFoos().keySet());
oldKeys.removeAll(children.keySet());

for (Kpi oldKey : oldKeys) {
    fooDao.delete(mainObject.getFoos().get(oldKey));
}

… failed, as did this, for some reason:

// work around DATAJPA-727    
Stream<DerivationChild> toRemove = oldKeys.parallelStream().map(oldKey -> mainObject.getFoos().get(oldKey));
fooDao.deleteInBatch(toRemove::iterator);

In the end, the following workaround worked for me:

// work around DATAJPA-727
fooDao.deleteInBatch(oldKeys.parallelStream().map(oldKey ->
  mainObject.getFoos().get(oldKey)).collect(Collectors.toList()));

So, yes, this is likely a bug in Spring Data JPA, please fix it. Thanks!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 9, 2018

Oliver Drotbohm commented

Again, we're just forwarding calls to JPA. You're very likely able to reproduce this with plain JPA by calling em.remove(…) and em.persist(…)

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 9, 2018

mirabilos commented

I don’t know JPA (I’m not even a Java™ developer, just thrown into cold water here)… is there an extant JPA bug already, then?

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 9, 2018

Mauro Molinari commented

@mirabilos it depends on the provider you're using (EclipseLink, Hibernate?).

Regarding EclipseLink, I discussed with a committer on the mailing list a couple of years ago regarding a problem which I suspect is related, you'll find the link in one of my comments above

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 9, 2018

mirabilos commented

Ah okay. We’re using Hibernate

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented May 17, 2019

Jens Schauder commented

The original issue seems the normal JPA behaviour.

If there exists a problem beyond the normal behaviour of JPA or on of it implementations please create a new issue with a test case attached which demonstrates that it works in JPA but not in Spring Data JPA.

 

 

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Aug 18, 2020

Michael Berg commented

I'm going to get in trouble for rekindling this ancient thread but I have to say that I have the exact same problem as OP and there seems to be noone in the world who can explain what is going on.

Jens Schauder you mention that this "seems the normal JPA behavior". What do you base that on? I mean, Spring is creating a proxy for a CrudRepository which involves implementing the methods that support the derived deleteBy operations. Is it not possible at all that a bug might hide there? How can it be "normal JPA behavior" that a delete operation does not work?

 

 

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Aug 18, 2020

Oliver Drotbohm commented

Please re-read my comment here.

A JPA provider is allowed to delay operations on the database to potentially batch those up to potentially gain performance benefits. So what you see here is Hibernate delaying the delete and not performing it even in the light of a subsequent call to EntityManager.persist(…). There is nothing we can do about this as we're not controlling what the EntityManager does and we cannot consistently flush() it just to make sure it does "the right thing"™.

I've repeatedly hinted at the fact that the behavior should be reproducible on plain JPA/Hibernate by arranging the calls in the same way. If you have that, I'm pretty sure, the Hibernate team is willing to look into the issue

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Aug 18, 2020

Michael Berg commented

Oliver Drotbohm I get your point, really I do. But if things are really working as intended, then I frankly don't see how Spring derived deleteBy methods (and probably other inserting or updating statements) can ever be made to work reliably. At least not in the simple, convention driven style that I think the team was aiming for because you would need to get your hands dirty in the Hibernate substrate to make very basic things actually work. Given how much effort has been put into hiding all that away with default implementations of infrastructure beans and a minimum of required annotations in your application, this seems counter intuitive.

Absolutely everyone will end up where I am today and regardless of who's "fault" this is, many will look at Spring and conclude that it probably isn't working right. I think some reflection on this situation is warranted,. even if it is not completely fair to Spring (again - I get your point).

If no code fix in the traditional way is realistically possible then it would be extremely helpful with some implementation notes or best practice examples in the Spring documentation, because there IS no documentation that shows how to properly manage this problem with the latest Spring data/Spring boot combination. It just doesn't exist. https://docs.spring.io/spring-data/jpa/docs/current/reference/html/#reference

 

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Aug 18, 2020

Oliver Drotbohm commented

I get your point, really I do. But if things are really working as intended, then I frankly don't see how Spring derived deleteBy methods (and probably other inserting or updating statements) can ever be made to work reliably.

We seem to deal with a Hibernate bug (acknowledgement from that team outstanding). Spring Data JPA builds on top of JPA, not Hibernate. Chances are, things work different on a different JPA provider. The one exposing the problematic behavior could also be fixed. You could also manually call JpaRepository.flush() but that carries JPA specific code into repository clients which we usually recommend to avoid. Literally all of this stems from JPAs session concept and that's basically what you buy into when you choose it. We can't change the way the underlying persistence technology is defined to work.

Absolutely everyone will end up where I am today and regardless of who's "fault" this is, many will look at Spring and conclude that it probably isn't working right. I think some reflection on this situation is warranted,. even if it is not completely fair to Spring (again - I get your point).

Hence the request for a reproducer on plain JPA. If you have that, this can be brought with Hibernate and maybe even JPA to fix this or even get the spec to be more precise.

If no code fix in the traditional way is realistically possible then it would be extremely helpful with some implementation notes or best practice examples in the Spring documentation, because there IS no documentation that shows how to properly manage this problem with the latest Spring data/Spring boot combination. It just doesn't exist.

I'd argue it can't because it's not something that Spring Data JPA changes a bit over pure JPA. And if we start to document "here's something that you need to consider if you delete and recreate a JPA entity with a unique constraint in the very same transaction" our reference documentation turns into "How to not shoot yourself in the foot with JPA". The Spring Data JPA documentation is not about how to use JPA or even more: how to use Hibernate correctly. Even more so, if it's something it can (and probably should?) be fixed or improved in the latter

@Alaincruzc90
Copy link

@Alaincruzc90 Alaincruzc90 commented May 19, 2022

I found a "hacky" way to solve this "bug". It seems we can force the order of the persistence events by fetching the entity again.

this.entityManager.delete(entity);
Entity e = this.entityManager.findById(id)
otherEntity.setId(entity.getId());
this.entityManager.save(otherEntity);

Hope this example helps anyone else surfing for a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: declined type: bug
Projects
None yet
Development

No branches or pull requests

3 participants