Add support for boolean datatype

[kingmesal]

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.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: c7f6fe272ac5cc6e769235dd5791d3398b491616

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

| Name | Type | | ------ | ----- | | d1-orm | Minor |

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

[kingmesal]

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.

[Skye-31]

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.

[kingmesal]

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...

1. I had not considered that to be an issue, because it didn't even cross my mind because I am accustomed to writing code like:

   const F = 0; const T = 1;
   if (F == false) // yields true
   if (F == true) // yields false
   if (T == true) // yields true 
   if (T == false) // yields false

   All because Javascript tests those values for thruthiness. Now if someone did a === the test would fail.

2. Logically this type of a feature should be implemented at the DB (in this case D1) layer. Clearly it is not yet...

If we think the === is a legit issue, then we should defer this feature to the database (i'm ok with that), because, I think that type checking/validating & coercing EVERY value returned in every query result will end up being too expensive for the benefit of getting a "boolean" equivalent value.

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 == but will NOT pass the === test. Then if some point in the future the D1 driver returns a typed boolean, then === should just work.

[Skye-31]

coercing EVERY value returned in every query result will end up being too expensive for the benefit of getting a "boolean" equivalent value We can improve the performance of this quite a lot by computing the columns that are the boolean type in the Model Constructor, and only modifying those values when returning.

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.

[kingmesal]

coercing EVERY value returned in every query result will end up being too expensive for the benefit of getting a "boolean" equivalent value We can improve the performance of this quite a lot by computing the columns that are the boolean type in the Model Constructor, and only modifying those values when returning.

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.

[Skye-31]

NONE of the databases actually store a boolean as anything except a 0 or 1 Postgresql has proper support for booleans.

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 is_admin: 1 | 0.

In it's current form, this will result in unexpected behaviour that could otherwise be mitigated.

[kingmesal]

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
}

[Skye-31]

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. I wouldn't feel comfortable merging this PR with the inconsistency in data types currently.

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

[kingmesal]

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...

[Skye-31]

Ref #44, this is no longer an issue! We can infer from DataTypes.BOOLEAN that the constraint will be 1 | 0, rather than true (meaning we also don't need to coerce the types they provide!)

[Skye-31]

Implemented in https://github.com/Interactions-as-a-Service/d1-orm/commit/b214e74a5c7c56fb593008b87bf819affb52fb47, but thanks for the contribution!