spring-data-relational icon indicating copy to clipboard operation
spring-data-relational copied to clipboard

Introduce configuration-based switch to enable/disable identifier quoting [DATAJDBC-548]

Open spring-projects-issues opened this issue 5 years ago • 5 comments

Mark Paluch opened DATAJDBC-548 and commented

Based on the feedback from Dave Cramer, who runs the pgjdbc driver, we should consider a possibility to disable identifier quoting


Reference URL: https://twitter.com/dave_cramer/status/1262396128156422150

Issue Links:

  • DATAJDBC-553 Force quote default makes using multiple databases very hard ("is duplicated by")

1 votes, 2 watchers

spring-projects-issues avatar May 19 '20 08:05 spring-projects-issues

What is a solution 3 years later? I saw blog entry showing mappingContext.setForceQuote(false) - but shouldn't similar config be applied on column level to quote it when it is needed and not quote by default? SpringBoot with Postgres + HSQLDB is popular mix and it would be nice it is supported out of the box by Spring Data JDBC. Unfortunately documentation https://docs.spring.io/spring-data/jdbc/docs/3.0.2/reference/html/ doesn't mention about ForceQuote

kodstark avatar Mar 13 '23 15:03 kodstark

I have the opposite issue, where configuration to set forceQuote to true would also help: A pre-existing MariaDB database with a column group that I want to use with R2DBC, but fails on inserts, because GROUP is "special" syntax and causes a grammar error.

In my mind to solve #604, the dialect should be able to specificy a set of reserved identifiers that have to be quoted to prevent malformed sql being generated.

The forceQuote property is actually a bit of a misnomer -- what it does is set SqlIdentifier#quoted which in turn acts more like enableIdentifierProcessing when SqlIdentifier#toSql is called. Which I'd argue is a bug.

The dialect, specifically it's IdentifierProcessing should always get a chance to influence quoting and not depend on generic settings in the relational (meta-)model (i.e. BasicRelationalPersistentProperty#forceQuote)

What might solve this is:

  • Mark setForceQuote and getForceQuote as deprecated because it's easy to misuse because it's not "forcing quoting" but disabling identifier processing.
  • Add setPreferQuote and getPreferQuote instead, use those, rename the field
  • Add a new method to IdentifierProcessing
    default boolean shouldQuote(String identifier, boolean perferQuoted) {
        return preferQuoted;
    }
    
  • Add a new IdentifierProcessing subclass that considers a set of reserved words (or Predicate<String>) that will always return true from shouldQuote and preferQuoted otherwise.
  • Replace the condition of the ternary operator in DefaultSqlIdentifier#toSql and DerivedSqlIdentifier#toSQL with a call to shouldQuote giving the string and quoted.

This way, the current observable behavior doesn't change and dialects can be changed over time to specify their reserved syntax.

Xiphoseer avatar Jul 26 '24 14:07 Xiphoseer

@kodstark While I strongly recommend against using different databases in test and production (use Testcontainers instead) Spring Data JDBC supports running against the two databases just fine, as long as table and column names are the same, which they aren't if you use lower case names in postgres and upper case names in HsqlDB.

schauder avatar Aug 02 '24 07:08 schauder

@Xiphoseer

The forceQuote property is actually a bit of a misnomer -- what it does is set SqlIdentifier#quoted which in turn acts more like enableIdentifierProcessing when SqlIdentifier#toSql is called. Which I'd argue is a bug.

This is very intentional behaviour. This guarantees that we use the default conversion used by the database in use (upper case for most). Without that users of certain databases would have to quote each and every identifier in their database by default, which most people most certainly wouldn't like.

We decided explicitly against any handling of quoting based on keywords, because this is brittle and expensive in terms of maintenance, since we'd would have to monitor all supported databases for changes in keywords, and quoting identifiers resolves that issue.

schauder avatar Aug 02 '24 07:08 schauder