Client config for boolean casting #15
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
voinik
commented
May 2, 2024
| @@ -71,6 +72,7 @@ export function valueResultFromProto(result: proto.StmtResult, intMode: IntMode) | |||
| const stmtResult = stmtResultFromProto(result); | |||
| let value: Value | undefined; | |||
| if (result.rows.length > 0 && stmtResult.columnNames.length > 0) { | |||
| // TODO: How do we solve this? AFAICS we don't have column data when fetching a single value, so we don't know when to cast ints to booleans | |||
There was a problem hiding this comment.
This is the biggest drawback of the whole PR I think. Since we don't get any column type data when fetching pure values, we can't know when we should cast. We'd have to make it very clear that the casting config only applies to query/queryRow and not when fetching values. Thoughts?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR was instigated due to my Discord post and my involvement of this Kysely GitHub issue. In short: after switching from PlanetScale to Turso I was expecting boolean DB values that I fetch in my program to be actual booleans, but they weren't! I learned that SQLite saves booleans as integers, and hrana doesn't cast them to booleans.
As a workaround, I then forked the @libsql/kysely-libsql Kysely driver to get it to pass hrana/libsql's selected column data so that I could write a Kysely plugin that can do the cast in my own application code. The plugin can be found in the README of my fork, if anyone's interested.
But this led me to wonder why hrana isn't doing that cast. When data is inserted (updates/creates), JS booleans are in fact cast to integers/bigints prior to insertion. I would expect this to be done the in other direction as well. Considering hrana is already going through all the columns in the query results, it seemed like unnecessary overhead to then do that again in the developer's application code. And thus this PR was born.
I've basically added an optional config parameter that has an optional
castBooleansproperty, for backwards compatibility. You can even use it to pass down other configuration in the future.I got this working with the different @libsql/libsql-client-ts clients with a few minor adjustments (passing the config down where needed, about 25 lines of changes in that repo in total).
Please let me know what you think!
Edit: I just now changed all my tables to STRICT. The issue with that is that we are no longer allowed to use other data types than INT, INTEGER, BLOB, TEXT, REAL and ANY. So
booleanisn't allowed. This makes my solution unusable for STRICT tables. I also noticed my solution (for now) only works if the column type was set to 'boolean', meaning it won't work for 'bool', for example; perhaps a.contains('bool')would suffice. Anyway, feel free to close this PR if you think there are too many conditions for this to work.