SQL supplier in R2DBC DatabaseClient is eagerly invoked

[EnricSala]

Affects spring-r2dbc 5.3.23

---

The R2DBC DatabaseClient offers two methods to provide the SQL query:

1. GenericExecuteSpec sql(String sql);
2. GenericExecuteSpec sql(Supplier<String> sqlSupplier);

We were expecting that the latter would evaluate the sqlSupplier lazily, i.e. deferred until subscription. For example we were attempting to use this supplier for large or dynamic queries which we would like to build only in cases where the Publisher is actually subscribed.

However, it seems that the sqlSupplier is being invoked eagerly on assembly. For example this test doesn't pass:

@Test
void sqlSupplierInvocationIsDeferredUntilSubscription() {
  DatabaseClient client = ...;
  Clock clock = ...;

  AtomicBoolean invoked = new AtomicBoolean(false);

  // Assemble a publisher, but don't subscribe yet
  Mono<Void> operation =
      client.sql(
              () -> {
                invoked.set(true);

                return String.format(
                    "SELECT * FROM test WHERE ts < '%s'",
                    clock.instant());
              })
          .map(r -> r.get("ts", Instant.class))
          .all()
          .then();

  // Unexpectedly, the SQL supplier has been invoked
  assertThat(invoked).isFalse();

  // Would expect SQL is deferred until subscription
  operation.block();
  assertThat(invoked).isTrue();
}

The issue appears to happen in DefaultGenericExecuteSpec#execute, which eagerly invokes the sqlSupplier to hold a reference and pass it to several steps of the processing, see:

spring-framework/spring-r2dbc/src/main/java/org/springframework/r2dbc/core/DefaultDatabaseClient.java

Lines 325 to 329 in 5ea8ba1

	private <T> FetchSpec<T> execute(Supplier<String> sqlSupplier, BiFunction<Row, RowMetadata, T> mappingFunction) {
	String sql = getRequiredSql(sqlSupplier);
	Function<Connection, Statement> statementFunction = connection -> {
	if (logger.isDebugEnabled()) {
	logger.debug("Executing SQL statement [" + sql + "]");

As a workaround, we are currently wrapping the chain in a Mono.defer(() -> ...), but this is inconvenient and easy to forget.