#138 - Allow multiple usages of the same named parameter.

We now allow multiple usages of the same named parameter if the underlying database supports identifiable placeholders.

SELECT * FROM person where name = :id or lastname = :id

gets translated to

SELECT * FROM person where name = $1 or lastname = $1

Author: mp911de

src/main/java/org/springframework/data/r2dbc/core/NamedParameterExpander.java

@@ -29,6 +29,9 @@
  * This class expands SQL from named parameters to native style placeholders at execution time. It also allows for
  * expanding a {@link java.util.List} of values to the appropriate number of placeholders.
  * <p>
+ * References to the same parameter name are substituted with the same bind marker placeholder if a
+ * {@link BindMarkersFactory} uses {@link BindMarkersFactory#identifiablePlaceholders() identifiable} placeholders.
+ * <p>
  * <b>NOTE: An instance of this class is thread-safe once configured.</b>
  *
  * @author Mark Paluch

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

@@ -31,13 +31,17 @@
 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.
  * <p>
  * Only intended for internal use within Spring's Data's R2DBC framework. Partially extracted from Spring's JDBC named
  * parameter support.
  * <p>
+ * References to the same parameter name are substituted with the same bind marker placeholder if a
+ * {@link BindMarkersFactory} uses {@link BindMarkersFactory#identifiablePlaceholders() identifiable} placeholders.
+ * <p>
  * This is a subset of Spring Frameworks's {@code org.springframework.r2dbc.namedparam.NamedParameterUtils}.
  *
  * @author Thomas Risberg
@@ -260,7 +264,7 @@ private static int skipCommentsAndQuotes(char[] statement, int position) {
 	public static PreparedOperation<String> substituteNamedParameters(ParsedSql parsedSql,
 			BindMarkersFactory bindMarkersFactory, BindParameterSource paramSource) {
 
-		BindMarkerHolder markerHolder = new BindMarkerHolder(bindMarkersFactory.create());
+		NamedParameters markerHolder = new NamedParameters(bindMarkersFactory);
 
 		String originalSql = parsedSql.getOriginalSql();
 		List<String> paramNames = parsedSql.getParameterNames();
@@ -276,9 +280,11 @@ public static PreparedOperation<String> substituteNamedParameters(ParsedSql pars
 			int startIndex = indexes[0];
 			int endIndex = indexes[1];
 			actualSql.append(originalSql, lastIndex, startIndex);
+			NamedParameters.NamedParameter marker = markerHolder.getOrCreate(paramName);
 			if (paramSource.hasValue(paramName)) {
 				Object value = paramSource.getValue(paramName);
 				if (value instanceof Collection) {
+
 					Iterator<?> entryIter = ((Collection<?>) value).iterator();
 					int k = 0;
 					while (entryIter.hasNext()) {
@@ -294,19 +300,19 @@ public static PreparedOperation<String> substituteNamedParameters(ParsedSql pars
 								if (m > 0) {
 									actualSql.append(", ");
 								}
-								actualSql.append(markerHolder.addMarker(paramName));
+								actualSql.append(marker.addPlaceholder());
 							}
 							actualSql.append(')');
 						} else {
-							actualSql.append(markerHolder.addMarker(paramName));
+							actualSql.append(marker.addPlaceholder());
 						}
 
 					}
 				} else {
-					actualSql.append(markerHolder.addMarker(paramName));
+					actualSql.append(marker.getPlaceholder());
 				}
 			} else {
-				actualSql.append(markerHolder.addMarker(paramName));
+				actualSql.append(marker.getPlaceholder());
 			}
 			lastIndex = endIndex;
 		}
@@ -387,22 +393,79 @@ public int hashCode() {
 	}
 
 	/**
-	 * Holder for bind marker progress.
+	 * Holder for bind markers progress.
 	 */
-	private static class BindMarkerHolder {
+	static class NamedParameters {
 
 		private final BindMarkers bindMarkers;
-		private final Map<String, List<BindMarker>> markers = new TreeMap<>();
+		private final boolean identifiable;
+		private final Map<String, List<NamedParameter>> references = new TreeMap<>();
+
+		NamedParameters(BindMarkersFactory factory) {
+			this.bindMarkers = factory.create();
+			this.identifiable = factory.identifiablePlaceholders();
+		}
+
+		/**
+		 * Get the {@link NamedParameter} identified by {@code namedParameter}.
+		 *
+		 * @param namedParameter
+		 * @return
+		 */
+		NamedParameter getOrCreate(String namedParameter) {
+
+			List<NamedParameter> reference = this.references.computeIfAbsent(namedParameter, ignore -> new ArrayList<>());
+
+			if (reference.isEmpty()) {
+				NamedParameter param = new NamedParameter(namedParameter);
+				reference.add(param);
+				return param;
+			}
 
-		BindMarkerHolder(BindMarkers bindMarkers) {
-			this.bindMarkers = bindMarkers;
+			if (this.identifiable) {
+				return reference.get(0);
+			}
+
+			NamedParameter param = new NamedParameter(namedParameter);
+			reference.add(param);
+			return param;
 		}
 
-		String addMarker(String name) {
+		List<NamedParameter> getMarker(String name) {
+			return this.references.get(name);
+		}
+
+		class NamedParameter {
+
+			private final String namedParameter;
+			private final List<BindMarker> placeholders = new ArrayList<>();
+
+			NamedParameter(String namedParameter) {
+				this.namedParameter = namedParameter;
+			}
+
+			/**
+			 * Create a placeholder to translate a single value into a bindable parameter.
+			 * <p>
+			 * Can be called multiple times to create placeholders for array/collections.
+			 *
+			 * @return
+			 */
+			String addPlaceholder() {
+
+				BindMarker bindMarker = NamedParameters.this.bindMarkers.next(this.namedParameter);
+				this.placeholders.add(bindMarker);
+				return bindMarker.getPlaceholder();
+			}
+
+			String getPlaceholder() {
+
+				if (this.placeholders.isEmpty()) {
+					return addPlaceholder();
+				}
 
-			BindMarker bindMarker = this.bindMarkers.next(name);
-			this.markers.computeIfAbsent(name, ignore -> new ArrayList<>()).add(bindMarker);
-			return bindMarker.getPlaceholder();
+				return this.placeholders.get(0).getPlaceholder();
+			}
 		}
 	}
 
@@ -414,13 +477,13 @@ private static class ExpandedQuery implements PreparedOperation<String> {
 
 		private final String expandedSql;
 
-		private final Map<String, List<BindMarker>> markers;
+		private final NamedParameters parameters;
 
 		private final BindParameterSource parameterSource;
 
-		ExpandedQuery(String expandedSql, BindMarkerHolder bindMarkerHolder, BindParameterSource parameterSource) {
+		ExpandedQuery(String expandedSql, NamedParameters parameters, BindParameterSource parameterSource) {
 			this.expandedSql = expandedSql;
-			this.markers = bindMarkerHolder.markers;
+			this.parameters = parameters;
 			this.parameterSource = parameterSource;
 		}
 
@@ -435,13 +498,7 @@ public void bind(BindTarget target, String identifier, Object value) {
 				return;
 			}
 
-			if (bindMarkers.size() == 1) {
-				bindMarkers.get(0).bind(target, value);
-			} else {
-
-				Assert.isInstanceOf(Collection.class, value,
-						() -> String.format("Value [%s] must be an Collection with a size of [%d]", value, bindMarkers.size()));
-
+			if (value instanceof Collection) {
 				Collection<Object> collection = (Collection<Object>) value;
 
 				Iterator<Object> iterator = collection.iterator();
@@ -460,6 +517,10 @@ public void bind(BindTarget target, String identifier, Object value) {
 						bind(target, markers, valueToBind);
 					}
 				}
+			} else {
+				for (BindMarker bindMarker : bindMarkers) {
+					bindMarker.bind(target, value);
+				}
 			}
 		}
 
@@ -483,16 +544,26 @@ public void bindNull(BindTarget target, String identifier, Class<?> valueType) {
 				return;
 			}
 
-			if (bindMarkers.size() == 1) {
-				bindMarkers.get(0).bindNull(target, valueType);
-				return;
+			for (BindMarker bindMarker : bindMarkers) {
+				bindMarker.bindNull(target, valueType);
 			}
-
-			throw new UnsupportedOperationException("bindNull(…) can bind only singular values");
 		}
 
-		private List<BindMarker> getBindMarkers(String identifier) {
-			return this.markers.get(identifier);
+		List<BindMarker> getBindMarkers(String identifier) {
+
+			List<NamedParameters.NamedParameter> parameters = this.parameters.getMarker(identifier);
+
+			if (parameters == null) {
+				return null;
+			}
+
+			List<BindMarker> markers = new ArrayList<>();
+
+			for (NamedParameters.NamedParameter parameter : parameters) {
+				markers.addAll(parameter.placeholders);
+			}
+
+			return markers;
 		}
 
 		@Override

src/main/java/org/springframework/data/r2dbc/dialect/BindMarkersFactory.java

@@ -26,6 +26,17 @@ public interface BindMarkersFactory {
 	 */
 	BindMarkers create();
 
+	/**
+	 * Return whether the {@link BindMarkersFactory} uses identifiable placeholders.
+	 *
+	 * @return whether the {@link BindMarkersFactory} uses identifiable placeholders. {@literal false} if multiple
+	 *         placeholders cannot be distinguished by just the {@link BindMarker#getPlaceholder() placeholder}
+	 *         identifier.
+	 */
+	default boolean identifiablePlaceholders() {
+		return true;
+	}
+
 	/**
 	 * Create index-based {@link BindMarkers} using indexes to bind parameters. Allow customization of the bind marker
 	 * placeholder {@code prefix} to represent the bind marker as placeholder within the query.
@@ -40,6 +51,7 @@ public interface BindMarkersFactory {
 	static BindMarkersFactory indexed(String prefix, int beginWith) {
 
 		Assert.notNull(prefix, "Prefix must not be null!");
+
 		return () -> new IndexedBindMarkers(prefix, beginWith);
 	}
 
@@ -56,7 +68,19 @@ static BindMarkersFactory indexed(String prefix, int beginWith) {
 	static BindMarkersFactory anonymous(String placeholder) {
 
 		Assert.hasText(placeholder, "Placeholder must not be empty!");
-		return () -> new AnonymousBindMarkers(placeholder);
+
+		return new BindMarkersFactory() {
+
+			@Override
+			public BindMarkers create() {
+				return new AnonymousBindMarkers(placeholder);
+			}
+
+			@Override
+			public boolean identifiablePlaceholders() {
+				return false;
+			}
+		};
 	}
 
 	/**

src/test/java/org/springframework/data/r2dbc/core/AbstractDatabaseClientIntegrationTests.java

@@ -152,6 +152,18 @@ public void executeSelect() {
 				}).verifyComplete();
 	}
 
+	@Test // gh-2
+	public void executeSelectNamedParameters() {
+
+		DatabaseClient databaseClient = DatabaseClient.create(connectionFactory);
+
+		databaseClient.execute("SELECT id, name, manual FROM legoset WHERE name = :name or manual = :name") //
+				.bind("name", "unknown").as(LegoSet.class) //
+				.fetch().all() //
+				.as(StepVerifier::create) //
+				.verifyComplete();
+	}
+
 	@Test // gh-2
 	public void insert() {