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

SqlServerDialect should be case sensitive #914

Closed
jandurovec opened this issue Jan 20, 2021 · 3 comments
Closed

SqlServerDialect should be case sensitive #914

jandurovec opened this issue Jan 20, 2021 · 3 comments
Labels
type: enhancement A general enhancement

Comments

@jandurovec
Copy link

jandurovec commented Jan 20, 2021

Scenario desription

Microsoft SQL Server is case insensitive by default, but can be changed to case sensitive when COLLATION server property is set (to some ..._CS_... value).

Let's assume we have such SQL Server instance (e.g. with COLLATION set to Latin1_General_CS_AS).

When I declare my entity like this

@Data
@AllArgsConstructor
public class MyObject {
  private String myProperty;
}

then the following SQL statement is generated by default: SELECT "MY_OBJECT"."MY_PROPERTY" AS "MY_PROPERTY" FROM "MY_OBJECT".

If I want to have table and column names corresponding to class/field names, I have 2 options

  • Add @Table and @Column annotations to each field
  • Implement a custom NamingStrategy

As I have a lot of entities and lot of fields, NamingStrategy seems like a better option, since I don't want all my entities to have duplicated literals like this

@Data
@AllArgsConstructor
@Table("MyObject")
public class MyObject {
  @Column("myProperty")
  private String myProperty;
}

I'll implement a custom NamingStrategy

new NamingStrategy() {
  @Override
  public String getTableName(Class<?> type) {
    Assert.notNull(type, "Type must not be null");
    return type.getSimpleName();
  }

  @Override
  public String getColumnName(RelationalPersistentProperty property) {
    Assert.notNull(property, "Property must not be null");
    return property.getName();
  }
}

Now the underscores are gone, but the select SQL looks like SELECT "MYOBJECT"."MYPROPERTY" AS "MYPROPERTY" FROM "MYOBJECT".

I.e. the identifiers get converted to uppercase which leads to com.microsoft.sqlserver.jdbc.SQLServerException: Invalid object name 'MYOBJECT', because my DB is case sensitive and the correct table name is MyObject not MYOBJECT.

Surprisingly, if I don't use a NamingStrategy but add explicit @Table and @Column annotations, it works as expected.

Issue analysis

The problem is that the SqlIdentifier instances generated by RelationalPersistentEntityImpl and BasicRelationalPersistentProperty are of different types when names are specified explicitly and when derived from class/field names.

While DefaultSqlIdentifier class (used when table/column names are explicitly specified) ignores IdentifierProcessing letter case standardization (is it intentional or a bug?) when generating SQL, the DerivedSqlIdentifier pushes the identifier through standardizeLetterCase method which causes the identifier to be converted to upper-case (as SqlServerDialect returns IdentifierProcessing.ANSI).

Solution proposal

I believe that SqlServerDialect.getIdentifierProcessing() should return IdentifierProcessing.NONE instead of IdentifierProcessing.ANSI.

If the SQL Server is in case-insensitive mode, then it doesn't make any difference whether the generated SQL is SELECT FROM "MY_OBJECT" or SELECT FROM "My_Object" (i.e. changing from ANSI to NONE won't break the case-insensitive mode).

However, in case-sensitive mode, having IdentifierProcessing set to NONE would help better control the object identifiers and it would save a lot of code duplication. I.e. in the scenario above I'd be able to use the NamingStrategy as expected without having to duplicate each class and field name in explicit @Table or @Column annotation.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 20, 2021
@schauder
Copy link
Contributor

Is SqlServer case insensitive even when the identifiers are quoted? If not going from ANSI to NONE would break things.

My first instinct tells me that you should implement (or we should have) a separate dialect for this case.

@jandurovec
Copy link
Author

Yes SqlServer is case insensitive even when quoted.

E.g. if I have COLLATION set to Lating1_General_CI_AS (_CI_ indicating case insensitive) and there's a dbo.MyObject table in the database, then all of these queries work (both quoted and unquoted) and return results

SELECT * FROM dbo.MyObject
SELECT * FROM DBO.MYOBJECT
SELECT * FROM "dbo"."MyObject"
SELECT * FROM "DBO"."MYOBJECT"

@schauder
Copy link
Contributor

Ok, we'll go with your proposal.

@schauder schauder added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 21, 2021
@schauder schauder added this to the 2.2 M3 (2021.0.0) milestone Jan 21, 2021
schauder added a commit that referenced this issue Aug 31, 2022
Quoting is important since it allows use of keywords as names.
We do not change the letter casing.
In a default setup the database does not care since it is case-insensitive.
If configured to be case-sensitive it makes sense to pass on what ever letter casing there is, since you seem to care.

Closes #1216
See #914
schauder added a commit that referenced this issue Dec 7, 2022
Quoting is important since it allows use of keywords as names.
We do not change the letter casing.
In a default setup the database does not care since it is case-insensitive.
If configured to be case-sensitive it makes sense to pass on what ever letter casing there is, since you seem to care.

Closes #1216
See #914
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants