Incorrect handling of object name literals, when special characters / case-sensitivity is involved

General Discussion
1

That’s a valid point, I updated my pull request regards oracle fixes with a more simpler quoting approach for QUOTE_ALL_OBJECTs. The reasoning for smart quoting of the objects is no longer relevant.


Regards not simply quoting everything, this would break backwards compatibility for I imagine most users of Liquibase.

2

https://github.com/liquibase/liquibase/pull/115/commits


is the pull request

3

I posted a pull request for setting an object quoting strategy that Nathan merged.


If set to legacy, it’s the same as now

If set to QUOTE_ALL_OBJECTS, then any object name that is case sensitive in is quoted.

e.g. TESTTABLE in oracle would not be quoted but testTable would be. 

in PostgreSQL, testtable would not be quoted but TESTTABLE would be.



4

I have just registered two bug reports here:

I feel that the handling of case-sensitivity and special characters is generally incorrect in Liquibase. Here’s a sample implementation from Liquibase’s PostgresDatabase:

  1.     public String escapeDatabaseObject(String objectName) {
  2.         if (objectName == null) {
  3.             return null;
  4.         }
  5.         if (objectName.contains("-") || hasMixedCase(objectName) || startsWithNumeric(objectName) || isReservedWord(objectName)) {
  6.             return "\"" + objectName + "\"";
  7.         } else {
  8.             return super.escapeDatabaseObject(objectName);
  9.         }
  10.     }

I strongly recommend that you do not implement such “clever” rules like “hasMixedCase()”, etc, as these will not include all the corner cases. From my experience, case-sensitivity is not something that you can “discover” with such rules. Hence it is best to treat ALL literals as case-sensitive. Do note that, you should also escape the literals that you quote.


I think it’s worth investing a little time to handle these things correctly. I’m the developer of jOOQ and I have gathered experience with 14 database dialects. My experience lead to the following rules:

  • It is best to always escape ALL literals (except for the SQLite database, which is a bit buggy)
  • MySQL supports different ways of escaping. Using backticks ( ` ) works best, though.
  • T-SQL databases (Sybase ASE, Sybase SQL Anywhere, SQL Server) use brackets ( [ ] )
  • All other databases implement the SQL standard, by using double quotes ( " )

Here’s a sample piece of code, taken from jOOQ’s DefaultRenderContext (feel free to reuse it):

  1.             switch (configuration.getDialect()) {

  3.                 // MySQL supports backticks and double quotes
  4.                 case MYSQL:
  5.                     sql("`").sql(StringUtils.replace(literal, "`", "``")).sql("`");
  6.                     break;

  8.                 // SQLite is supposed to support all sorts of delimiters, but it
  9.                 // seems too buggy
  10.                 case SQLITE:
  11.                     sql(literal);
  12.                     break;

  14.                 // T-SQL databases use brackets
  15.                 case ASE:
  16.                 case SQLSERVER:
  17.                 case SYBASE:
  18.                     sql("[").sql(StringUtils.replace(literal, "]", "]]")).sql("]");
  19.                     break;

  21.                 // Most dialects implement the SQL standard, using double quotes
  22.                 case CUBRID:
  23.                 case DB2:
  24.                 case DERBY:
  25.                 case FIREBIRD:
  26.                 case H2:
  27.                 case HSQLDB:
  28.                 case INGRES:
  29.                 case ORACLE:
  30.                 case POSTGRES:
  31.                 default:
  32.                     sql('"').sql(StringUtils.replace(literal, "\"", "\"\"")).sql('"');
  33.                     break;



5

What if the actual table name “test table” was case-sensitive as in:

  1. CREATE TABLE "test table" (
  2.   "a" INT,
  3.   "B" INT
  4. )

Funny, that QUOTE_ALL_OBJECTS would only quote some objects, though :slight_smile:


I don’t understand the aversion against quoted literals. Some people may find them a bit unreadable, so it might make sense to introduce a QUOTE_NO_OBJECTS or QUOTE_SOME_OBJECTS strategy for them. But if you quote ALL objects by default, you cannot run into these kinds of bugs…

6

I think it’s better to have two different modes, one is implemented for now and one you suggested.

The reason for current mode: for example, i work with oracle DB and postgres DB. And i write SQL queries by hands, aligning table names like i wish - MyCoolNameA A inner join MyCoolNameB B etc.

But in PostgreSQL it will be converted internally by DB engine to mycoolname, and in Oracle to MYCOOLNAME.

But i want to have one changeset for both DBs and I can write it or in uppercase, or in lowercase


The reason for mode you suggested: ORM frameworks of different types who automatically quotes table and column names when generating SQLs, so if I write in changeset MYCOOLNAME or mycoolname - both won’t work on the same time on Oracle and Postgres.


7

Sorry, wasn’t logged.


Nikolay Ustinov

8

Another point in favor quoting/escaping is that some words are reserved words. I currently have a changeset written like this:



because it turns out that MySQL sees the naked word ORDER in the generated SQL and panics.

Manually putting in backticks is ugly, and it will break someday when Liquibase starts doing the right thing, so I would very much rather not have to do it in the first place.

9

Actually, how can I go about specifying QUOTE_ALL_OBJECTS? I’m looking for a property to set on SpringLiquibase, but it isn’t there.