ReactiveCrudRepository.save() doesn't work with PostgreSQL

[steven-sheehy]

ReactiveCrudRepository.save() doesn't work with PostgreSQL. Using DatabaseClient.insert() or ConnectionFactory directly does work. I've created a small demo project that reproduces this at https://github.com/steven-sheehy/r2dbc-insert. I'm not entirely sure it's related to PostgreSQL, but that's the only thing I've tried.

I'm not sure if this is the appropriate project to open this as there are a lot of projects involved.

[fjacobs]

Do you have a stacktrace?

[steven-sheehy]

There's no crash or indication of error. The save shows as successful but does not appear in the database as confirmed by a subsequent count. The above demo can confirm that by just running ./mvnw clean install and viewing the failed test.

[mp911de]

You're probably speaking about this snippet:

		Customer customer = new Customer();
		customer.setId(1L);
		customer.setFirstName("John");
		assertEquals(customer, customerRepository.save(customer).block());
		assertEquals(1L, customerRepository.count().block());

The repository derives based on ID presence/absence whether the object is new or an existing one. Providing an Id value issues an UPDATE statement. If there's no row in the database, nothing is updated. We addressed this pitfall with #232 where we emit an error if the UPDATE statement did not yield an update.

Please implement Persistable to tell the repository whether the object is a new or existing one. Ideally, use database-generated Id's instead of providing your own.

[steven-sheehy]

In my real application, we have a natural ID and it's a very large table so it's not space efficient to store a database generated ID. I can confirm that making my entity implement Persistable and tracking its inserted state does indeed work. Thanks for making me aware of this interface.

I'm glad that this scenario will at least throw an exception in the next release so it's clear to the user what they need to fix. Only thing I don't quite like is polluting the domain with transient isNew logic. Would there be any value to add a ReactiveCrudRepository.insert() method or overload save() with another param to indicate it's an update vs insert? Feel free to close this if those ideas are not appealing.

[mp911de]

Paging @schauder. We have a similar arrangement in JDBC where the repository contains also a insert method. For consistency, it could make sense to provide such a method in Spring Data R2DBC as well.

[schauder]

Almost. The insert method in Spring Data JDBC is available in the JdbcAggregateTemplate and not available in a repository interface.

But of course that allows you to easily add it to a repository as a custom method.

[mp911de]

The insert method in Spring Data JDBC is available in the JdbcAggregateTemplate and not available in a repository interface.

Sorry, I mixed it up, you're right.

For Spring Data R2DBC, one can use databaseClient.insert() to insert an object without using a presence-check.

Closing as we're already consistent with what Spring Data JDBC provides and we have a guard in place to prevent silent no-op updates.