Skip to content
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.

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

MetadataBackedField#getPath seems to be very inefficient for field names that are not "simple" #619

Closed
amoffet opened this issue Jul 6, 2021 · 4 comments
Assignees
Labels
status: superseded An issue that has been superseded by another

Comments

@amoffet
Copy link

amoffet commented Jul 6, 2021

spring-data-r2dbc version 1.2.9

Given a field name such as person_change_id QueryMapper#forName will iterate over the parts of the path:

person
change
id

trying to construct a PropertyPath. The root call is:

    private PersistentPropertyPath<RelationalPersistentProperty> getPath(String pathExpression) {
      try {
        PropertyPath path = this.forName(pathExpression);
        return this.isPathToJavaLangClassProperty(path) ? null : this.mappingContext.getPersistentPropertyPath(path);
      } catch (PropertyReferenceException | MappingException var3) {
        return null;
      }
    }

The method above will be called for the first part of the path (person).

The owning type (entity) is "asked" if it has a property of that kind ( see constructor PropertyPath<String , TypeInformation, List>):

    TypeInformation<?> propertyType = owningType.getProperty(propertyName);
    if (propertyType == null) {
      throw new PropertyReferenceException(propertyName, owningType, base);
    } else {

If it does not (the owning type has personChangeId and not person), it throws the exception which is converted to a "null" in QueryMapper (as shown in the code at the top of this issue).

Each time a Query of the same kind is executed, this same code path is followed. (SELECT * FROM changes WHERE person_change_id = (any number)). The field we are trying to construct metadata for comes from the select list. In my application, I'm seeing hundreds of thousands of exceptions in a few minutes.

Admittedly, I haven't been able to dig deeply enough yet to understand what condition yields a good property path in the MetadataBackedField, but it seems quite specialized. No field in my simple entity meets the criteria:

id BIGSERIAL PRIMARY KEY,
person_change_id BIGINT NOT NULL,
group_id VARCHAR(255) NOT NULL,
person_count_key VARCHAR(255) NOT NULL,
create_date TIMESTAMP NOT NULL default current_timestamp,
update_date TIMESTAMP NOT NULL default current_timestamp

But I will get 5 exceptions from a simple query (each of which are converted to null).

See picture of stack trace for additional context:

Stack Trace

This is the dominant exception in my application:

Exceptions

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 6, 2021
@amoffet amoffet changed the title MetadataBackedField#getPath seems to be inefficient for field names that are not "simple" MetadataBackedField#getPath seems to be very inefficient for field names that are not "simple" Jul 6, 2021
@mp911de
Copy link
Member

mp911de commented Jul 16, 2021

@mp911de mp911de added the for: team-attention An issue we need to discuss as a team to make progress label Jul 16, 2021
@amoffet
Copy link
Author

amoffet commented Aug 9, 2021

I'm interested in knowing how the process goes from here. It looks like it's stalled. Does the team consider these things on iteration boundaries?

@amoffet
Copy link
Author

amoffet commented Jan 11, 2022

Like to bring this to the front again for consideration. It's a real performance killer.

@mp911de mp911de added status: superseded An issue that has been superseded by another and removed for: team-attention An issue we need to discuss as a team to make progress status: waiting-for-triage An issue we've not yet triaged labels Jul 12, 2023
@mp911de
Copy link
Member

mp911de commented Jul 12, 2023

Superseded by spring-projects/spring-data-commons#2837

@mp911de mp911de closed this as completed Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

3 participants