New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for boolean datatype #43
Add support for boolean datatype #43
Conversation
🦋 Changeset detectedLatest commit: c7f6fe2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
I just occurred to me there is more on the default value side of this, which requires knowledge of the data type in order to have a valid default value. Working on that next. |
We'll also need to coerce the values that are returned on First() or All() if this is implemented, otherwise users may be expecting a Boolean and receive an integer, which could cause unexpected bugs. |
I've been thinking about this and there are a couple things to consider...
All because Javascript tests those values for thruthiness. Now if someone did a If we think the If we think this feature is useful in the ORM, perhaps merely documentation, explaining the situation, we choose not to coerce the value to an actual typed boolean. Meaning it will pass the |
I see your point about raising this to the level of the database rather than the ORM, so I'll definitely be mentioning it in an upcoming meeting I have with the PM, however I think this strict equality issue is important, so it's acceptable for us to implement currently, especially for a Typescript focused library. |
I'm reading lots of docs and different drivers examples in different languages as well as different drivers for typescript and javascript and everything leads me back to... strict type equality should not matter because NONE of the databases actually store a boolean as anything except a 0 or 1. The fact that Javascript provides strict equality to include the actual data type really should only matter if someone needed to check the data type, which in this case, if a user queried the database without the ORM they would only get a zero or one. I think if D1 provides support for it, awesome, but if not, it just doesn't seem like a big deal. |
My key issue here, alongside breaking strict equality, is the fact that a person using the library must supply a type when creating a Model, for example type User = {
id: number;
is_admin: boolean;
} We would then coerce the is_admin value to 1 or 0. When this value is returned for the user, we would then receive said 1 or 0, which doesn't match the type provided, meaning their type definition is incorrect. One of the pieces of upcoming work I have planned is to automatically generate the type from the Model's definition, where it could then automatically be set to In it's current form, this will result in unexpected behaviour that could otherwise be mitigated. |
To be clear, I'm not disagreeing with your rationale... From a usage perspective however, I can't recall the last time I've seen even a single line of code written that looks like: if (user.is_admin === true) {
// do x
} It is always: if (user.is_admin) {
//do x
} |
That's true, the key problem is the mismatched types, you might, say, compare a User you have inserted into the database (has it as an integer), with a user you have yet to insert, etc. Seeing as we know in advance which columns will have booleans, it'll be very cheap to compute this, rather than iterating over every key |
I understand that example but going back to how data / records are used, I'm not sure the example you provide makes a lot of sense, because... a record is created, and stored in the database for persistence... it is later pulled out to be used... Comparing if userA is equal to userB isn't a valid use case because you have to walk the objects to do a comparison.. that is to say, for what use case were you holding onto the record to do a comparison later? The data could have been changed in the database. Comparing userA to userB is more likely to be done by "select * from db where user name=X ..." no comparison necessary If you were performing field by field value equality it would still pass that test. Anyways, just thoughts... |
Ref #44, this is no longer an issue! |
Implemented in b214e74, but thanks for the contribution! |
Sqlite supports a boolean datatype which gets automatically coerced into a 0 or 1.
The new datatype was added, along with type coercion to support all the query types.
The change to support this ends up being fairly trivial, but I would love feedback and any thoughts on the implementation or suggestions on alteration.
I think this implementation sets it up to allow other types of coercion with even less updates.