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

Fix placeholders being mapped to it's driver value instead of the value #2759

Merged
merged 10 commits into from
Aug 7, 2024

Conversation

veloii
Copy link
Contributor

@veloii veloii commented Aug 7, 2024

This fixes #2568. I've also modified the SQLite prepared statement test to include a boolean placeholder.

@AndriiSherman
Copy link
Member

Thanks a lot!
Could you please also add a few more test cases? We need to check if the same works for the update function in SQLite, and we need the same set of tests in the mysql.common and pg.common files.
If you don't have time for that, I can add those tests myself. Also, if you need any help, just ping me so I can guide you

@AndriiSherman
Copy link
Member

cc @veloii

@veloii
Copy link
Contributor Author

veloii commented Aug 7, 2024

Sorry - I was out. I'll add the tests in now. How should they be named?

@AndriiSherman
Copy link
Member

Sorry - I was out. I'll add the tests in now. How should they be named?

We can name it like: "insert: placeholders on columns with encoder", "update: placeholders on columns with encoder"

@veloii
Copy link
Contributor Author

veloii commented Aug 7, 2024

The set method on update doesn't allow placeholders. Is this a type bug or current limitation?

@AndriiSherman
Copy link
Member

Oh, I guess it's a limitation, or maybe we can't even use it in updates. We'll need to check. For now, just skip update tests and focus only on inserts

@veloii
Copy link
Contributor Author

veloii commented Aug 7, 2024

Sorry for the delay, should all be done now! I was unable to verify the MySQL test locally but I assume the action will test it anyway.

@AndriiSherman AndriiSherman changed the base branch from main to beta August 7, 2024 14:59
@AndriiSherman AndriiSherman merged commit d486e9b into drizzle-team:beta Aug 7, 2024
0 of 6 checks passed
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.

[BUG]: boolean mode not working with prepared statements (bettersqlite)
2 participants