#23 - Address review comments.

Original pull request: #47.

Author: mp911de

src/main/asciidoc/reference/r2dbc-repositories.adoc

@@ -120,4 +120,4 @@ The annotated query uses native bind markers, which are Postgres bind markers in
 
 NOTE: R2DBC repositories do not support query derivation.
 
-NOTE: R2DBC repositories bind parameters to placeholders by index.
+NOTE: R2DBC repositories bind internally parameters to placeholders via `Statement.bind(…)` by index.

src/main/asciidoc/reference/r2dbc.adoc

@@ -459,13 +459,16 @@ db.execute()
 
 .R2DBC Native Bind Markers
 ****
-R2DBC uses database-native bind markers that depend on the actual database.
-If you are familiar with JDBC, then you're also familiar with `?` (question mark) bind markers.
-JDBC drivers translate question mark bind markers to database-native markers as part of statement execution.
+R2DBC uses database-native bind markers that depend on the actual database vendor.
+As an example, Postgres uses indexed markers such as `$1`, `$2`, `$n`.
+Another example is SQL Server that uses named bind markers prefixed with `@` (at).
 
-Postgres uses indexed markers (`$1`, `$2`), SQL Server uses named bind markers prefixed with `@` as its native bind marker syntax.
-Spring Data R2DBC leverages `Dialect` implementations to expand named parameters to native bind markers at the time of query execution which gives you a certain degree of query portability across various database vendors.
-You can still use native bind markers if you prefer to do so.
+This is different from JDBC which requires `?` (question mark) as bind markers.
+In JDBC, the actual drivers translate question mark bind markers to database-native markers as part of their statement execution.
+
+Spring Data R2DBC allows you to use native bind markers or named bind markers with the `:name` syntax.
+
+Named parameter support leverages ``Dialect``s  to expand named parameters to native bind markers at the time of query execution which gives you a certain degree of query portability across various database vendors.
 ****
 
 The query-preprocessor unrolls named `Collection` parameters into a series of bind markers to remove the need of dynamic query creation based on the number of arguments.

src/main/java/org/springframework/data/r2dbc/function/DatabaseClient.java

@@ -112,12 +112,12 @@ interface Builder {
 		/**
 		 * Configures {@link NamedParameterExpander}.
 		 *
-		 * @param namedParameters must not be {@literal null}.
+		 * @param expander must not be {@literal null}.
 		 * @return {@code this} {@link Builder}.
 		 * @see NamedParameterExpander#enabled()
 		 * @see NamedParameterExpander#disabled()
 		 */
-		Builder namedParameters(NamedParameterExpander namedParameters);
+		Builder namedParameters(NamedParameterExpander expander);
 
 		/**
 		 * Configures a {@link Consumer} to configure this builder.

src/main/java/org/springframework/data/r2dbc/function/DefaultDatabaseClient.java

@@ -254,21 +254,30 @@ protected DefaultGenericExecuteSpec createGenericExecuteSpec(Supplier<String> sq
 	private static void doBind(Statement<?> statement, Map<String, SettableValue> byName,
 			Map<Integer, SettableValue> byIndex) {
 
-		byIndex.forEach((i, o) -> {
+		bindByIndex(statement, byIndex);
+		bindByName(statement, byName);
+	}
+
+	private static void bindByName(Statement<?> statement, Map<String, SettableValue> byName) {
+
+		byName.forEach((name, o) -> {
 
 			if (o.getValue() != null) {
-				statement.bind(i.intValue(), o.getValue());
+				statement.bind(name, o.getValue());
 			} else {
-				statement.bindNull(i.intValue(), o.getType());
+				statement.bindNull(name, o.getType());
 			}
 		});
+	}
 
-		byName.forEach((name, o) -> {
+	private static void bindByIndex(Statement<?> statement, Map<Integer, SettableValue> byIndex) {
+
+		byIndex.forEach((i, o) -> {
 
 			if (o.getValue() != null) {
-				statement.bind(name, o.getValue());
+				statement.bind(i.intValue(), o.getValue());
 			} else {
-				statement.bindNull(name, o.getType());
+				statement.bindNull(i.intValue(), o.getType());
 			}
 		});
 	}
@@ -340,7 +349,7 @@ <T> FetchSpec<T> exchange(String sql, BiFunction<Row, RowMetadata, T> mappingFun
 					}
 				});
 
-				doBind(statement, Collections.emptyMap(), byIndex);
+				bindByIndex(statement, byIndex);
 
 				return statement;
 			};

src/main/java/org/springframework/data/r2dbc/function/DefaultDatabaseClientBuilder.java

@@ -96,11 +96,11 @@ public Builder dataAccessStrategy(ReactiveDataAccessStrategy accessStrategy) {
 	 * @see org.springframework.data.r2dbc.function.DatabaseClient.Builder#namedParameters(org.springframework.data.r2dbc.function.NamedParameterExpander)
 	 */
 	@Override
-	public Builder namedParameters(NamedParameterExpander namedParameters) {
+	public Builder namedParameters(NamedParameterExpander expander) {
 
-		Assert.notNull(namedParameters, "NamedParameterExpander must not be null!");
+		Assert.notNull(expander, "NamedParameterExpander must not be null!");
 
-		this.namedParameters = namedParameters;
+		this.namedParameters = expander;
 		return this;
 	}
 

src/main/java/org/springframework/data/r2dbc/function/DefaultTransactionalDatabaseClientBuilder.java

@@ -73,8 +73,8 @@ public TransactionalDatabaseClient.Builder dataAccessStrategy(ReactiveDataAccess
 	 * @see org.springframework.data.r2dbc.function.DefaultDatabaseClientBuilder#dataAccessStrategy(org.springframework.data.r2dbc.function.NamedParameterSupport)
 	 */
 	@Override
-	public TransactionalDatabaseClient.Builder namedParameters(NamedParameterExpander namedParameters) {
-		super.namedParameters(namedParameters);
+	public TransactionalDatabaseClient.Builder namedParameters(NamedParameterExpander expander) {
+		super.namedParameters(expander);
 		return this;
 	}
 

src/main/java/org/springframework/data/r2dbc/function/NamedParameterUtils.java

@@ -162,16 +162,6 @@ public static ParsedSql parseSqlStatement(String sql) {
 						continue;
 					}
 				}
-				if (c == '?') {
-					int j = i + 1;
-					if (j < statement.length && (statement[j] == '?' || statement[j] == '|' || statement[j] == '&')) {
-						// Postgres-style "??", "?|", "?&" operator should be skipped
-						i = i + 2;
-						continue;
-					}
-					unnamedParameterCount++;
-					totalParameterCount++;
-				}
 			}
 			i++;
 		}

src/main/java/org/springframework/data/r2dbc/function/TransactionalDatabaseClient.java

@@ -192,12 +192,12 @@ interface Builder extends DatabaseClient.Builder {
 		/**
 		 * Configures {@link NamedParameterExpander}.
 		 *
-		 * @param namedParameters must not be {@literal null}.
+		 * @param expander must not be {@literal null}.
 		 * @return {@code this} {@link Builder}.
 		 * @see NamedParameterExpander#enabled()
 		 * @see NamedParameterExpander#disabled()
 		 */
-		Builder namedParameters(NamedParameterExpander namedParameters);
+		Builder namedParameters(NamedParameterExpander expander);
 
 		/**
 		 * Configures a {@link Consumer} to configure this builder.

src/test/java/org/springframework/data/r2dbc/function/NamedParameterUtilsUnitTests.java

@@ -50,7 +50,7 @@ public void shouldParseSql() {
 		String sql2 = "xxx &a yyyy ? zzzzz";
 		ParsedSql psql2 = NamedParameterUtils.parseSqlStatement(sql2);
 		assertThat(psql2.getParameterNames()).containsExactly("a");
-		assertThat(psql2.getTotalParameterCount()).isEqualTo(2);
+		assertThat(psql2.getTotalParameterCount()).isEqualTo(1);
 		assertThat(psql2.getNamedParameterCount()).isEqualTo(1);
 
 		String sql3 = "xxx &ä+:ö" + '\t' + ":ü%10 yyyy ? zzzzz";