By default R2DBC uses quoted identifiers #2066
Conversation
| if (sqlName == null || sqlName.processing != processing) { | ||
|
|
||
| this.sqlName = sqlName = new CachedSqlName(processing, quoted ? processing.quote(name) : name); | ||
| String normalized = processing.standardizeLetterCase(name); |
There was a problem hiding this comment.
This change breaks PartTreeJdbcQueryUnitTests and many more.
There was a problem hiding this comment.
Several tests in JDBC are broken by this change
[ERROR] PartTreeJdbcQueryUnitTests.wrapsLikeOperatorParameterWithPercentSymbolsForNotContainingQuery:478
expected: "SELECT "users"."ID" AS "ID", "users"."AGE" AS "AGE", "users"."ACTIVE" AS "ACTIVE", "users"."LAST_NAME" AS "LAST_NAME", "users"."FIRST_NAME" AS "FIRST_NAME", "users"."DATE_OF_BIRTH" AS "DATE_OF_BIRTH", "users"."HOBBY_REFERENCE" AS "HOBBY_REFERENCE", "hated"."NAME" AS "HATED_NAME", "users"."USER_CITY" AS "USER_CITY", "users"."USER_STREET" AS "USER_STREET" FROM "users" LEFT OUTER JOIN "HOBBY" "hated" ON "hated"."USERS" = "users"."ID" WHERE "users"."FIRST_NAME" NOT LIKE :first_name"
but was: "SELECT "USERS"."ID" AS "ID", "USERS"."AGE" AS "AGE", "USERS"."ACTIVE" AS "ACTIVE", "USERS"."LAST_NAME" AS "LAST_NAME", "USERS"."FIRST_NAME" AS "FIRST_NAME", "USERS"."DATE_OF_BIRTH" AS "DATE_OF_BIRTH", "USERS"."HOBBY_REFERENCE" AS "HOBBY_REFERENCE", "HATED"."NAME" AS "HATED_NAME", "USERS"."USER_CITY" AS "USER_CITY", "USERS"."USER_STREET" AS "USER_STREET" FROM "USERS" LEFT OUTER JOIN "HOBBY" "HATED" ON "HATED"."USERS" = "USERS"."ID" WHERE "USERS"."FIRST_NAME" NOT LIKE :FIRST_NAME"
[ERROR] Errors:
[ERROR] JdbcRepositoryCreateIfNotFoundLookUpStrategyTests.derivedQueryShouldWork:50->AbstractJdbcRepositoryLookUpStrategyTests.callDerivedQuery:72 » InvalidDataAccessApiUsage No value supplied for the SQL parameter 'NAME': No value registered for key 'NAME'
[ERROR] JdbcRepositoryCreateLookUpStrategyTests.declaredQueryShouldWork:45->AbstractJdbcRepositoryLookUpStrategyTests.callDeclaredQuery:60 » InvalidDataAccessApiUsage No value supplied for the SQL parameter 'NAME': No value registered for key 'NAME'
[ERROR] JdbcRepositoryCreateLookUpStrategyTests.derivedQueryShouldWork:51->AbstractJdbcRepositoryLookUpStrategyTests.callDerivedQuery:72 » InvalidDataAccessApiUsage No value supplied for the SQL parameter 'NAME': No value registered for key 'NAME'
schauder
force-pushed
the
issue/1993-force-quoting
branch
from
2b4b8fe to
f0a0031
Compare
There was a problem hiding this comment.
I generally wonder about our testing approach. Enabling quoting requires downstream changes to every test we have where we interact with tables or verify SQL. The way table and column names are represented in the tests is a function of quoting and identifier processing.
How about having a set of test (like the dialect-specific ones in r2dbc.convert and the Template tests) that make use of quoted identifiers. For all other cases, we could disable quoting and keep tests as they are.
Quoting introduces another level of complexity while the verifications we run have a different semantic subject.
| */ | ||
| public class R2dbcMappingContext extends RelationalMappingContext { | ||
|
|
||
| /** | ||
| * Create a new {@link R2dbcMappingContext}. | ||
| */ | ||
| public R2dbcMappingContext() { | ||
| setForceQuote(false); | ||
| setForceQuote(true); |
There was a problem hiding this comment.
setForceQuote(…) could be removed entirely as it sets the value to its default value, see JdbcMappingContext.
| super(namingStrategy); | ||
| setForceQuote(false); | ||
|
|
||
| setForceQuote(true); |
There was a problem hiding this comment.
setForceQuote(…) could be removed entirely as it sets the value to its default value, see JdbcMappingContext.
Most are test more or less obvious test fixes.
Interesting things that became obvious:
TableNameQueryPreoprocessorUnitTestsRenderContextFactorhas aNamingStrategy!? That sounds all wrong to me. SeePostgresDialectRenderingUnitTestsRemoved IdentifierProcessing from PostgresLockClause since it was cause circular dependencies between class constructions.
Closes #1993
Supersedes #2047