search

chucknorris / roundhouse

RoundhousE is a Database Migration Utility for .NET using sql files and versioning based on source control
http://projectroundhouse.org
916 stars 249 forks source link

Postgresql case sensitivity #366

Open Roemer opened 5 years ago

Roemer commented 5 years ago

I noticed that all identifiers (like database name) are converted to lower case. Whereas Postgresql actually supports case sensitivity by default. The rule is, that if an identifier is inside quotes, it is case sensitive, if not, it is not case sensitive. Maybe that's something that should be considered in Roundhouse?

bladeoflight16 commented 5 years ago

The naming standard for identifiers in PostgreSQL is all lowercase separated by underscores. Sometimes known as snake-case (because Python popularized it).

Especially in the name of the database itself, there's no benefit to implementing case sensitivity. All it's going to do is cause errors and confusion when people try to connect. This would be especially troublesome in PostgreSQL where the database name must be specified as part of the connection string. "I spelled the name exactly right. Why isn't it connecting? What does it mean the database doesn't exist?"

As for case sensitivity for table and column names and the like, unquoted names are case insensitive by default, as you mention. You should be able to quote them in your script manually if you really need a specific case. E.g., you're trying to match your identifier names in your code.

This is just an opinion, but I'd honestly just advocate for avoiding case sensitivity if at all possible. It's a real pain when you're actually writing SQL in a client like pgAdmin. It's much more convenient to break the programming language's naming standards and match PostgreSQL's than the other way around.

erikbra commented 5 years ago

Just so tat I understand you correctly, @bladeoflight16, you think the current behaviour is "more correct" or more convenient in practice?

bladeoflight16 commented 5 years ago

@erikbra Both. The database name itself should never be case sensitive. There is no gain to doing so. Other names (tables, views, columns, etc.) created in scripts should be quoted in the script by the author if they want case sensitivity on them; anything in the script currently conforms to default PostgreSQL behavior, as far as I'm aware.

erikbra commented 4 years ago

@Roemer do you have any additional comments on this issue? If not, I'm leaning on closing it, given @bladeoflight16's comments

mnichols-dl commented 4 years ago

Seems like an odd hill to die on for a tool named "RoundhousE"...

I'm having to rename some databases in RDS that were created with PascalCasing. I can't connect to them with RoundhousE in the specific use case of dropping the database. It just seems unnecessary to enforce best practice in a way that breaks the tool for some instead of just expecting that people use the correct name in the connection string. If they don't know the casing or can't figure that out quick enough, that's on them, right?

erikbra commented 4 years ago

Just a question of clarification, @mnichols-dl , Are you not able to connect to the databases that have PascalCase, or do you have several databases that differ only in casing? If the first is the issue, then we have a more serious problem. Of course you should be able to connect to your databases no matter the casing.

I'm not fluent at all in PostgreSQL. Do you have any thoughts on how we could change RoundhousE so that it accepts any case of the DB? Do we use quotes where we shouldn't anywhere, or are there other issues?

Roemer commented 4 years ago

I was unable to do anything with a database that was not fully lowercase, I had to rename them in order for RoundhousE to work.

bladeoflight16 commented 4 years ago

@Roemer I suggest changing the title and description of the ticket to reflect that. That's a bug. Your request appeared to be asking for case sensitivity to be added to RoundhousE.

erikbra commented 2 years ago

@Roemer, could you please check if https://github.com/erikbra/grate solves this problem for you? I think it should.