fix #230: Fix schema (in)sensitive casing issue #234
Conversation
| @@ -256,7 +256,7 @@ private async Task<bool> RunSchemaExists() | |||
| { | |||
| string sql = $"SELECT s.schema_name FROM information_schema.schemata s WHERE s.schema_name = '{SchemaName}'"; | |||
| var res = await ExecuteScalarAsync<string>(ActiveConnection, sql); | |||
| return res == SchemaName; | |||
| return res != null; // #230: If the server found a record that's good enough for us | |||
There was a problem hiding this comment.
Will it be null, or can it be DBNull?
There was a problem hiding this comment.
AFAIK handling the DBNull returned from the sql client is Dapper's job, and it converts to either null or default() depending on the type it's trying to map to.
If you have any info to the contrary I'd be very keen to know about it!
|
G'day @erikbra , just wondering if there's anything holding up this fix from your side? I tried to convert another existing Roundhouse/SQL server project to grate recently and still have to force v1.3.2 to get it to work... Hoping this and a few other fixes will get me closer to the line |
|
Hi, @wokket - thanks for being patient. No, nothing other than myself holding this up (but that is a big enough hurdle by itself sometimes). I resolved some merge conflicts, and I'm waiting for the tests to run, and I'm ready to merge if they run as they should. It's been hard to prioritise a lot of time on grate the last months, sorry. |
|
Seems like tests are failing on MariaDB and SqlServer, unfortunately, @wokket |
|
Probably not surprising after nearly 9 months 🤣 I'll try and see what's going on when I get a chance... I definitely feel like focussing on closing out some of these bugs/weirdness/idiosyncrasies that have crept in is a good thing. |
…spect the failing tests indicate the bug is still present on other DBMS's, but I don't know enough to go chasing that right now.
wokket
force-pushed
the
feat/#230-schema-casing-MkII
branch
from
f0f34c0
to
973fc48
Compare
|
OK, after fighting with inconsistent test runs locally for a few hours I think we still have some stuff to sort out here. In the current The new check for whether a table (like Do we want to change the table exists checks to return both the schema and table as found in the database and use that for the rest of the migration? I understand the case-insensitive lookups as a workaround for bug #245, and keeping back-compat with existing database already migrated is obviously paramount, but it feels like forcing case insensitivity in some places, and respecting the target casing rules are fundamentally incompatible :( |
|
Related: After one year, I don't think i issue #245 is actually fixed for postgres. I tried the latest 1.5.4 release, and exactly as a year ago, grate creates "ScriptsRun[Error]" and "Version" tables, even though "scriptsrun" and "version" exists. |
|
This one should be solved by. other means now :) |
Had some merge issues but think I'm there.
Assorted changes to DBMS's that are (generally) case insensitive.
PGSql is a little interesting with case-sensitive tables but case insensitive schemas (as currently implemented), but so be it.
There's possibly an option to bypass the
Existschecks entirely for those DBMS's that supportcreate table if not existssyntax, but that's a bridge too far for me time-wise atm.