Case issue on materialized='table'

[st3vemartin]

When executing my project against Redshift I was having an issue with the table not being dropped / recreated. It turned out that it was a case sensitive issue, where dbt would create the table name as "Account", but was (most likely) dropping a table if exists of "account" - Giving me a pesky table exists error. I changed my sql file name from "Account" to "account" and the table exists error cleared.

[cmcarthur]

thanks @st3vemartin! it's likely that this is caused by inconsistent use of adapter.quote in materialization code. someone will take a look.

[ridhoq]

Hey there, I was looking around for a good issue to start contributing and I found this.

Is this the line where adapter.quote would need to be used?

What kind of tests should be added along with this fix? I don't see a Redshift unit test - should we start a new one?

[drewbanin]

hey @ridhoq! It's not totally clear what the correct behavior should be here. We can

1. make sure we quote identifiers everywhere, or
2. don't quote identifiers anywhere

There's another complication here: Databases like Snowflake treat identifiers differently: #567

Let me give this some more thought -- I think we'll end up wanting to not quote identifiers

[ridhoq]

hey @drewbanin! My (perhaps superficial) concern with not quoting any identifiers is that a user could supply a reserved word as an identifier which could result in the db throwing an error. But, I'm obviously not familiar with the codebase and we may already be handling this elsewhere - I'll defer to you :)

[drewbanin]

yeah, good point @ridhoq. That was our initial impetus for quoting these identifiers! I think though, that folks probably shouldn't be naming their models or columns after database reserved words. We have another feature, alias that can help with this too.

We need to be consistent with quoting everywhere, and I like the idea of enabling users to opt-in to quoting rather than having dbt decide for them. This isn't such a huge deal on Redshift, but there's a big difference between table_name and "table_name" on Snowflake, for instance. I'm definitely open to alternatives!

[ridhoq]

@drewbanin gotcha. I agree - users shouldn't be using identifiers with database reserved words. The only time I think it could trip somebody up is if they were to switch to a different database with their same models and find that one of the identifiers they used previously is a reserved word in the new database that they're using. But maybe that's an extreme edge case as I can't even think of an example lol

Opting into quoting sounds like a very reasonable approach (and would solve the above edge case). I would want to make sure that it's easy to implement new database engines as well since each database has their own weird rules around escaping/quoting - SQL Server comes to mind. Do you see this "force quoted identifiers" setting as a command line argument or a setting in the profile?

[drewbanin]

fixed by: #727 (going out in 0.10.1)