JasperFx / weasel

Database Development Made Easy for .Net
MIT License
65 stars 21 forks source link

Added support for case sensitive SQL generation for PostgreSQL #103

Closed oskardudycz closed 11 months ago

oskardudycz commented 11 months ago

Fixes #100

Note: Consider making case-sensitive comparison a default in the next major version. Also, remove the singleton PostgresInstanceProvider.

jeremydmiller commented 11 months ago

I hate our industry sometimes. Like I said in the Discord, I think I'd rather just explicitly deal with this in DDL generation code than make sweeping changes to the DbObjectName usage because that's everywhere in code that uses Weasel.

Could also soften this maybe by having a shorthand for PostgresqlProvider, but that kind of Fluent Interface usage where you have to use some kind of "static type.method" within fluent interface method calls tends to be bad for usability.

I'd even rather have a PgObjectName that inherits from DbObjectName and overrides the corrective behavior.

I always hate slowing down pull requests, but I'd rather we think on the usability here instead of just going with this.

oskardudycz commented 11 months ago

@jeremydmiller sounds fair, I'll try to check those options and see how they play with this case.

jeremydmiller commented 11 months ago

@oskardudycz I'm still not wild about any of this, but it's better. If you'd at least take off the [Obsolete] tag on the DbObjectName(string, string) constructor, I'm good with this. DbObjectName is used all over the Wolverine codebase to do SQL generation in ways that wouldn't be impacted by any of this case sensitivity stuff.

oskardudycz commented 11 months ago

As we discussed, setting Obsolete on DBObjectName makes it more explicit for users other than us. I think that we'll still need to have follow up PR to clean it up in major version, so as we agreed: I'm pulling that in 👍