Skip to content

SqlIdentifier has redundant getReference(IdentifierProcessing) method #1110

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

Closed
mipo256 opened this issue Dec 8, 2021 · 0 comments
Closed
Assignees
Labels
status: pending-design-work Needs design work before any code can be developed

Comments

@mipo256
Copy link
Contributor

mipo256 commented Dec 8, 2021

I have noticed that SqlIdentifier has redundant SqlIdentifier#getReference(IdentifierProcessing) method. The thing is, in accordance to the documentation, this method should:

Return the reference name after applying {@link IdentifierProcessing} rules.

But the thing this method does not apply anything in reality and acts in identical manner to SqlIdentifier#getReference(). Nither the DefaultSqlIdentifer, nor DerivedSqlIdentifier do not do basically anything to their underlying sql parameter name (neadless to say about CompositeSqlIdentifier), for example:

DerivedSqlIdentifier:

@Override
public String getReference(IdentifierProcessing processing) {
    return this.name;
}

And DefaultSqlIdentifier:

@Override
public String getReference(IdentifierProcessing processing) {
    return name;
}

So, in reality, this method does not differ from SqlIdentifier#getReference() - it really confuses. Even more - we already have SqlIdentifier#toSql(IdentifierProcessing) which it really intended to apply IdentifierProcessing to the SqlIdentifier. I assume that this is an architectural miscalculation. Jens @schauder, I think we need to clean this up in order to eliminate confusion.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 8, 2021
@schauder schauder added the status: pending-design-work Needs design work before any code can be developed label Jan 24, 2022
@schauder schauder self-assigned this Jan 24, 2022
@odrotbohm odrotbohm removed the status: waiting-for-triage An issue we've not yet triaged label Feb 7, 2022
@mipo256 mipo256 mentioned this issue Mar 24, 2022
@kurtn718 kurtn718 self-assigned this Mar 20, 2023
kurtn718 added a commit to kurtn718/spring-data-relational that referenced this issue Mar 20, 2023
The method also fixed the bug of this method not honoring the IdentifierProcessing parameter, so the functionality is now identical to the toSql(IdentifierProcessing) method.

Closes spring-projects#1110
schauder added a commit that referenced this issue Mar 23, 2023
Added `@author` tags.
Undo the change in behaviour of getReference(IdentifierProcessing).

See #1110
Original pull request #1458
@schauder schauder removed their assignment Mar 23, 2023
@schauder schauder added this to the 3.1 RC1 (2023.0.0) milestone Mar 23, 2023
schauder pushed a commit that referenced this issue Mar 23, 2023
schauder added a commit that referenced this issue Mar 23, 2023
Original pull request #1209
See #1110
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment