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

Should PostgresCompiler quote identifiers by default? #671

Open
mbcrawfo opened this issue May 17, 2023 · 3 comments
Open

Should PostgresCompiler quote identifiers by default? #671

mbcrawfo opened this issue May 17, 2023 · 3 comments

Comments

@mbcrawfo
Copy link

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.

@ahmad-moussawi
Copy link
Contributor

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

@cg-roling
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants