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

Default ID value on new record is inserted when optimistic locking is enabled #557

Closed
jnfeinstein opened this issue Mar 14, 2021 · 3 comments
Assignees
Labels
type: bug A general bug

Comments

@jnfeinstein
Copy link

I believe the issue occurs here. The version is incremented prior to deciding if the ID should be included. This results in the record being considered not new for purposes of including the ID.

CREATE TABLE account (
    id SERIAL PRIMARY KEY,
    name TEXT NOT NULL,
    version INTEGER NOT NULL
);
@Table
data class Account(
    @Id var id: Long = 0L,
    var name: String = "",
    @Version val version: Long = 0L,
)
org.springframework.dao.DataIntegrityViolationException: executeMany; SQL [INSERT INTO account (id, name, version) VALUES ($1, $2, $3)]; duplicate key value violates unique constraint "account_pkey"; nested exception is io.r2dbc.postgresql.ExceptionFactory$PostgresqlDataIntegrityViolationException: [23505] duplicate key value violates unique constraint "account_pkey"
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 14, 2021
@jnfeinstein jnfeinstein changed the title Default ID value is inserted when optimistic locking is enabled on new record Default ID value on new record is inserted when optimistic locking is enabled Mar 14, 2021
@mp911de mp911de self-assigned this Mar 15, 2021
@mp911de
Copy link
Member

mp911de commented Mar 16, 2021

This works as expected. We include all non-null fields in the INSERT statement when inserting an object. In your case, the @Id field is declared as non-null long hence its value cannot be null. If the identifier field is null then it won't be included in the INSERT statement. The reference documentation is pretty clear about this behavior.

@mp911de mp911de added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 16, 2021
@jnfeinstein
Copy link
Author

Please see here:

Spring Data R2DBC does not attempt to insert values of identifier columns when the entity is new and the identifier value defaults to its initial value. That is 0 for primitive types and null if the identifier property uses a numeric wrapper type such as Long.

I've confirmed that the behavior is correct when @Version is not present.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 16, 2021
@mp911de
Copy link
Member

mp911de commented Mar 16, 2021

Thanks for the hint and revisiting this topic, it qualifies as an actual bug. I think it requires a bit of investigation on how we want to approach the issue. We will need to post-process OutboundRow in some sense. Probably removing the Id from OutboundRow if its value maps to a primitive default seems the better way to go. We also should link the ID generation section with the section regarding inserts as these topics are related to each other.

@mp911de mp911de added type: bug A general bug and removed status: feedback-provided Feedback has been provided labels Mar 16, 2021
@mp911de mp911de added this to the 1.2.6 (2020.0.6) milestone Mar 17, 2021
mp911de added a commit that referenced this issue Mar 17, 2021
mp911de added a commit that referenced this issue Mar 17, 2021
…tyTemplate.

We now check in R2dbcEntityTemplate whether we need to skip the Id value if its value is null or a primitive using its default. Previously, the check was located in the converter. Converting an entity afteri ncrementing the version of a versioned entity would write the Id value as the entity was no longer considered to be new.

Closes #557.
mp911de added a commit that referenced this issue Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants