DATAJPA-1307 - JDBC style query parameters work for native queries.

JDBC style query parameters denoted by a simple ? work for native queries.
They can't be used with JPA queries nor with other parameter formats like ?3, :name or SpEL parameters.

src/main/java/org/springframework/data/jpa/repository/query/AbstractStringBasedJpaQuery.java

@@ -64,6 +64,9 @@ abstract class AbstractStringBasedJpaQuery extends AbstractJpaQuery {
 		this.countQuery = query.deriveCountQuery(method.getCountQuery(), method.getCountQueryProjection());
 
 		this.parser = parser;
+
+		Assert.isTrue(method.isNativeQuery() || !query.usesJdbcStyleParameters(),
+				"JDBC style parameters (?) are not supported for JPA queries.");
 	}
 
 	/*

src/main/java/org/springframework/data/jpa/repository/query/DeclaredQuery.java

@@ -91,4 +91,11 @@ interface DeclaredQuery {
 	default boolean usesPaging() {
 		return false;
 	}
+
+	/**
+	 * Returns wether the query uses JDBC style parameters, i.e. parameters denoted by a simple ? without any index or name.
+	 *
+	 * @return Wether the query uses JDBC style parameters.
+	 */
+	boolean usesJdbcStyleParameters();
 }

src/main/java/org/springframework/data/jpa/repository/query/EmptyDeclaredQuery.java

@@ -99,4 +99,13 @@ class EmptyDeclaredQuery implements DeclaredQuery {
 
 		return DeclaredQuery.of(countQuery);
 	}
+
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jpa.repository.query.DeclaredQuery#usesJdbcStyleParameters()
+	 */
+	@Override
+	public boolean usesJdbcStyleParameters() {
+		return false;
+	}
 }

src/main/java/org/springframework/data/jpa/repository/query/StringQuery.java

@@ -54,6 +54,7 @@ class StringQuery implements DeclaredQuery {
 	private final @Nullable String alias;
 	private final boolean hasConstructorExpression;
 	private final boolean containsPageableInSpel;
+	private final boolean usesJdbcStyleParameters;
 
 	/**
 	 * Creates a new {@link StringQuery} from the given JPQL query.
@@ -67,9 +68,11 @@ class StringQuery implements DeclaredQuery {
 
 		this.bindings = new ArrayList<>();
 		this.containsPageableInSpel = query.contains("#pageable");
+		Metadata queryMeta = new Metadata();
 		this.query = ParameterBindingParser.INSTANCE.parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(query,
-				this.bindings);
+				this.bindings, queryMeta);
 
+		this.usesJdbcStyleParameters = queryMeta.usesJdbcStyleParameters;
 		this.alias = QueryUtils.detectAlias(query);
 		this.hasConstructorExpression = QueryUtils.hasConstructorExpression(query);
 	}
@@ -106,6 +109,15 @@ class StringQuery implements DeclaredQuery {
 				.of(countQuery != null ? countQuery : QueryUtils.createCountQueryFor(query, countQueryProjection));
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.jpa.repository.query.DeclaredQuery#usesJdbcStyleParameters()
+	 */
+	@Override
+	public boolean usesJdbcStyleParameters() {
+		return usesJdbcStyleParameters;
+	}
+
 	/*
 	 * (non-Javadoc)
 	 * @see org.springframework.data.jpa.repository.query.DeclaredQuery#getQueryString()
@@ -171,7 +183,11 @@ class StringQuery implements DeclaredQuery {
 		INSTANCE;
 
 		static final String EXPRESSION_PARAMETER_PREFIX = "__$synthetic$__";
-		private static final Pattern PARAMETER_BINDING_BY_INDEX = Pattern.compile("\\?(\\d+)");
+		public static final String POSITIONAL_OR_INDEXED_PARAMETER = "\\?(\\d*+(?![#\\w]))";
+		// .....................................................................^ not followed by a hash or a letter.
+		// .................................................................^ zero or more digits.
+		// .............................................................^ start with a question mark.
+		private static final Pattern PARAMETER_BINDING_BY_INDEX = Pattern.compile(POSITIONAL_OR_INDEXED_PARAMETER);
 		private static final Pattern PARAMETER_BINDING_PATTERN;
 		private static final String MESSAGE = "Already found parameter binding with same index / parameter name but differing binding type! "
 				+ "Already have: %s, found %s! If you bind a parameter multiple times make sure they use the same binding.";
@@ -197,7 +213,7 @@ class StringQuery implements DeclaredQuery {
 			builder.append("(?: )?"); // some whitespace
 			builder.append("\\(?"); // optional braces around parameters
 			builder.append("(");
-			builder.append("%?(\\?(\\d+))%?"); // position parameter and parameter index
+			builder.append("%?(" + POSITIONAL_OR_INDEXED_PARAMETER + ")%?"); // position parameter and parameter index
 			builder.append("|"); // or
 
 			// named parameter and the parameter name
@@ -214,8 +230,8 @@ class StringQuery implements DeclaredQuery {
 		 * Parses {@link ParameterBinding} instances from the given query and adds them to the registered bindings. Returns
 		 * the cleaned up query.
 		 */
-		String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(String query,
-				List<ParameterBinding> bindings) {
+		String parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(String query, List<ParameterBinding> bindings,
+				Metadata queryMeta) {
 
 			String result = query;
 			Matcher matcher = PARAMETER_BINDING_PATTERN.matcher(query);
@@ -240,6 +256,7 @@ class StringQuery implements DeclaredQuery {
 
 			QuotationMap quotationMap = new QuotationMap(query);
 
+			boolean usesJpaStyleParameters = false;
 			while (matcher.find()) {
 
 				if (quotationMap.isQuoted(matcher.start())) {
@@ -248,30 +265,50 @@ class StringQuery implements DeclaredQuery {
 
 				String parameterIndexString = matcher.group(INDEXED_PARAMETER_GROUP);
 				String parameterName = parameterIndexString != null ? null : matcher.group(NAMED_PARAMETER_GROUP);
-				Integer parameterIndex = parameterIndexString == null ? null : Integer.valueOf(parameterIndexString);
+				Integer parameterIndex = getParameterIndex(parameterIndexString);
+
 				String typeSource = matcher.group(COMPARISION_TYPE_GROUP);
 				String expression = null;
 				String replacement = null;
 
 				if (parameterName == null && parameterIndex == null) {
+
 					expressionParameterIndex++;
 
-					if (parametersShouldBeAccessedByIndex) {
+					if ("".equals(parameterIndexString)) {
+
 						parameterIndex = expressionParameterIndex;
-						replacement = "?" + parameterIndex;
+						queryMeta.usesJdbcStyleParameters = true;
 					} else {
-						parameterName = EXPRESSION_PARAMETER_PREFIX + expressionParameterIndex;
-						replacement = ":" + parameterName;
+
+						usesJpaStyleParameters = true;
+
+						if (parametersShouldBeAccessedByIndex) {
+
+							parameterIndex = expressionParameterIndex;
+							replacement = "?" + parameterIndex;
+						} else {
+
+							parameterName = EXPRESSION_PARAMETER_PREFIX + expressionParameterIndex;
+							replacement = ":" + parameterName;
+						}
 					}
 
 					expression = matcher.group(EXPRESSION_GROUP);
+				} else {
+					usesJpaStyleParameters = true;
 				}
 
+				if (usesJpaStyleParameters && queryMeta.usesJdbcStyleParameters) {
+					throw new IllegalArgumentException("Mixing of ? parameters and other forms like ?1 is not supported");
+				}
+
+				String replacementTarget = matcher.group(2);
 				switch (ParameterBindingType.of(typeSource)) {
 
 					case LIKE:
 
-						Type likeType = LikeParameterBinding.getLikeTypeFrom(matcher.group(2));
+						Type likeType = LikeParameterBinding.getLikeTypeFrom(replacementTarget);
 						replacement = replacement != null ? replacement : matcher.group(3);
 
 						if (parameterIndex != null) {
@@ -299,10 +336,11 @@ class StringQuery implements DeclaredQuery {
 
 						bindings.add(parameterIndex != null ? new ParameterBinding(null, parameterIndex, expression)
 								: new ParameterBinding(parameterName, null, expression));
+
 				}
 
 				if (replacement != null) {
-					result = replaceFirst(result, matcher.group(2), replacement);
+					result = replaceFirst(result, replacementTarget, replacement);
 				}
 
 			}
@@ -310,6 +348,15 @@ class StringQuery implements DeclaredQuery {
 			return result;
 		}
 
+		@Nullable
+		private Integer getParameterIndex(@Nullable String parameterIndexString) {
+
+			if (parameterIndexString == null || parameterIndexString.isEmpty()) {
+				return null;
+			}
+			return Integer.valueOf(parameterIndexString);
+		}
+
 		private static String replaceFirst(String text, String substring, String replacement) {
 
 			int index = text.indexOf(substring);
@@ -326,8 +373,12 @@ class StringQuery implements DeclaredQuery {
 
 			int greatestParameterIndex = -1;
 			while (parameterIndexMatcher.find()) {
+
 				String parameterIndexString = parameterIndexMatcher.group(1);
-				greatestParameterIndex = Math.max(greatestParameterIndex, Integer.parseInt(parameterIndexString));
+				Integer parameterIndex = getParameterIndex(parameterIndexString);
+				if (parameterIndex != null) {
+					greatestParameterIndex = Math.max(greatestParameterIndex, parameterIndex);
+				}
 			}
 
 			return greatestParameterIndex;
@@ -839,4 +890,8 @@ class StringQuery implements DeclaredQuery {
 			return quotedRanges.stream().anyMatch(r -> r.contains(index));
 		}
 	}
+
+	static class Metadata {
+		private boolean usesJdbcStyleParameters = false;
+	}
 }

src/test/java/org/springframework/data/jpa/repository/UserRepositoryTests.java

@@ -2202,6 +2202,14 @@ public class UserRepositoryTests {
 		softly.assertAll();
 	}
 
+	@Test // DATAJPA-1307
+	public void testFindByEmailAddressJdbcStyleParameter() throws Exception {
+
+		flushTestUsers();
+
+		assertThat(repository.findByEmailNativeAddressJdbcStyleParameter("gierke@synyx.de")).isEqualTo(firstUser);
+	}
+
 	private Page<User> executeSpecWithSort(Sort sort) {
 
 		flushTestUsers();

src/test/java/org/springframework/data/jpa/repository/query/ParameterBindingParserUnitTests.java

@@ -70,7 +70,8 @@ public class ParameterBindingParserUnitTests {
 	public void checkHasParameter(SoftAssertions softly, String query, boolean containsParameter, String label) {
 
 		List<ParameterBinding> bindings = new ArrayList<>();
-		ParameterBindingParser.INSTANCE.parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(query, bindings);
+		ParameterBindingParser.INSTANCE.parseParameterBindingsOfQueryIntoBindingsAndReturnCleanedQuery(query, bindings,
+				new StringQuery.Metadata());
 		softly.assertThat(bindings.size()) //
 				.describedAs(String.format("<%s> (%s)", query, label)) //
 				.isEqualTo(containsParameter ? 1 : 0);

src/test/java/org/springframework/data/jpa/repository/query/SimpleJpaQueryUnitTests.java

@@ -15,9 +15,12 @@
  */
 package org.springframework.data.jpa.repository.query;
 
+import static org.assertj.core.api.Assertions.*;
 import static org.hamcrest.Matchers.*;
-import static org.junit.Assert.*;
-import static org.mockito.ArgumentMatchers.*;
+import static org.junit.Assert.assertThat;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.*;
 
 import java.lang.reflect.Method;
@@ -219,6 +222,20 @@ public class SimpleJpaQueryUnitTests {
 		verify(em, times(2)).createQuery(anyString());
 	}
 
+	@Test // DATAJPA-1307
+	public void jdbcStyleParametersOnlyAllowedInNativeQueries() throws Exception {
+
+		// just verifying that it doesn't throw an exception
+		createJpaQuery(SampleRepository.class.getMethod("legalUseOfJdbcStyleParameters", String.class));
+
+		assertThatExceptionOfType(IllegalArgumentException.class) //
+				.isThrownBy( //
+						() -> createJpaQuery( //
+								SampleRepository.class.getMethod("illegalUseOfJdbcStyleParameters", String.class) //
+						) //
+		);
+	}
+
 	private AbstractJpaQuery createJpaQuery(Method method) {
 
 		JpaQueryMethod queryMethod = new JpaQueryMethod(method, metadata, factory, extractor);
@@ -236,6 +253,12 @@ public class SimpleJpaQueryUnitTests {
 		@Query(value = "SELECT u FROM User u WHERE u.lastname = ?1", nativeQuery = true)
 		List<User> findNativeByLastname(String lastname, Pageable pageable);
 
+		@Query(value = "SELECT u FROM User u WHERE u.lastname = ?", nativeQuery = true)
+		List<User> legalUseOfJdbcStyleParameters(String lastname);
+
+		@Query(value = "SELECT u FROM User u WHERE u.lastname = ?")
+		List<User> illegalUseOfJdbcStyleParameters(String lastname);
+
 		@Query(USER_QUERY)
 		List<User> findByAnnotatedQuery();
 

src/test/java/org/springframework/data/jpa/repository/query/StringQueryUnitTests.java

@@ -18,8 +18,10 @@ package org.springframework.data.jpa.repository.query;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 
+import java.util.Arrays;
 import java.util.List;
 
+import org.assertj.core.api.Assertions;
 import org.assertj.core.api.SoftAssertions;
 import org.junit.Rule;
 import org.junit.Test;
@@ -396,6 +398,77 @@ public class StringQueryUnitTests {
 		softly.assertAll();
 	}
 
+	@Test // DATAJPA-1307
+	public void detectsMultiplePositionalParameterBindingsWithoutIndex() {
+
+		SoftAssertions softly = new SoftAssertions();
+
+		String queryString = "select u from User u where u.id in ? and u.names in ? and foo = ?";
+		StringQuery query = new StringQuery(queryString);
+
+		softly.assertThat(query.getQueryString()).isEqualTo(queryString);
+		softly.assertThat(query.hasParameterBindings()).isTrue();
+		softly.assertThat(query.getParameterBindings()).hasSize(3);
+
+		softly.assertAll();
+	}
+
+	@Test // DATAJPA-1307
+	public void failOnMixedBindingsWithoutIndex() {
+
+		List<String> testQueries = Arrays.asList( //
+				"something = ? and something = ?1", //
+				"something = ?1 and something = ?", //
+				"something = :name and something = ?", //
+				"something = ?#{xx} and something = ?" //
+		);
+
+		for (String testQuery : testQueries) {
+
+			Assertions.assertThatExceptionOfType(IllegalArgumentException.class) //
+					.describedAs(testQuery).isThrownBy(() -> new StringQuery(testQuery));
+		}
+	}
+
+	@Test // DATAJPA
+	public void makesUsageOfJdbcStyleParameterAvailable() {
+
+		SoftAssertions softly = new SoftAssertions();
+
+		softly.assertThat(new StringQuery("something = ?").usesJdbcStyleParameters()).isTrue();
+
+		List<String> testQueries = Arrays.asList( //
+				"something = ?1", //
+				"something = :name", //
+				"something = ?#{xx}" //
+		);
+
+		for (String testQuery : testQueries) {
+
+			softly.assertThat(new StringQuery(testQuery) //
+					.usesJdbcStyleParameters()) //
+					.describedAs(testQuery) //
+					.isFalse();
+		}
+
+		softly.assertAll();
+	}
+
+	@Test // DATAJPA-1307
+	public void questionMarkInStringLiteral() {
+
+		SoftAssertions softly = new SoftAssertions();
+
+		String queryString = "select '? ' from dual";
+		StringQuery query = new StringQuery(queryString);
+
+		softly.assertThat(query.getQueryString()).isEqualTo(queryString);
+		softly.assertThat(query.hasParameterBindings()).isFalse();
+		softly.assertThat(query.getParameterBindings()).hasSize(0);
+
+		softly.assertAll();
+	}
+
 	public void checkNumberOfNamedParameters(String query, int expectedSize, String label) {
 
 		DeclaredQuery declaredQuery = DeclaredQuery.of(query);

src/test/java/org/springframework/data/jpa/repository/sample/UserRepository.java

@@ -549,6 +549,10 @@ public interface UserRepository
 	@Query("select firstname as firstname, lastname as lastname from User u where u.firstname = 'Oliver'")
 	Map<String, Object> findMapWithNullValues();
 
+	// DATAJPA-1307
+	@Query(value = "select * from SD_User u where u.emailAddress = ?", nativeQuery = true)
+	User findByEmailNativeAddressJdbcStyleParameter(String emailAddress);
+
 	interface RolesAndFirstname {
 
 		String getFirstname();