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

Improve strategy for persisting referenced entities [DATAJDBC-210] #437

Open
spring-projects-issues opened this issue Apr 23, 2018 · 5 comments
Assignees
Labels
in: jdbc Spring Data JDBC type: task A general task

Comments

@spring-projects-issues
Copy link

spring-projects-issues commented Apr 23, 2018

This is an Epic gathering various approaches to improve the writing behaviour of Spring Data JDBC.

If we have an Aggregate Root A referencing entities B on update the following SQL statements get executed:

  • delete all previously referenced Bs
  • update A
  • insert all now referenced Bs

We should improve this in multiple ways:

  1. Use upserts on Bs and only delete those not longer present
  2. Use batched statements for inserts (and updates) This is implemented.
  3. Allow detection of changed Bs. There are various possibilities how to do this
    • have an annotated attribute carrying this information
    • wrap everything in proxies to track this (I really don't like this because I think it's opening a can of worms)
    • one way or the other compare with the version currently in the database. Not sure how to do that, possibly with a JDBC specific API

Also, performancetests would be nice and probably a good first step.

Create separate issues to work on any of these. Use this issue to discuss these and other strategies to improve persisting performance.

Jens Schauder opened this ticket as DATAJDBC-210

@spring-projects-issues
Copy link
Author

Anton Reshetnikov commented

+1 for

  1. Use batched statements for inserts (and updates)\

I was going to create separate issue for batched jdbc operations in Spring DATA JDBC, but found this issue

@spring-projects-issues
Copy link
Author

Jens Schauder commented

Anton Reshetnikov batch inserts will have to wait until we have proper support for Id generation strategies, because the current one (via SERIAL/AUTOINCREMENT in the database) does not work with batches. See spring-projects/spring-framework#6530

@Husan
Copy link

Husan commented Nov 16, 2022

+1 for 1 st and 3.1

  1. Use upserts on Bs and only delete those not longer present
    3.1. have an annotated attribute carrying this information

I think that the 1 st option must be default behavior or may be switched on by some annotation on Aggregate Root.
2 nd option may be must be behavior when developer define which row must be updated which insert and which must be deleted. May be similar to isNew function must be changeOperation(or changeType) function in child aggregate which must be implemented from some interfays(like Persistable) and return values as "INSERT", "UPDATE", "DELETE", "NONE"(means non operation must be performent on reference collection element).
Example:

class AgregateRoot implements Persistable{
     .........
     @UpdateStrategy("UPSERT")
     Set<ChileAggregate> aggregate;
}

class ChildAggregate implements SomeInterface{
       .......
      public changeType(){
              return "UPDATE";
      }
}

@kap199297
Copy link

Hi @schauder, I was checking the code related to this issue. I was thinking of the below ideas.

Idea#1. Most of the database (supporting Spring boot JDBC) supports merge statements. For MySQL or MariaDB, we can go for ON DUPLICATE KEY UPDATE. I am not sure if it is a good idea to write a database-specific query.

Idea#2. Could we re-iterate the same root aggregate solution for all leaf entities?

For example: Aggregate Root A references Entities B and Entity B references Entities C.

JdbcAggregateTemplate

...
private <T> Function<T, RootAggregateChange<T>> changeCreatorSelectorForSave(T instance) {

		return context.getRequiredPersistentEntity(instance.getClass()).isNew(instance)
				? entity -> createInsertChange(prepareVersionForInsert(entity))
				: entity -> createUpdateChange(prepareVersionForUpdate(entity));
	}
...

WritingContext.java

void save() {
  if (isNew(root)) {
	  setRootAction(new DbAction.InsertRoot<>(root, rootIdValueSource));
	  insertReferenced().forEach(aggregateChange::addAction);
  } else {
  
	  setRootAction(new DbAction.UpdateRoot<>(root, previousVersion));
	  deleteReferenced().forEach(aggregateChange::addAction);
	  insertReferenced().forEach(aggregateChange::addAction);
  }
 }
  1. AggregateRoot A plan based on isNew() -> Only for root Aggregate
  2. Get the referencing of the entity list (List) using WritingContext.java
  3. Iterate and Use isNew(Step1) logic again for Entity and repeat the same untill we have zero leafEntity.
  4. For deletion of not present, we can go for the solution of a query mentioned in Only delete those referenced entities that are no longer present in an aggregate. Use an upsert for all others [DATAJDBC-224] #450.
    DELETE FROM <CHILD TABLE>
     WHERE
     PARENT_ID = :AGGREGATE_ID AND
     CHILD_ID NOT in (:id1, :id2, ......:id_last)

This is just my thinking as a newbie, you know better since you have written this code. ⁠😃

@schauder
Copy link
Contributor

you know better since you have written this code.
I'm not sure that is always true.

isNew doesn't work for entities within an aggregate, because they don't have a version and might not have an id. For most entities the primary key is just the reference to the parent entity plus a map key or a list index.

I am not sure if it is a good idea to write a database-specific query.

If only there were a standard that all the RDBMS adhere to... Seriously: That's what Dialect is for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: jdbc Spring Data JDBC type: task A general task
Projects
None yet
Development

No branches or pull requests

4 participants