sea-query
sea-query copied to clipboard
SeaQL
fix: sqlite datetime column type
Fixes
- [x] The datetime and date column types shouldn't be text, as stated by the official docs, they are numeric values.
Breaking Changes
- [ ]
The column type of datetime and date are changed from text to numeric. AFAIK the sea-orm migration tool is affected
Changes
- [ ]
in sqlite dates and times are mostly text based.
Mind sharing the source regarding this statement? I'm curious because the type affinity of datetime/date in sqlite is numeric, which means that if the literal DATETIME is used in the column schema, it will be numeric.
an option to do so would be the way to go.
The breaking change of the default behaviour concerns me as well. Where would you recommend to add such option? All I can think of is adding new ColumnTypes leaving the current values unchanged. But it requires adding new types only for sqlite, which may not be so elegant.
One last problem of the current behaviour is that the column type is converted to text in the table schema. As a result, sea-orm-cli generate entity command will not know if the column is datetime or plain text. So in the generated entities the datetime columns are always of type String. Manually fixing the types everytime you re-generate the entities is definitely not pleasant.
I mean the affinity of datetime is numeric, but since you cant store timezone in the numeric it is very common to use text for storage.
Sqlx seems to use the numeric and assumes UTC when converting back to time or chrono. Diesel on the other hand always uses text for storage and I am a big proponent of sea query being neutral in regard to the underlining engine.
This means sea query should support both. Though I am not against making the breaking change for the existing type and giving users an escape hatch with a new type. Many things in sea-query already work only for some databases. Its fine as long as it is documented.
@billy1624 I totally agree with you. There should be more beautiful ways to solve the column type infer problem. You can close this issue if the integrated solution will be tracked at https://github.com/SeaQL/sea-orm/issues/924