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
Issue #1684: Invalid query generated using OracleDriver for filter with Option column. #2059
Issue #1684: Invalid query generated using OracleDriver for filter with Option column. #2059
Conversation
…tion when suplied with default value through getOrElse.
Hi @alexFrankfurt, Thank you for your contribution! We really value the time you've taken to put this together. Before we proceed with reviewing this pull request, please sign the Lightbend Contributors License Agreement: |
Hi @alexFrankfurt, would you be willing to finish this? If you don't have time, let me know and I'll try to take over and bring this to completion. |
import slick.jdbc.OracleProfile | ||
|
||
class OptionBooleanTest extends AsyncTest[JdbcTestDB] { | ||
lazy val oracleProfile = tdb.profile.asInstanceOf[OracleProfile] |
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.
Can we change this test so that we can execute this for all databases? This is something that should work regardless of the profile that is used.
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.
Yes, I've made appropriate changes.
issue was reported 2 years ago now, and the fix was proposed 5 months ago. Can we please get this in? |
Guys, any news regarding this? We just migrated from squeryl to slick only to find that Slick can't properly handle boolean values..? |
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 am no expert on how these queries are generated, but this looks good to me.
FYI, I have just contributed/reviewed some PRs, but I am not able to merge
@kubukoz thank you for the PR |
@hvesalai not my PR ;) |
Happy to make slick better 👍 |
So sorry guys. Credit where credit is due. Thank you @alexFrankfurt ! |
Adds test and fixes #1684
Giving value by default and comparing with boolean like here wasn't working either. This PR implements fix for that case as well.