querybuilder icon indicating copy to clipboard operation
querybuilder copied to clipboard

Should PostgresCompiler quote identifiers by default?

Open mbcrawfo opened this issue 1 year ago • 3 comments

Throwing this out as a discussion topic because as far as I can find it hasn't been mentioned before.

A quirk of PostgreSQL is that quoting identifiers affects the behavior for how identifiers are handled. Quoting from the Postgres docs:

Quoting an identifier also makes it case-sensitive, whereas unquoted names are always folded to lower case. For example, the identifiers FOO, foo, and "foo" are considered the same by PostgreSQL, but "Foo" and "FOO" are different from these three and each other.

AFAIK, it's usually considered a Postgres best practice to avoid to avoid quoting identifiers unless you must use a name that collides with a reserved keyword. I don't know the history of SqlKata, but I suspect that like many .Net tools it probably started out focused on the MS stack (SQL Server) and was eventually expanded to support other systems. And of course in T-SQL delimiting an identifier with square brackets doesn't change any behavior, and it's not unusual to preemptively put delimiters around everything "just in case."

So given that context, the default behavior of PostgresCompiler can be a little annoying. For example if you have a DTO whose property names match your column names (using a non case-sensitive comparison) and you want to dynamically generate select statements using the property names, it simply won't work - you have take the extra step of transforming the names to lower case yourself. This feels like an unexpected behavior since I think most people are used to SQL being case-insensitive, and it can be confusing since many people aren't intimately familiar with the rules for quoting names in Postgres (they've just been taught not to do it). However it's not a show-stopper, since most people who are just typing column names in their code probably won't run into a problem.

Honestly I feel like the quoting behavior should probably be removed from PostgresCompiler entirely. I'm struggling to think of a scenario where I would want identifiers to be quoted by default. However this would be a breaking change if anyone is relying on that behavior, so instead perhaps introduce a property to control it so that something like new PostgresCompiler { QuoteIdentifiers = false } is available. I'd also suggest a callout in the documentation recommending to disable quoting unless you specifically need the legacy behavior.

mbcrawfo avatar May 17 '23 16:05 mbcrawfo

Thanks @mbcrawfo for the head up, but keeping quotes have two main advantages here:

  • Table/Columns created with quotes have to be called with quotes also.
  • Quotes help in preventing SQL injection.

Putting an option to disable quotes, is something that I am happy to discuss if we can overcome the above points

ahmad-moussawi avatar Jun 19 '23 14:06 ahmad-moussawi

I'm in a situation where we require identifiers to be unquoted, for both SQL Server and Postgres. Is that possible with SQLKata?

Situation: Migrating from SQL Server to Postgres, lowercasing identifiers during the migration, but we have a reporting system with hardcoded, mixed-case identifiers. Excluding quotes will allow both databases to work with a lowercased schema. We do not have any identifiers which require quoting.

cg-roling avatar Jul 02 '24 22:07 cg-roling

Replying with an answer to my own question: you can implement a custom compiler pretty easily: https://stackoverflow.com/questions/66053712/sqlkata-is-there-any-way-to-tell-the-sqlkata-compiler-to-not-wrap-identifiers

Unfortunately, this requires the 3.0.0 beta

cg-roling avatar Jul 30 '24 21:07 cg-roling