Skip to content
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

PostgreSQL support - proposal n.2: change tinyint to bools in database schema #495

Closed
wants to merge 4 commits into from

Conversation

xlii-chl
Copy link
Contributor

This pull-request is a little more ambitious than #494 as it changes the database schema. The code change is bigger and more likely to introduce breaking scenario, but since MySQL & MariaDB implictly convert booleans fields to tinyint and since SQLite isn't strongly typed, the impact on existing deployments may still be acceptable.

Those patch set up a new casting class, taking care of the strange behaviour of PDO/PostgreSQL fetching TRUE/FALSE values as (string)'t'/(string)'f' by default. For now, this patch is still a work in progress as I would prefer and still hope to use PDO::PARAM_*.

Like #494 :

  • Tests added to run on MySQL and PostgreSQL (on top of already running on SQLite). Since it seems that CodeIgniter 4.1.x sadly doesn't support PHP 7.3 anymore, I removed it from the tests' matrix.
  • Those patches also fix a couple of typos.

(on top of the run with SQLite)
This patch allows Myth:Auth to support all three of SQLite, MariaDB and
PostgreSQL.

- change 'tinyint' to 'boolean' on the migration,
- switch 0/1 to true/false coherently throughout the code.

Fix lonnieezell#324
@MGatner
Copy link
Collaborator

MGatner commented Feb 23, 2022

I will let @lonnieezell handle this one, but at the very least I think the database change should be an additional migration file rather than modifying the current one.

@xlii-chl
Copy link
Contributor Author

Noted.

Actually, the more I think about it, the less I'm convinced this is the way to go. It would be nice to have boolean fields being actual boolean types, but the difference between the DB storing as 0/1 and PostgreSQL storing as t/f may bring too much hassle for what it's worth.

Ping me if you'd like to see more, but I'll probably close it soon and focus on #494 otherwise.

@lonnieezell
Copy link
Owner

I think focusing on #493 is probably the better route here. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants