Skip to content

SimpleR2dbcRepository returns error if call save on an unchanged entity #253

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

Closed
maccamlcQ opened this issue Dec 9, 2019 · 3 comments
Closed
Labels
for: external-project For an external project and not something we can fix

Comments

@maccamlcQ
Copy link

maccamlcQ commented Dec 9, 2019

Spring Boot 2.2.1.RELEASE
Spring Data R2DBC 1.0.0.RELEASE
R2DBC-(Mysql/SPI) 0.8.0.RELEASE

If I call #save on an unchanged entity, then currently, I am getting an error: Failed to update table [%s]. Row with Id [%s] does not exist.

I am using the r2dbc-mysql library.

If I alter my path to ensure the item is always actually updated, before save gets called, then it succeeds as expected.

As this is a nested entity, it is being saved as a side-effect of the primary entity.

Is this a bug, as it worked previously for me, or do I need to update the code flow to perform a read and check if anything has changed before I call save?

@mp911de
Copy link
Member

mp911de commented Dec 9, 2019

Thanks for report. The mentioned exception is signaled when a save(…) determines that the object is not new and if the UPDATE statement executes without updating a row. We added this behavior with #232 if an UPDATE returns zero rows were affected by a change to avoid updates that do not exist on the database (e.g. object with an assigned Id is considered not new which is a common bug when someone assumes the row should be inserted).

We haven't considered that MySQL has slightly different behavior by default. The MySQL docs say:

For UPDATE statements, the affected-rows value by default is the number of rows actually changed. If you specify the CLIENT_FOUND_ROWS flag to mysql_real_connect() when connecting to mysqld, the affected-rows value is the number of rows “found”; that is, matched by the WHERE clause.

/cc @schauder @mirromutth

I'm not sure whether it would make sense to enable the CLIENT_FOUND_ROWS by default for R2DBC MySQL. We have learned also in other environments (e.g. H2) that some database defaults cause a higher obstacle to application portability. R2DBC drivers can override some defaults to achieve a greater level of portability. Happy to discuss options.

@mp911de mp911de added for: team-attention An issue we need to discuss as a team to make progress status: waiting-for-feedback We need additional information before we can continue labels Dec 9, 2019
@mirromutth
Copy link
Contributor

mirromutth commented Dec 9, 2019

Yes, for R2DBC MySQL, if x row(s) found/touched but only y row(s) was really changed, it will return the y for the count of affected rows instead of found/touched rows.

See https://github.com/mirromutth/r2dbc-mysql/blob/v0.8.0.RELEASE/src/main/java/dev/miku/r2dbc/mysql/constant/Capabilities.java#L29 , it is commented line, so this flag is disabled now.

It should be enabled by default if this will affect Spring Data R2DBC behavior. I will change it soon.

@mp911de mp911de added for: external-project For an external project and not something we can fix and removed for: team-attention An issue we need to discuss as a team to make progress status: waiting-for-feedback We need additional information before we can continue labels Jan 22, 2020
@mp911de
Copy link
Member

mp911de commented Jan 22, 2020

The issue will be handled in the driver hence closing this ticket.

@mp911de mp911de closed this as completed Jan 22, 2020
oshai added a commit to jasync-sql/jasync-sql that referenced this issue Aug 23, 2021
This commit adds system property jasync.mysql.CLIENT_FOUND_ROWS that
when set will create connections with the flag on.

CLIENT_FOUND_ROWS tells the mysql server to send back all rows in where
clause and not just the ones that were actually updated. See details in:
https://dev.mysql.com/doc/refman/8.0/en/information-functions.html
https://dev.mysql.com/doc/internals/en/capability-flags.html

This makes more sense in some scenarios. For example:
spring-projects/spring-data-r2dbc#253

Also renamed SSLRequestMessage to CapabilityRequestMessage to better
reflect the data.
oshai added a commit to jasync-sql/jasync-sql that referenced this issue Aug 24, 2021
This commit adds system property jasync.mysql.CLIENT_FOUND_ROWS that
when set will create connections with the flag on.

CLIENT_FOUND_ROWS tells the mysql server to send back all rows in where
clause and not just the ones that were actually updated. See details in:
https://dev.mysql.com/doc/refman/8.0/en/information-functions.html
https://dev.mysql.com/doc/internals/en/capability-flags.html

This makes more sense in some scenarios. For example:
spring-projects/spring-data-r2dbc#253

Other changes:
* Rename SSLRequestMessage to CapabilityRequestMessage to better
reflect the data.
* Set r2dbc to default CLIENT_FOUND_ROWS=true
* Add MysqlConnectionFactoryProvider.CLIENT_FOUND_ROWS property to
set/unset property.
* Add tests to affected rows on UpdateQuerySpec.kt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix
Projects
None yet
Development

No branches or pull requests

3 participants