-
Notifications
You must be signed in to change notification settings - Fork 43
Add native boolean type support to QueryBinding #141
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
Conversation
This change introduces proper boolean type handling in StructuredQueries to better support databases with native boolean types (like PostgreSQL) while maintaining backward compatibility with SQLite. ## Changes ### Core Features - Add `.bool(Bool)` case to `QueryBinding` enum - Update `Bool`'s `queryBinding` to use `.bool(self)` instead of `.int()` - Add boolean handling in `debugDescription` (returns "1"/"0" for SQLite compatibility) ### SQLite Support - SQLite driver converts `.bool` to integer (0/1) for compatibility - Tests continue to work with existing SQLite behavior ### Trigger Support - Add proper boolean handling in trigger statements ### Additional Improvements - Make `iso8601String` property public on Date extension - Make `quoted` method public in Quoting extension These visibility changes allow database drivers to implement their own database-specific SQL rendering methods (e.g., `postgreSQLDescription`) without duplicating these utility functions. ## Motivation Previously, boolean values were always encoded as integers, which caused type mismatch errors when working with PostgreSQL's native BOOLEAN type. This change allows database drivers to handle booleans appropriately: - SQLite continues to use integer representation (0/1) - PostgreSQL can use native BOOLEAN type - Other databases can implement their preferred boolean handling ## Testing All existing tests pass. The change is backward compatible and does not break existing SQLite-based code. Fixes compatibility issues with PostgreSQL and other databases that have native boolean types.
|
@coenttb Thanks! Do you have time to open a SharingGRDB PR against this branch to see what needs to change? Unfortunately these kinds of breaking changes will need to be coordinated. |
|
@stephencelis Done! SharingGRDB PR opened at #141 with minimal changes needed. Just adding the .bool case handling. |
|
Somehow you opened dual 141s 😄 https://github.com/pointfreeco/sharing-grdb/pull/141 |
Sources/StructuredQueriesCore/QueryRepresentable/Date+ISO8601.swift
Outdated
Show resolved
Hide resolved
| case .bool(let bool): | ||
| return bool ? "1" : "0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the minor nit, but we like to alphabetize for the most part (invalid is an outlier which is why it comes last).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered that and originally put it there, too. Was thinking it might signal 'this is not an sql type' to put it at the bottom.
|
@coenttb We intended to merge this soon. Any reason why you closed it? |
|
Didn't intentionally do it. But I would also advise against adopting this, and think database adapters through a little more. For my use, I decided to fork the repo and rewrite it for just Postgres. There were more foundational incompatibilities that I came across. I'm a little swamped, but from memory the primary blocking factor was in the Table protocol. Additionally, I had to make some edits in the macros as Postgres handling for nil IDs had to be done a little differently. You can check my progress at swift-structured-queries-postgres. |
|
While we do want to explore that soon, we think the tools are stable enough to cut a 1.0 relatively soon and think these gaps should be handled. We can wait for 2.0 for a better version in the future. |
Summary
This PR introduces proper boolean type handling in StructuredQueries to better support databases with native boolean types (like PostgreSQL) while maintaining backward compatibility with SQLite.
Problem
Previously, boolean values were always encoded as integers (
0/1), which caused type mismatch errors when working with PostgreSQL's nativeBOOLEANtype:Solution
.bool(Bool)case to theQueryBindingenumChanges
Core Features
.bool(Bool)case toQueryBindingenumBool'squeryBindingto use.bool(self)instead of.int()debugDescription(returns"1"/"0"for SQLite compatibility)SQLite Support
.boolto integer (0/1) for compatibilityTrigger Support
Additional Improvements
iso8601Stringproperty public on Date extensionquotedmethod public in Quoting extensionImpact
Testing
All existing tests pass. The change is backward compatible and does not break existing SQLite-based code.
Related
This change enables proper PostgreSQL support in downstream libraries like
swift-structured-queries-postgres.