micronaut-data icon indicating copy to clipboard operation
micronaut-data copied to clipboard

Generated values (@GeneratedValue) are not bound to the entity returned by save()

Open davidsonnabend opened this issue 1 year ago • 15 comments

Expected Behavior

Having an entity with properties annotated with @field:GeneratedValue should bind database generated values when using save() of a JdbcRepository (Dialect: POSTGRES).

Actual Behaviour

Having the following Event entity where the creation date (createdAt) must be generated by the database:

@Serdeable
@MappedEntity
data class Event(
    @field:Id
    @field:GeneratedValue
    val id: Long? = null,

    @field:Version
    val version: Long? = null,

    @field:GeneratedValue
    val createdAt: LocalDateTime? = null,

    val property: String,

    @Relation(value = Relation.Kind.MANY_TO_ONE)
    val child: Child? = null,
)

When saving a new Event using a JdbcRepository the returned instance has no value for createdAt. Using saveReturning() instead of save() the generated createdAt value is bound, but the child property is null.

I checked the query log of the PostgreSQL DB to see which queries are executed.

When using save() the following insert is executed:

INSERT INTO "event" ("version","property","child_id") VALUES ($1,$2,$3) RETURNING *

When using saveReturning() the query looks like this:

INSERT INTO "event" ("version","property","child_id") VALUES ($1,$2,$3) RETURNING "created_at","version","property","child_id","id"

From a database point of view both queries doing the same - all columns of the table are returned. Using save() just the generated id is bound to the returned entity.

Steps To Reproduce

  1. Clone example application
  2. Run test using ./gradlew test
  3. Test will fail because createdAt is null within the event returned by save()

Environment Information

  • Operating System: Ubuntu 23.10
  • JDK Version: 21.0.1-zulu

Example Application

https://github.com/davidsonnabend/micronaut-data-bind-generated-values

Version

4.2.2

davidsonnabend avatar Dec 18 '23 14:12 davidsonnabend

After spending some time trying to understand where the RETURNING * is coming from when using the simple save() method I found an answer by debugging the example application.

RETURNING * is added by org.postgresql.jdbc.PgConnection - so it is not part of the Insert query created by micronaut data.

When using save(), micronaut data explicitly bind the generated identity using getGeneratedIdentity() and updateEntityId(). The result of the RETURNING * statement added by PgConnection is not used.

On the other side saveReturning() "simply" creates an entity based of the result (entity = result.iterator().next()) - so all other (join-) properties will be null.

What is the intended behavior of micronaut data in that case?

Is it possible to configure the repository to return a "complete" entity or should executeReturning() merge the entity with the result instead of overriding it?

davidsonnabend avatar Dec 19 '23 14:12 davidsonnabend

What do you mean by complete entity? With joins? Not sure if that is supported

dstepanov avatar Dec 19 '23 15:12 dstepanov

I think in your report the generated field is empty, it might be omitted by mistake in the return query design

dstepanov avatar Dec 19 '23 15:12 dstepanov

In that repository, findById would not return child field either because it does not have @Join("child") which is needed for that scenario. The same way works saveReturning and it can't return child because there is no Join annotation defined on the repo. When I tried to add @Join("child") then getting error because child fields are not returned in the INSERT...RETURNING... statement.

radovanradic avatar Dec 19 '23 15:12 radovanradic

What do you mean by complete entity? With joins? Not sure if that is supported

Yes - with joins. As the result would look like when using the save() method without @GeneratedValue for createdAt.

In that repository, findById would not return child field either because it does not have @Join("child") which is needed for that scenario. The same way works saveReturning and it can't return child because there is no Join annotation defined on the repo.

Just to ensure that I understand it correctly: saveReturning works similar to findById - it is not using findById internally, right?

What would be the intended behavior in that specific case:

  1. save() fetches and bind values of @GeneratedValue annotated columns (next to identities) correctly
  2. saveReturning() returns relation properties the same way as save() is doing it
  3. No support at all and the service layer needs to handle this case by fetching the entity "manually" after save() or saveReturning() was executed

davidsonnabend avatar Dec 19 '23 20:12 davidsonnabend

It is reusing the same logic to fetch the results of INSERT...RETURNING Just to mention, currently save() is also not fetching data, it just sets potentially generated id values to the entity object after save.

radovanradic avatar Dec 20 '23 08:12 radovanradic

I would like to help fixing this issue. For me it makes most sense to bind all values of @GeneratedValue annotated columns after saving the entity the same way as it is done with generated ids.

What do you think?

davidsonnabend avatar Jan 08 '24 10:01 davidsonnabend

What if somebody modifies one of the values and wants to see it in the entity? I think this issue can be solved by coping all the relationships back to the entity if the ID matches.

dstepanov avatar Jan 08 '24 10:01 dstepanov

What if somebody modifies one of the values and wants to see it in the entity?

If the entity is saved with a specific value for a @GeneratedValue annotated property, this value will be used because the values of the entity will be fetched from the database.

I think this issue can be solved by coping all the relationships back to the entity if the ID matches.

This suggestion refers to fixing the implementation of saveReturning(), right?

davidsonnabend avatar Jan 08 '24 10:01 davidsonnabend

Yes, I can imagine different strategies here.

  1. We can create a new entity from the returning response and copy the relationships after
  2. We keep the old entity / copy and update the new values (What if something is missing?)

dstepanov avatar Jan 08 '24 10:01 dstepanov

For me it looks like we should handle this in two different tickets:

  1. Fix handling of properties annotated with @GeneratedValue
  2. Keep relationships of entity returned by saveReturning

davidsonnabend avatar Jan 08 '24 11:01 davidsonnabend

What are you suggesting?

dstepanov avatar Jan 08 '24 11:01 dstepanov

I suggest to discuss & fix issues with binding of generated values in combination with save() within this ticket.

Next to this I would create another ticket like "Keep relationships of entity returned by saveReturning".

This way we can avoid confusion and find the most suitable solution for each case.

Are you ok with this?

davidsonnabend avatar Jan 09 '24 06:01 davidsonnabend

Sorry, I missed the first problem. For createdAt, you should use @DateCreated, which will fill the value before it's inserted. Essentially it's not that easy to get generated values when an entry is insered, some JDBC drivers support only the identity to be returned.

dstepanov avatar Jan 09 '24 18:01 dstepanov

For createdAt, you should use @DateCreated, which will fill the value before it's inserted.

I know about @DateCreated. In our specific business case the createdAt date is (and must be) created by the datebase. Thats the reason I created this issue.

Then I will create another issue to handle relationships for saveReturning and leave this issue open so that the team could discuss whether to find a solution or don't support binding generated values.

davidsonnabend avatar Jan 10 '24 07:01 davidsonnabend