#310 - Fix bind marker reuse when expanding collection arguments using named parameters.

We now correctly reuse bind markers for named parameter substitution when using collection arguments to create dynamic argument lists. Previously, we allocated a new bind marker for each item in the collection which left the parameters intended for reuse unbound.

Author: mp911de

Diff for: src/main/java/org/springframework/data/r2dbc/core/NamedParameterUtils.java

@@ -31,7 +31,6 @@
 import org.springframework.data.r2dbc.dialect.BindMarkersFactory;
 import org.springframework.data.r2dbc.dialect.BindTarget;
 import org.springframework.util.Assert;
-import org.springframework.util.CollectionUtils;
 
 /**
  * Helper methods for named parameter parsing.
@@ -287,6 +286,7 @@ public static PreparedOperation<String> substituteNamedParameters(ParsedSql pars
 
 					Iterator<?> entryIter = ((Collection<?>) value).iterator();
 					int k = 0;
+					int counter = 0;
 					while (entryIter.hasNext()) {
 						if (k > 0) {
 							actualSql.append(", ");
@@ -300,11 +300,13 @@ public static PreparedOperation<String> substituteNamedParameters(ParsedSql pars
 								if (m > 0) {
 									actualSql.append(", ");
 								}
-								actualSql.append(marker.addPlaceholder());
+								actualSql.append(marker.getPlaceholder(counter));
+								counter++;
 							}
 							actualSql.append(')');
 						} else {
-							actualSql.append(marker.addPlaceholder());
+							actualSql.append(marker.getPlaceholder(counter));
+							counter++;
 						}
 
 					}
@@ -459,12 +461,16 @@ String addPlaceholder() {
 			}
 
 			String getPlaceholder() {
+				return getPlaceholder(0);
+			}
+
+			String getPlaceholder(int counter) {
 
-				if (this.placeholders.isEmpty()) {
-					return addPlaceholder();
+				while (counter + 1 > this.placeholders.size()) {
+					addPlaceholder();
 				}
 
-				return this.placeholders.get(0).getPlaceholder();
+				return this.placeholders.get(counter).getPlaceholder();
 			}
 		}
 	}

Diff for: src/test/java/org/springframework/data/r2dbc/core/NamedParameterUtilsUnitTests.java

@@ -31,6 +31,8 @@
 import org.springframework.data.r2dbc.dialect.PostgresDialect;
 import org.springframework.data.r2dbc.dialect.SqlServerDialect;
 import org.springframework.data.r2dbc.mapping.SettableValue;
+import org.springframework.util.LinkedMultiValueMap;
+import org.springframework.util.MultiValueMap;
 
 /**
  * Unit tests for {@link NamedParameterUtils}.
@@ -335,6 +337,52 @@ public void bindNull(int index, Class<?> type) {
 		});
 	}
 
+	@Test // gh-310
+	public void multipleEqualCollectionParameterReferencesBindsValueOnce() {
+
+		String sql = "SELECT * FROM person where name IN (:ids) or lastname IN (:ids)";
+
+		BindMarkersFactory factory = BindMarkersFactory.indexed("$", 0);
+
+		MultiValueMap<Integer, Object> bindings = new LinkedMultiValueMap<>();
+
+		PreparedOperation<String> operation = NamedParameterUtils.substituteNamedParameters(sql, factory,
+				new MapBindParameterSource(
+						Collections.singletonMap("ids", SettableValue.from(Arrays.asList("foo", "bar", "baz")))));
+
+		assertThat(operation.toQuery())
+				.isEqualTo("SELECT * FROM person where name IN ($0, $1, $2) or lastname IN ($0, $1, $2)");
+
+		operation.bindTo(new BindTarget() {
+			@Override
+			public void bind(String identifier, Object value) {
+				throw new UnsupportedOperationException();
+			}
+
+			@Override
+			public void bind(int index, Object value) {
+				assertThat(index).isIn(0, 1, 2);
+				assertThat(value).isIn("foo", "bar", "baz");
+
+				bindings.add(index, value);
+			}
+
+			@Override
+			public void bindNull(String identifier, Class<?> type) {
+				throw new UnsupportedOperationException();
+			}
+
+			@Override
+			public void bindNull(int index, Class<?> type) {
+				throw new UnsupportedOperationException();
+			}
+		});
+
+		assertThat(bindings).containsEntry(0, Collections.singletonList("foo")) //
+				.containsEntry(1, Collections.singletonList("bar")) //
+				.containsEntry(2, Collections.singletonList("baz"));
+	}
+
 	@Test // gh-138
 	public void multipleEqualParameterReferencesForAnonymousMarkersBindsValueMultipleTimes() {