Skip to content

Caching path resolution #2336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
schauder wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Caching path resolution #2336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51d858f
2335 - Prepare branch
schauder Aug 3, 2026
7112d19
Caching path resolution
schauder Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>4.2.0-SNAPSHOT</version>
<version>4.2.0-2335-SNAPSHOT</version>
<packaging>pom</packaging>

<name>Spring Data Relational Parent</name>
Expand Down
2 changes: 1 addition & 1 deletion spring-data-jdbc-distribution/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>4.2.0-SNAPSHOT</version>
<version>4.2.0-2335-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>

Expand Down
4 changes: 2 additions & 2 deletions spring-data-jdbc/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-jdbc</artifactId>
<version>4.2.0-SNAPSHOT</version>
<version>4.2.0-2335-SNAPSHOT</version>

<name>Spring Data JDBC</name>
<description>Spring Data module for JDBC repositories.</description>
Expand All @@ -13,7 +13,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>4.2.0-SNAPSHOT</version>
<version>4.2.0-2335-SNAPSHOT</version>
</parent>

<properties>
Expand Down
48 changes: 7 additions & 41 deletions spring-data-jdbc/src/main/java/org/springframework/data/jdbc/core/convert/QueryMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,17 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

import org.jspecify.annotations.Nullable;

import org.springframework.data.core.PropertyPath;
import org.springframework.data.core.PropertyReferenceException;
import org.springframework.data.core.TypeInformation;
import org.springframework.data.domain.Sort;
import org.springframework.data.jdbc.core.mapping.JdbcValue;
import org.springframework.data.jdbc.support.JdbcUtil;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.mapping.PersistentPropertyAccessor;
import org.springframework.data.mapping.PersistentPropertyPath;
import org.springframework.data.mapping.context.MappingContext;
import org.springframework.data.relational.core.mapping.PropertyPathResolver;
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
import org.springframework.data.relational.core.query.CriteriaDefinition;
Expand Down Expand Up @@ -63,6 +60,7 @@ public class QueryMapper {

private final JdbcConverter converter;
private final MappingContext<? extends RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext;
private final PropertyPathResolver propertyPathResolver;

@mp911de mp911de Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QueryMapper is created quite frequently (e.g. AggregateReader.createCondition, StatementFactory via PartTreeJdbcQuery.execute or createDeleteQueries), therefore we create many instances of a cache that do not survive long enough to establish meaningful caching.


/**
* Creates a new {@link QueryMapper} with the given {@link JdbcConverter}.
Expand All @@ -75,6 +73,7 @@ public QueryMapper(JdbcConverter converter) {

this.converter = converter;
this.mappingContext = converter.getMappingContext();
this.propertyPathResolver = new PropertyPathResolver(this.mappingContext);
}

/**
Expand Down Expand Up @@ -305,7 +304,7 @@ private Condition mapCondition(CriteriaDefinition criteria, MapSqlParameterSourc

// IN/NOT_IN with collection of composite/embedded values: expand to (… AND …) OR (…)
if ((Comparator.IN.equals(comparator) || Comparator.NOT_IN.equals(comparator))
&& value instanceof Collection<?> collection) {
&& value instanceof Collection<?> collection) {

Condition condition = null;

Expand Down Expand Up @@ -760,6 +759,7 @@ public Comparator getComparator() {
public boolean isIgnoreCase() {
return delegate.isIgnoreCase();
}

@Override
public boolean isGroup() {
return false;
Expand All @@ -776,7 +776,6 @@ public SqlIdentifier getColumn() {
return null;
}


@Nullable
@Override
public Object getValue() {
Expand Down Expand Up @@ -863,7 +862,7 @@ public SQLType getSqlType() {
/**
* Extension of {@link Field} to be backed with mapping metadata.
*/
protected static class MetadataBackedField extends Field {
protected class MetadataBackedField extends Field {

@mp911de mp911de Aug 11, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: removing static makes the MappingContext<? extends RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext field superfluous


private final RelationalPersistentEntity<?> entity;
private final MappingContext<? extends RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext;
Expand Down Expand Up @@ -891,7 +890,7 @@ protected MetadataBackedField(SqlIdentifier name, RelationalPersistentEntity<?>
this.entity = entity;
this.mappingContext = context;

this.path = getPath(name.getReference());
this.path = propertyPathResolver.resolve(entity, name.getReference());

RelationalPersistentProperty persistentProperty = null;
if (this.path != null) {
Expand Down Expand Up @@ -923,39 +922,6 @@ public SqlIdentifier getMappedColumnName() {
return this.property == null ? super.getMappedColumnName() : this.property.getColumnName();
}

/**
* Returns the {@link PersistentPropertyPath} for the given {@code pathExpression}.
*/
@Nullable
private PersistentPropertyPath<RelationalPersistentProperty> getPath(String pathExpression) {

try {

PropertyPath path = forName(pathExpression);

if (isPathToJavaLangClassProperty(path)) {
return null;
}

return this.mappingContext.getPersistentPropertyPath(path);
} catch (MappingException | PropertyReferenceException e) {
return null;
}
}

private PropertyPath forName(String path) {

if (entity.getPersistentProperty(path) != null) {
return PropertyPath.from(Pattern.quote(path), entity.getTypeInformation());
}

return PropertyPath.from(path, entity.getTypeInformation());
}

private boolean isPathToJavaLangClassProperty(PropertyPath path) {
return path.getType().equals(Class.class) && path.getLeafProperty().getOwningType().getType().equals(Class.class);
}

@Nullable
public PersistentPropertyPath<RelationalPersistentProperty> getPath() {
return path;
Expand Down
47 changes: 47 additions & 0 deletions ...a-jdbc/src/test/java/org/springframework/data/jdbc/core/convert/QueryMapperUnitTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.MockedConstruction;

import org.springframework.core.convert.converter.Converter;
import org.springframework.data.annotation.Id;
import org.springframework.data.core.PropertyReferenceException;
import org.springframework.data.domain.Sort;
import org.springframework.data.jdbc.core.mapping.JdbcMappingContext;
import org.springframework.data.relational.core.mapping.Column;
Expand Down Expand Up @@ -569,6 +571,51 @@ void shouldMapSortPathIntoEmbeddable() {
.contains(OrderByField.from(table.column(SqlIdentifier.quoted("HOME_COUNTRY_NAME")), Sort.Direction.ASC));
}

@Test // GH-2335
void derivedQueryPropertyLookupDoesNotThrow() {

try (MockedConstruction<PropertyReferenceException> mocked = mockConstruction(PropertyReferenceException.class)) {

Condition condition = mapper.getMappedObject(parameterSource, Criteria.where("customerId").in(1L, 2L, 3L),
Table.create("order"), context.getRequiredPersistentEntity(Order.class));

assertThat(condition)
.hasToString("order.\"CUSTOMER_ID\" IN (?[:customer_id], ?[:customer_id1], ?[:customer_id2])");
assertThat(mocked.constructed()).isEmpty();
}
}

@Test // GH-2335
void rawColumnNameLookupIsCachedAcrossInvocations() {

// first try uses exception for flow control
try (MockedConstruction<PropertyReferenceException> mocked = mockConstruction(PropertyReferenceException.class)) {

List<OrderByField> fields = mapper.getMappedSort(Table.create("order"), Sort.by("customer_id"),
context.getRequiredPersistentEntity(Order.class));

assertThat(fields).extracting(Objects::toString).containsExactly("order.customer_id ASC");
assertThat(mocked.constructed()).hasSize(1);
}

// further attempts use the cache
try (MockedConstruction<PropertyReferenceException> mocked = mockConstruction(PropertyReferenceException.class)) {

Condition condition = mapper.getMappedObject(parameterSource, Criteria.where("customer_id").is(1L),
Table.create("order"), context.getRequiredPersistentEntity(Order.class));

assertThat(condition).hasToString("order.customer_id = ?[:customer_id]");
assertThat(mocked.constructed()).isEmpty();
}
}

static class Order {

@Id Long orderId;
Long customerId;
String status;
}

private Condition map(Criteria criteria) {

return mapper.getMappedObject(parameterSource, criteria, Table.create("person"),
Expand Down
4 changes: 2 additions & 2 deletions spring-data-r2dbc/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<modelVersion>4.0.0</modelVersion>

<artifactId>spring-data-r2dbc</artifactId>
<version>4.2.0-SNAPSHOT</version>
<version>4.2.0-2335-SNAPSHOT</version>

<name>Spring Data R2DBC</name>
<description>Spring Data module for R2DBC</description>
Expand All @@ -13,7 +13,7 @@
<parent>
<groupId>org.springframework.data</groupId>
<artifactId>spring-data-relational-parent</artifactId>
<version>4.2.0-SNAPSHOT</version>
<version>4.2.0-2335-SNAPSHOT</version>
</parent>

<properties>
Expand Down
50 changes: 7 additions & 43 deletions spring-data-r2dbc/src/main/java/org/springframework/data/r2dbc/query/QueryMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,19 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

import org.jspecify.annotations.Nullable;

import org.springframework.data.core.PropertyPath;
import org.springframework.data.core.PropertyReferenceException;
import org.springframework.data.core.TypeInformation;
import org.springframework.data.domain.Sort;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.mapping.PersistentProperty;
import org.springframework.data.mapping.PersistentPropertyAccessor;
import org.springframework.data.mapping.PersistentPropertyPath;
import org.springframework.data.mapping.context.MappingContext;
import org.springframework.data.r2dbc.convert.R2dbcConverter;
import org.springframework.data.r2dbc.dialect.R2dbcDialect;
import org.springframework.data.relational.core.dialect.Escaper;
import org.springframework.data.relational.core.mapping.PropertyPathResolver;
import org.springframework.data.relational.core.mapping.RelationalPersistentEntity;
import org.springframework.data.relational.core.mapping.RelationalPersistentProperty;
import org.springframework.data.relational.core.query.CriteriaDefinition;
Expand Down Expand Up @@ -67,6 +64,7 @@ public class QueryMapper {
private final R2dbcConverter converter;
private final R2dbcDialect dialect;
private final MappingContext<? extends RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext;
private final PropertyPathResolver propertyPathResolver;

/**
* Creates a new {@link QueryMapper} with the given {@link R2dbcConverter}.
Expand All @@ -83,6 +81,7 @@ public QueryMapper(R2dbcDialect dialect, R2dbcConverter converter) {
this.converter = converter;
this.dialect = dialect;
this.mappingContext = (MappingContext) converter.getMappingContext();
this.propertyPathResolver = new PropertyPathResolver(this.mappingContext);
}

/**
Expand Down Expand Up @@ -336,8 +335,8 @@ private Condition getCondition(CriteriaDefinition criteria, MutableBindings bind
return mapCondition(criteria, bindings, table, entity, false);
}

private Condition combine(@Nullable Condition currentCondition,
CriteriaDefinition.Combinator combinator, Condition nextCondition) {
private Condition combine(@Nullable Condition currentCondition, CriteriaDefinition.Combinator combinator,
Condition nextCondition) {

if (currentCondition == null) {
currentCondition = nextCondition;
Expand Down Expand Up @@ -794,7 +793,7 @@ public RelationalPersistentProperty getRequiredProperty() {
/**
* Extension of {@link Field} to be backed with mapping metadata.
*/
protected static class MetadataBackedField extends Field {
protected class MetadataBackedField extends Field {

private final RelationalPersistentEntity<?> entity;
private final MappingContext<? extends RelationalPersistentEntity<?>, RelationalPersistentProperty> mappingContext;
Expand Down Expand Up @@ -834,7 +833,7 @@ protected MetadataBackedField(SqlIdentifier name, RelationalPersistentEntity<?>
this.entity = entity;
this.mappingContext = context;

this.path = getPath(name.getReference());
this.path = propertyPathResolver.resolve(entity, name.getReference());

RelationalPersistentProperty persistentProperty = null;
if (this.path != null) {
Expand Down Expand Up @@ -865,41 +864,6 @@ public SqlIdentifier getMappedColumnName() {
return this.property == null ? super.getMappedColumnName() : this.property.getColumnName();
}

/**
* Returns the {@link PersistentPropertyPath} for the given {@code pathExpression}.
*
* @param pathExpression the path expression to use.
* @return
*/
private @Nullable PersistentPropertyPath<RelationalPersistentProperty> getPath(String pathExpression) {

try {

PropertyPath path = forName(pathExpression);

if (isPathToJavaLangClassProperty(path)) {
return null;
}

return this.mappingContext.getPersistentPropertyPath(path);
} catch (MappingException | PropertyReferenceException e) {
return null;
}
}

private PropertyPath forName(String path) {

if (entity.getPersistentProperty(path) != null) {
return PropertyPath.from(Pattern.quote(path), entity.getTypeInformation());
}

return PropertyPath.from(path, entity.getTypeInformation());
}

private boolean isPathToJavaLangClassProperty(PropertyPath path) {
return path.getType().equals(Class.class) && path.getLeafProperty().getOwningType().getType().equals(Class.class);
}

@Override
public TypeInformation<?> getTypeHint() {

Expand Down
Loading
Loading