SanderGi / sqlite-auto-migrator

Simple, flexible and automated SQLite database migration tool
https://www.npmjs.com/package/sqlite-auto-migrator
MIT License
0 stars 1 forks source link

Migration changes all my columns to lower case #5

Open buzzy opened 2 weeks ago

buzzy commented 2 weeks ago

This one had me scratching my head for a long time! It seems that the migration changes my column names, which made code fail.

Having a .sql like this:

image

Created a column with the name "pushtoken". Notice the small "t":

image

This makes code like this fail:

image

In the map-function, the row.pushToken is undefined, as the column returned from sqlite is now row.pushtoken

I know sqlite itself is case-insensitive, but TypeScript is and it would be nice to not be forced to use all lower-case names.

SanderGi commented 2 weeks ago

Good catch. A quick workaround until I get a fix out would be to specify the desired output case in your sql queries, e.g. SELECT id as id, pushToken as pushToken FROM sessions WHERE user = :user AND pushToken IS NOT NULL;. I could see this being good practice in general (though especially when dealing with a case insensitive language inside a sensitive one) since the SQLite documentation otherwise provides no guarantee as to the column names returned: image Without specifying the case you want in your output, your code could theoretically break between sqlite versions (unless Bun provides stronger guarantees than the SQLite docs, haven't been able to verify if they do).

My thoughts on a more convenient solution would be to add a flag to the migrator case_sensitive_identifiers which defaults to false. When true, it would treat case changes to identifiers (column/table/view/trigger/virtual table names) as renames and migrate the database to match the case. This ensures backwards compatibility, can be enabled at any time to update existing identifiers to match case with the schema, and avoids potential diverging database states that could occur with other solutions. Let me know what your thoughts are and if you would prefer a flag for each identifier type.

buzzy commented 2 weeks ago

I switched to all lower-case name, so I do not really need it. However, I think it still makes sense to add it, because I would assume most people would assume their definition from the .sql-file to be left unchanged. I also think it's quite unlikely that people would use AS to specify the same name again. I have never seen that being done "in the wild".

SanderGi commented 2 weeks ago

Good points. This makes me think case sensitivity should be the default behavior of the migrator to avoid confusion. The only caveat is that it would have to use the first appearance of the column name in the schema as the canonical name. If it interpreted the schema itself case sensitively, then you could have situations where you declare a trigger/foreign key/etc. with a different case version of the column/table name. It would not make sense for the migrator to treat this as a reference to a separate column/table since that is not how SQLite would interpret it if executing the schema. This itself could cause some confusion regarding which case the migrator is using. Perhaps the migrator should throw an error when parsing a schema that uses different cases of the same identifier to avoid any confusion?

buzzy commented 2 weeks ago

Yes, that sounds logical. People should not mix cases for the same field anyways.